From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web08.15543.1606461452025519864 for ; Thu, 26 Nov 2020 23:17:32 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BB05531B; Thu, 26 Nov 2020 23:17:30 -0800 (PST) Received: from [192.168.1.81] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 64E753F70D; Thu, 26 Nov 2020 23:17:29 -0800 (PST) Subject: Re: [PATCH v1 1/1] ArmPkg/ArmGicDxe: fix writes to GICD_IPRIORITYR when ARE enable To: Quan Nguyen , Leif Lindholm , devel@edk2.groups.io Cc: Open Source Review , Victor Gallardo References: <20201028012112.12125-1-quan@os.amperecomputing.com> From: "Ard Biesheuvel" Message-ID: Date: Fri, 27 Nov 2020 08:17:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201028012112.12125-1-quan@os.amperecomputing.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/28/20 2:21 AM, Quan Nguyen wrote: > According to ARM doc IHI 0069F, section 11.9.18, "Accessing the > GICD_IPRIORITYR:", "These registers are always used when affinity > routing is not enabled. When affinity routing is enabled for the > Security state of an interrupt: > * GICR_IPRIORITYR is used instead of GICD_IPRIORITYR where n = 0 > to 7 (that is, for SGIs and PPIs)." > > The current ArmGicV3 code tries to initialize all the IPRIORITYR > registers to a default state via the Distributor (GICD), so skip > writes to the first eight IPRIORITYR registers when Affinity Routing > is Enabled. > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Signed-off-by: Victor Gallardo > Signed-off-by: Quan Nguyen Isn't this already being taken into account in ArmGicDisableInterrupt()? > --- > ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > index d7da1f198d9e..bc543502481b 100644 > --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c > @@ -378,6 +378,7 @@ GicV3DxeInitialize ( > UINTN RegShift; > UINT64 CpuTarget; > UINT64 MpId; > + BOOLEAN AffinityRoutingEnabled = FALSE; > > // Make sure the Interrupt Controller Protocol is not already installed in > // the system. > @@ -391,11 +392,21 @@ GicV3DxeInitialize ( > // Routing enabled. So ensure that the ARE bit is set. > if (!FeaturePcdGet (PcdArmGicV3WithV2Legacy)) { > MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE); > + // If Affinity Routing is Enabled, the first 32 interrupts (SGI and PPI) > + // can be programmed only through Redistributor interface (GICR). > + // Initializing the GICD_IPRIORITYR registers for these interrupts can be > + // skipped as the Redistributor will be powered up and initialized > + // at the appropriate time (e.g. in EL3 by trusted firmware). > + AffinityRoutingEnabled = TRUE; > } > > for (Index = 0; Index < mGicNumInterrupts; Index++) { > GicV3DisableInterruptSource (&gHardwareInterruptV3Protocol, Index); > > + if (AffinityRoutingEnabled && Index < 32) { > + continue; > + } > + > // Set Priority > RegOffset = Index / 4; > RegShift = (Index % 4) * 8; >