From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out4436.biz.mail.alibaba.com (out4436.biz.mail.alibaba.com [47.88.44.36]) by mx.groups.io with SMTP id smtpd.web12.4364.1614045881400353594 for ; Mon, 22 Feb 2021 18:04:41 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: linux.alibaba.com, ip: 47.88.44.36, mailfrom: huangming@linux.alibaba.com) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R501e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04394;MF=huangming@linux.alibaba.com;NM=1;PH=DS;RN=4;SR=0;TI=SMTPD_---0UPJpN3l_1614045867; Received: from MingdeMacBook-Pro.local(mailfrom:huangming@linux.alibaba.com fp:SMTPD_---0UPJpN3l_1614045867) by smtp.aliyun-inc.com(127.0.0.1); Tue, 23 Feb 2021 10:04:27 +0800 Subject: Re: [PATCH edk2 v1 1/1] ArmPkg/ArmGicLib: Fix GICR_IPRIORITYR address wrong issue To: Leif Lindholm Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, guoheyi@linux.alibaba.com References: <20210220070839.29988-1-huangming@linux.alibaba.com> <20210222125210.GS1664@vanye> From: "Ming Huang" Message-ID: <2de9d872-eb6c-3856-e05f-562a23bad72c@linux.alibaba.com> Date: Tue, 23 Feb 2021 10:04:27 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <20210222125210.GS1664@vanye> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 2/22/21 8:52 PM, Leif Lindholm wrote: > Hi Ming, > > On Sat, Feb 20, 2021 at 15:08:39 +0800, Ming Huang wrote: >> The address of GICR_IPRIORITYR is in SGI_base frame. ARM_GICR_CTLR_FRAME_SIZE >> should add to GicCpuRedistributorBase for GICR_IPRIORITYR. Otherwise RAS >> error(Uncorrected software error) will reported in ArmGicDxe. >> >> Signed-off-by: Ming Huang >> --- >> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c >> index 8ef32b33a1..7a54972455 100644 >> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c >> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c >> @@ -235,6 +235,9 @@ ArmGicSetInterruptPriority ( >> return; >> } >> >> + // The address of GICR_IPRIORITYR is in SGI_base frame. >> + // ARM_GICR_CTLR_FRAME_SIZE should add to GicCpuRedistributorBase for GICR_IPRIORITYR. >> + GicCpuRedistributorBase += ARM_GICR_CTLR_FRAME_SIZE; > > I agree with the error report, but not the fix. > Changing the value of a variable called GicCpuRedistributorBase to > something that is not the base address of the redistributor makes the > code confusing. > > If you look at the subsequent function, ArmGicEnableInterrupt, it > resolvess the same situation using the ISENABLER_ADDRESS macro > defined at the top of the file: > > #define ISENABLER_ADDRESS(base,offset) ((base) + \ > ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * offset)) > > I would suggest creating an IPRIORITY_ADDRESS macro by the same > pattern and using that. > > Would that solution be OK with you? Good idea. Thanks for your solution. Modify it in v2. Thanks, Ming > > Best Regards, > > Leif > >> MmioAndThenOr32 ( >> GicCpuRedistributorBase + ARM_GIC_ICDIPR + (4 * RegOffset), >> ~(0xff << RegShift), >> -- >> 2.17.1 >>