Re: [PATCH v2] xen: gic-v3: Introduce CONFIG_GICV3_NR_LRS

["Halder, Ayan Kumar" <ayankuma@xxxxxxx>]

On 08/04/2026 15:24, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
> > On 18 Mar 2026, at 14:09, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> >
> > One key requirement of Xen functional safety is to reduce the number
> > of lines of code to be safety certified. Besides, a safety certified
> > Xen requires a static hardware configuration to be defined. This static
> > hardware configuration is described as per the test hardware/emulator
> > hardware configuration against which Xen is verified.
> >
> > Introduce GICV3_NR_LRS with the two aims in mind:
> > 1. User should set the number of GICV3 list registers as per the test
> > hardware so that the unwanted code can be removed using GCC's dead
> > code elimination or preprocessor's config.
> > 2. By doing #1, one can ensure that there is no untested code due to
> > unsupported hardware platform and thus there is no safety impact due
> > to untested code.
> >
> > However if the user does not set GICV3_NR_LRS, then it is set to 0.
> > Thus Xen will fallback to the default scenario (i.e. read the hardware
> > register to determine the number of LRS).
> >
> > 1. In gicv3_save_lrs()/gicv3_restore_lrs(), use the number of list
> > registers from GICV3_NR_LRS (if defined) instead of gicv3_info.nr_lrs.
> > This ensures that if the hardware does not support more than 4 LRs
> > (for example), the code accessing LR 4-15 is never reached. The
> > compiler can eliminate the unsupported cases as the switch case uses a
> > constant conditional.
> >
> > 2. RAZ/WI for the unsupported LRs.
> >
> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> > ---
> > Changelog:
> >
> > v1 - 1. s/lrs/LRS
> > 2. Implement RAZ/WI instead of panic
> >
> > Few comments which were not addressed
> > 1. Do "gicv3_info.nr_lrs to LRS" in gicv3_hyp_init() and keep the code
> > unchanged in gicv3_save_lrs()/gicv3_restore_lrs() -- This prevents the
> > compiler from doing dead code elimination as the switch condition cannot
> > be evaluated at compile time.
> > I am not sure how to get around this issue.
> >
> > xen/arch/arm/Kconfig  |  9 +++++++++
> > xen/arch/arm/gic-v3.c | 14 ++++++++++++--
> > 2 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 2f2b501fda..6540013f97 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -276,6 +276,15 @@ config PCI_PASSTHROUGH
> >
> > endmenu
> >
> > +config GICV3_NR_LRS
> > + int "Number of GICv3 Link Registers supported" if EXPERT
> > + depends on GICV3
> > + range 0 16
> 16 is the maximum supported since ICH_VTR_EL2.ListRegs is 4 bits [1],
> however how are we handling the case when GICV3_NR_LRS is greater
> than the supported number of LR registers?
>
> Shall we check that in gicv3_hyp_init()?
My intention is that when user sets GICV3_NR_LRS , it overwrites the 
value obtained from reading the hardware. IOW, I was thinking of 
something like
gicv3_info.nr_lrs = GICV3_NR_LRS

But you have a valid point that what will happen if the user sets 
GICV3_NR_LRS to value greater than that supported by the hardware. I 
think we should print a warning in that case. This was suggested by 
Julien in v1, but I missed that.
> > + default 0
> > + help
> > +  Controls the number of Link registers to be accessed.
> > +  Keep it set to 0 to use a value obtained from a hardware register.
> > +
> > menu "ARM errata workaround via the alternative framework"
> > depends on HAS_ALTERNATIVE
> >
> > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> > index bc07f97c16..eaae95eb4d 100644
> > --- a/xen/arch/arm/gic-v3.c
> > +++ b/xen/arch/arm/gic-v3.c
> > @@ -51,6 +51,8 @@ static DEFINE_PER_CPU(void __iomem*, rbase);
> > #define GICD                   (gicv3.map_dbase)
> > #define GICD_RDIST_BASE        (this_cpu(rbase))
> > #define GICD_RDIST_SGI_BASE    (GICD_RDIST_BASE + SZ_64K)
> > +#define LRS                    (CONFIG_GICV3_NR_LRS ?: \
> > +                                gicv3_info.nr_lrs)
> >
> > /*
> >   * Saves all 16(Max) LR registers. Though number of LRs implemented
> > @@ -59,7 +61,7 @@ static DEFINE_PER_CPU(void __iomem*, rbase);
> > static inline void gicv3_save_lrs(struct vcpu *v)
> > {
> >      /* Fall through for all the cases */
> > -    switch ( gicv3_info.nr_lrs )
> > +    switch ( LRS )
> >      {
> >      case 16:
> >          v->arch.gic.v3.lr[15] = READ_SYSREG_LR(15);
> > @@ -121,7 +123,7 @@ static inline void gicv3_save_lrs(struct vcpu *v)
> > static inline void gicv3_restore_lrs(const struct vcpu *v)
> > {
> >      /* Fall through for all the cases */
> > -    switch ( gicv3_info.nr_lrs )
> > +    switch ( LRS )
> >      {
> >      case 16:
> >          WRITE_SYSREG_LR(v->arch.gic.v3.lr[15], 15);
> > @@ -178,6 +180,10 @@ static inline void gicv3_restore_lrs(const struct vcpu *v)
> >
> > static uint64_t gicv3_ich_read_lr(int lr)
> > {
> > +    /* RAZ for unsupported LR */
> > +    if ( lr >= LRS )
> > +        return 0;
> > +
> >      switch ( lr )
> >      {
> >      case 0: return READ_SYSREG_LR(0);
> > @@ -203,6 +209,10 @@ static uint64_t gicv3_ich_read_lr(int lr)
> >
> > static void gicv3_ich_write_lr(int lr, uint64_t val)
> > {
> > +    /* WI for unsupported LR */
> > +    if ( lr >= LRS )
> > +        return;
> > +
> >      switch ( lr )
> >      {
> >      case 0:
> Now, since we are using CONFIG_GICV3_NR_LRS or gicv3_info.nr_lrs in 
> gicv3_save_lrs/gicv3_restore_lrs,
> there are other part of the codebase using nr_lrs (gic_get_nr_lrs() is one of 
> them), but all the callers of that
> function will use the HW nr_lrs and not the CONFIG_GICV3_NR_LRS, so I think 
> some work needs to be done
> to align them or there will be mismatches at runtime with possible loss of 
> information.
I will fix that.

- Ayan

> [1] 
> https://developer.arm.com/documentation/111179/2025-09_ASL1/AArch64-Registers/ICH-VTR-EL2--Interrupt-Controller-VGIC-Type-Register
>
> Cheers,
> Luca