From patchwork Mon Sep 8 21:34:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 37016 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qa0-f72.google.com (mail-qa0-f72.google.com [209.85.216.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D0BF220491 for ; Mon, 8 Sep 2014 21:37:51 +0000 (UTC) Received: by mail-qa0-f72.google.com with SMTP id cm18sf43472484qab.7 for ; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:in-reply-to:message-id :references:user-agent:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-post:list-help:list-subscribe:sender :errors-to:x-original-sender:x-original-authentication-results :mailing-list:list-archive:content-type; bh=b0D/uwq19sjWtDgBpA4DKhyVQ1cchPaArBNDBodxJ+g=; b=CWi6Otuk9KZrd+swbCejvKoOYsXFpfKC+URy+g1dT0ZGAJ3JwilbiCamHCp1AQiG9C VKntcq37QXwiEMq7TOeqfy8/Qxh/PkKdMSwg6EM3oYF1M9o/q4olRpN6+OaCAtbtbpJ5 MBmhy6fOYv9ej3YMQFkKdzFo5atK5WOpzAYdnnJHtg7ee145Pg44KVodO3dg59sML+ol dXKvJRaA5F6F1y38tLufiy1UcqKMgQHj2abXbTYQ0Pv81TD5QFdkEs1y754zli1iCYtC LzUsllxvncTuiO29s6nBZDTitBLHnwfdFo3jFP82CR9PX+o9g4SDiyAf51NcjaDwuXpp TqKA== X-Gm-Message-State: ALoCoQlHKkHZmSr8ZcC5bJD/lWAYiGm3cA2kbafPwnHTOYhu/fN/xcRvynv91uvl5/TezRy7mcD5 X-Received: by 10.52.182.66 with SMTP id ec2mr19353032vdc.0.1410212271602; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.18.193 with SMTP id 59ls1952593qgf.7.gmail; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) X-Received: by 10.52.75.100 with SMTP id b4mr1739892vdw.66.1410212271499; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) Received: from mail-vc0-f178.google.com (mail-vc0-f178.google.com [209.85.220.178]) by mx.google.com with ESMTPS id w7si4466592vcn.49.2014.09.08.14.37.51 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 08 Sep 2014 14:37:51 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.178 as permitted sender) client-ip=209.85.220.178; Received: by mail-vc0-f178.google.com with SMTP id hy4so1497139vcb.9 for ; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) X-Received: by 10.221.64.142 with SMTP id xi14mr11579440vcb.31.1410212271423; Mon, 08 Sep 2014 14:37:51 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.45.67 with SMTP id uj3csp193487vcb; Mon, 8 Sep 2014 14:37:50 -0700 (PDT) X-Received: by 10.52.245.101 with SMTP id xn5mr10113041vdc.32.1410212270789; Mon, 08 Sep 2014 14:37:50 -0700 (PDT) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id o15si4465612vct.51.2014.09.08.14.37.50 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 08 Sep 2014 14:37:50 -0700 (PDT) Received-SPF: none (google.com: xen-devel-bounces@lists.xen.org does not designate permitted sender hosts) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XR6bU-0007zF-R5; Mon, 08 Sep 2014 21:36:04 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XR6bT-0007z0-CP for xen-devel@lists.xen.org; Mon, 08 Sep 2014 21:36:03 +0000 Received: from [85.158.139.211:22067] by server-7.bemta-5.messagelabs.com id C7/F2-30869-2412E045; Mon, 08 Sep 2014 21:36:02 +0000 X-Env-Sender: Stefano.Stabellini@citrix.com X-Msg-Ref: server-2.tower-206.messagelabs.com!1410212160!13229565!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n X-StarScan-Received: X-StarScan-Version: 6.11.3; banners=-,-,- X-VirusChecked: Checked Received: (qmail 19264 invoked from network); 8 Sep 2014 21:36:01 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-2.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 8 Sep 2014 21:36:01 -0000 X-IronPort-AV: E=Sophos;i="5.04,488,1406592000"; d="scan'208";a="169510899" Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.80) with Microsoft SMTP Server id 14.3.181.6; Mon, 8 Sep 2014 17:35:59 -0400 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1XR6bP-0003Jt-6w; Mon, 08 Sep 2014 22:35:59 +0100 Date: Mon, 8 Sep 2014 22:34:14 +0100 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: Vijay Kilari In-Reply-To: Message-ID: References: User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-DLP: MIA2 Cc: "xen-devel@lists.xen.org" , Ian Campbell , Stefano Stabellini Subject: Re: [Xen-devel] xen/arm: GIC: unable to inject hw irq warning X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: stefano.stabellini@eu.citrix.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.178 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: On Sat, 6 Sep 2014, Vijay Kilari wrote: > On Thu, Sep 4, 2014 at 4:19 AM, Stefano Stabellini > wrote: > > On Wed, 3 Sep 2014, Vijay Kilari wrote: > >> Hi Stefano, > >> > >> I face following warning messages for LPI/ITS interrupt. > >> Can you throw some light on this warning? > >> > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR0 > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR0 > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR1 > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR1 > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR1 > >> (XEN) gic.c:394:d0v0 unable to inject hw irq=8736 into d0v0: already > >> active in LR0 > > > > Hello Vijay, > > the warning is for a condition that should not happen. All the following > > must be true for the warning to trigger: > > > > - the irq is an hardware irq (p->desc != NULL) > > > > - the irq is active in the LR register (GICH_LR_ACTIVE) > > > > - we need to inject a second irq while the first one is still active > > > > The GICv2 specification explicitely states that it is not possible to > > set an hardware irq as pending and active in the LR registers (5-175). > > Here in the code we are checking only for LR active state(0x2) and not > for ''pending and active' (0x3) state. True. If the irq is GICH_LR_ACTIVE we do not set it GICH_LR_PENDING for hardware interrupts. > > Regardless from the GICv2 specification, it shouldn't be possible to > > receive a second hardware interrupt in Xen while the guest is still > > handling the first one. So the question is: how is it possible that xen > > is even trying to inject a second irq while the first one is still > > active? I would check how GIC_IRQ_GUEST_QUEUED was set. > > Where in GICv2 specification it is men>tioned?. It is explained in the general handling of interrupts, 3.2. "When a processor takes the interrupt exception, it reads the GICC_IAR of its CPU interface to acknowledge the interrupt. This read returns an Interrupt ID, and for an SGI, the source processor ID, that the processor uses to select the correct interrupt handler. When it recognizes this read, the GIC changes the state of the interrupt as follows: * if the pending state of the interrupt persists when the interrupt becomes active, or if the interrupt is generated again, from pending to active and pending. * otherwise, from pending to active" Later in the same chapter: "A CPU interface never signals to the connected processor any interrupt that is active and pending. It only signals interrupts that are pending and have sufficient priority" > In case of GICv3 LPI's are edge triggered and there is no active state. > Once the hypervisor has EOI'd the LPI, it will be delivered again if > the device generates a new edge. See 4.3.2 and 4.1.5 Thanks for the pointer, very interesting! The spec also says that LPIs are never ACTIVE, so how is it possible that the irq is ACTIVE in the LR register in your case? Maybe only physical LPIs are never active but virtual LPIs can be active? That is plausible but very confusing. Could you please run a few tests to confirm this theory? If that is correct then I think we'll have to make a few changes to accommodate LPIs: - do not set the HW bit in GICH_LRs for LPIs, because the corresponding physical irq doesn't need to be deactivated anyway - once we remove the HW bit, LPIs become purely virtual irqs at the GICH_LR level, therefore we can set them ACTIVE & PENDING in GICH_LR from gic_update_one_lr See the attached patch as reference, not even compile tested. If the LPI is already PENDING in GICH_LRs, it is still possible to loose an interrupt but I think that is acceptable (interrupt coalescing). > In case of Xen if interrupt is already in LR (pending or active), Xen > ignores the interrupt right? > If so, are we not missing the interrupt? Yes, we do loose an interrupt but such cases shouldn't happen in the first place with hardware interrupts on GICv2. For LPIs we need to make changes. diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index 78ad4de..576512e 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -395,7 +395,7 @@ static void gicv2_update_lr(int lr, const struct pending_irq *p, << GICH_V2_LR_PRIORITY_SHIFT) | ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)); - if ( p->desc != NULL ) + if ( p->desc != NULL && !is_lpi(p->irq) ) { if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) ) lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ; diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 6611ba0..74e2ab4 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -343,7 +343,7 @@ static void gic_update_one_lr(struct vcpu *v, int i) if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) && test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status) ) { - if ( p->desc == NULL ) + if ( p->desc == NULL || is_lpi(irq) ) { lr_val.state |= GICH_LR_PENDING; gic_hw_ops->write_lr(i, &lr_val); diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index a0c07bf..1b3b0fc 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -154,6 +154,11 @@ #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \ DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7) +static inline bool_t is_lpi(int irq) +{ + return irq >= 8192; +} + /* * GICv2 register that needs to be saved/restored * on VCPU context switch