Re: [edk2-devel] [PATCH edk2 v2 1/1] ArmPkg/ArmGic: Fix GICR_IPRIORITYR address wrong issue

["Leif Lindholm" <leif@nuviainc.com>]

Hi Ming,

Thanks for this.
However, just after I sent my reply, Heyi suggested another
improvement to this patch. Can you please implement that, and submit a
v2?

Best Regards,

Leif

On Wed, Feb 24, 2021 at 16:26:33 +0800, Ming Huang wrote:
> 
> 
> On 2/23/21 8:58 PM, Leif Lindholm wrote:
> > On Tue, Feb 23, 2021 at 13:42:49 +0100, Ard Biesheuvel wrote:
> >> On Tue, 23 Feb 2021 at 12:07, Ming Huang <huangming@linux.alibaba.com> wrote:
> >>>
> >>> The register address of GICR_IPRIORITYR is in SGI_base frame. Add
> >>> IPRIORITY_ADDRESS macro for getting GICR_IPRIORITYR address. Otherwise
> >>> GIC RAS error(Uncorrected software error) may report in ArmGicDxe.
> >>
> >> NOTE: missing sign-off
> >>
> >> Patch seems fine to me
> >>
> >> Tested-by: Ard Biesheuvel <ardb@kernel.org> # QEMU/kvm guest on ThunderX2
> > 
> > Also looks good in QEMU.
> > Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> > 
> > Ming, can you confirm that the contribution conforms to the developer
> > certificate of origin by replyint with your Signed-off-by: ?
> 
> Sorry, I miss the Signed-off-by.
> please help to add my Signed-off-by:
> Signed-off-by: Ming Huang <huangming@linux.alibaba.com>
> 
> Thanks,
> Ming
> 
> > 
> >>
> >>
> >>
> >>> ---
> >>>  ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
> >>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> >>> index 8ef32b33a1..b4d3965acb 100644
> >>> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> >>> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> >>> @@ -30,6 +30,9 @@
> >>>  #define ICENABLER_ADDRESS(base,offset) ((base) + \
> >>>            ARM_GICR_CTLR_FRAME_SIZE +  ARM_GICR_ICENABLER + (4 * offset))
> >>>
> >>> +#define IPRIORITY_ADDRESS(base,offset) ((base) + \
> >>> +          ARM_GICR_CTLR_FRAME_SIZE +  ARM_GIC_ICDIPR + (4 * offset))
> >>> +
> >>>  /**
> >>>   *
> >>>   * Return whether the Source interrupt index refers to a shared interrupt (SPI)
> >>> @@ -236,7 +239,7 @@ ArmGicSetInterruptPriority (
> >>>      }
> >>>
> >>>      MmioAndThenOr32 (
> >>> -      GicCpuRedistributorBase + ARM_GIC_ICDIPR + (4 * RegOffset),
> >>> +      IPRIORITY_ADDRESS (GicCpuRedistributorBase, RegOffset),
> >>>        ~(0xff << RegShift),
> >>>        Priority << RegShift
> >>>        );
> >>> --
> >>> 2.17.1
> >>>
> 
> 
> 
> 
>