From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 43C8521184ACA for ; Wed, 7 Nov 2018 07:23:14 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id w7-v6so23220587itd.1 for ; Wed, 07 Nov 2018 07:23:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Z/BQoFmJ6GkRRVszbVal2x9AfSIP2XOr+7Ka6y3iykw=; b=bB9Frbp2DQnri1q7qHyuNgXivs+4K1Sk/YK0p6Q9QXOMjfnmlDJk0t1XrnvW1nGMUW GFlerIDlMATkqYvDdmABk4NQXJBAjg9PF6ptZ/2a0pLrBFTJGAIqqn6DwGcqAXcNpS1t h2co9GT3mqBH2yfN/AC0M60vk5qAO1geS0+7M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Z/BQoFmJ6GkRRVszbVal2x9AfSIP2XOr+7Ka6y3iykw=; b=rWnjK2k7p2F8pNe8fcYP0nA76wLeaci8hWMuUAGpsqZDmin2Ycz26tJzS9KCdxCJH5 Rvor+/iwvHwsw7Dvd4sq02u1neV3Uf0mb3zgJrp2G9ohk0hfkrMjH+2YwFJ8DMFF0zve 0T0j71KC16roRaIim+J+Z7HRS5+2WWckJ+UEu69jTDy/W74ZXl/QhsmsJkpmFrwSQXE2 Iq0x4xgS0Vb+ilkndMjq37oy0/rPB4+C3xmUryFg8QUeBhCwK5uKrrCjrPxe6WwimhLR hyvZJcPhi+fFb5XzgQ/Bbb8oB/G2orK/QdRdLFjMLX6Nlo84nOxoRQ1UmSD218X1vCjb UpdQ== X-Gm-Message-State: AGRZ1gKVaYBmm/gpRQErfGXaUzWcKE3pn+CjlWUjFVI3Mb06M61xxbnG QMkngbD6gryGuAgJ0w4NRJgexDOlbl/yDW978N3bhw== X-Google-Smtp-Source: AJdET5f5ABUSsLKCrGU6U4Ml45zT/F+hKS+32wBg0gzoZ3S73VUtx3/IuQSLwJEpLMnp6M+1EVfDksSQzRove43aZKY= X-Received: by 2002:a24:2190:: with SMTP id e138-v6mr586879ita.71.1541604193463; Wed, 07 Nov 2018 07:23:13 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a6b:4f16:0:0:0:0:0 with HTTP; Wed, 7 Nov 2018 07:23:12 -0800 (PST) In-Reply-To: <9c5536f3-29c1-9cd5-3078-1596e2ce25ba@arm.com> References: <20181029045708.6292-1-ming.huang@linaro.org> <20181029045708.6292-2-ming.huang@linaro.org> <9c5536f3-29c1-9cd5-3078-1596e2ce25ba@arm.com> From: Ard Biesheuvel Date: Wed, 7 Nov 2018 16:23:12 +0100 Message-ID: To: Marc Zyngier Cc: Ming Huang , Leif Lindholm , linaro-uefi , "edk2-devel@lists.01.org" , Graeme Gregory , "Kinney, Michael D" , Laszlo Ersek , wanghuiqiang , huangming , Jason Zhang , huangdaode@hisilicon.com, John Garry , Xinliang Liu , zhangfeng56@huawei.com Subject: Re: [PATCH edk2/ArmPkg v1 1/1] ArmPkg: Fix Gic interrupt routing modes bug X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Nov 2018 15:23:14 -0000 Content-Type: text/plain; charset="UTF-8" On 7 November 2018 at 16:07, Marc Zyngier wrote: > On 07/11/18 14:48, Ard Biesheuvel wrote: >> (+ Marc) >> >> >> On 29 October 2018 at 05:57, Ming Huang wrote: >>> As GicV3 Spec, Interrupt Routing Modes should be 0 for >>> routing the SPIs to the primary CPU. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ming Huang >>> --- >>> ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> index 01154848f443..1558db31713a 100644 >>> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c >>> @@ -469,7 +469,7 @@ GicV3DxeInitialize ( >>> for (Index = 0; Index < (mGicNumInterrupts - 32); Index++) { >>> MmioWrite32 ( >>> mGicDistributorBase + ARM_GICD_IROUTER + (Index * 8), >>> - CpuTarget | ARM_GICD_IROUTER_IRM >>> + CpuTarget > > That's a very odd thing indeed. IRM being set means "broadcast to all > CPUs, ignoring the affinity", and there are a number of problems with that: > > - There is no guarantee that it is actually implemented > - Setting this bit *and* the affinity is like saying "yes" and "no" at > the same time > > So as far as I can see, this patch is correct, assuming that CpuTarget > is set to something sensible. What is a bit odd is the commit message, > which doesn't really explain the problem. How about something like: > > > Setting the GICD_IROUTERn.IMR and GICD_IROUTERn.{Aff3, Aff2, Aff1, Aff0} > at the same time is nonsensical (see 8.9.13 in the GICv3 spec, which > says of GICD_IROUTERn.IMR that "When this bit is set to 1, > GICD_IROUTER.{Aff3, Aff2, Aff1, Aff0} are UNKNOWN"). There is also no > guarantee that IMR is implemented (see GICD_TYPER.No1N which indicates > whether the implementation supports this or not). > > Let's thus not set this bit, as we want all SPIs to be delivered to the > same CPU, and not be broadcast to all of them. > > > Otherwise: > Acked-by: Marc Zyngier > Thanks Marc Pushed as 0adc6eae9480..b66e38b50134, with the commit log updated as per your suggestion