Hi Ayan,
On 05/03/2026 19:43, Ayan Kumar Halder wrote:
Set GICV3_NR_LRS as per the number of list registers in the supported
hardware. This ensures:
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. Similarly In gicv3_ich_read_lr()/gicv3_ich_write_lr() , we can
justify that the unsupported LRs (4-15) will never be reached as Xen
will panic if the runtime value (lr) exceeds GICV3_NR_LRS. Some
compiler can eliminate the code accessing LR 4-15.
In this situation, using panic() is better than accessing a list
register which is not present in the hardware
3. Whenever GICV3_NR_LRS is defined, the default condition and the
related BUG() cannot be reached at all.
I am not sure how this is better. You will still crash Xen is 'lr' >=
GICV3_NR_LRS. Can you provide some details?
> > As part of functional safety effort, we are trying to enable system
integrator to configure Xen for a specific platform with a predefind
set of GICv3 list registers. So that we can minimize the chance of
runtime issues and reduce the codesize that will execute.
Signed-off-by: Ayan Kumar Halder <[email protected]>
Signed-off-by: Michal Orzel <[email protected]>
---
xen/arch/arm/Kconfig | 9 +++++++++
xen/arch/arm/gic-v3.c | 12 ++++++++++--
2 files changed, 19 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
+ 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..fb2985fd52 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)
We should avoid lowercase define, in particular with generic names like
'lrs'. I think in this case, I would rather update gicv3_info.nr_lrs:
gicv3_info.nr_lrs = min(gv3_info.nr_lrs, CONFIG_GICV3_NR_LRS);
This would solve another problem where you don't sanity check that the
system effectively support CONFIG_GICV3_NR_LRS.
@@ -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,9 @@ static inline void gicv3_restore_lrs(const struct vcpu *v)
static uint64_t gicv3_ich_read_lr(int lr)
{
+ if ( lr >= lrs )
+ panic("Unsupported number of LRs\n");
Do we really have to panic in production build? Wouldn't it be better to
return '0' and maybe use ASSERT for a crash in debug build? Same below.
+
switch ( lr )
{
case 0: return READ_SYSREG_LR(0);
@@ -203,6 +208,9 @@ static uint64_t gicv3_ich_read_lr(int lr)
static void gicv3_ich_write_lr(int lr, uint64_t val)
{
+ if ( lr >= lrs )
+ panic("Unsupported number of LRs\n");
+
switch ( lr )
{
case 0:
Cheers,
--
Julien Grall
