xen/arch/arm/arm64/vsysreg.c | 21 ++++++++++++++++++++- xen/arch/arm/include/asm/arm64/sysregs.h | 9 --------- xen/arch/arm/include/asm/sysregs.h | 9 +++++++++ xen/arch/arm/include/asm/vreg.h | 20 ++++++++++++++++++++ xen/arch/arm/vcpreg.c | 12 +++++++++++- 5 files changed, 60 insertions(+), 11 deletions(-)
Xen exposes ID_AA64PFR0_EL1.GIC and ID_PFR1.GIC from domain_cpuinfo,
which is initialized from the sanitized host CPU feature state. This
does not necessarily match the virtual interrupt controller configured
for a domain.
A vGICv2 domain can therefore observe a nonzero GIC field when the host
supports the GIC system register interface, even though Xen disables that
interface for the domain. On a GICv4.1-capable host, a vGICv3 domain can
observe encoding 0b0011, although Xen exposes only its vGICv3 model.
Derive the fields from the domain's vGIC version instead. Expose 0b0000
for vGICv2 and 0b0001 for vGICv3. This covers ID_AA64PFR0_EL1 and the
ID_PFR1_EL1 alias in AArch64 state, as well as ID_PFR1 accessed through
CP15 in AArch32 state. Leave the alias unchanged when AArch32 is
unavailable.
Fixes: 07b9acea116e ("xen/arm: Add handler for ID registers on arm64")
Fixes: 8f81064a07c6 ("xen/arm: Add handler for cp15 ID registers")
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v5:
- Fold the vGIC version mapping into the shared ID register helper.
- Use the vreg_ and VREG_ prefixes for the helper and field width.
- Cosmetic changes after review
Changes in v4:
- Explain why ID_PFR1_EL1 is left unchanged without AArch32 EL0 support.
- Move all ID_PFR1_*_SHIFT definitions to the common asm/sysregs.h and
remove the unused cpregs.h includes from the ARM64 files.
- Return explicit unsigned GIC field values to avoid MISRA C Rule 10.3.
Changes in v3:
- Add direct dependencies for the shared ID register helpers.
- Move ID_PFR1_GIC_SHIFT to the common CP15 register header.
- Apply cosmetic fixes from review.
Changes in v2:
- Share the GIC ID field helpers between the AArch64 and AArch32 paths.
- Parenthesize the individual ASSERT conditions.
- Preserve ID_PFR1_EL1.GIC when AArch32 is unavailable.
- Target master instead of the 4.22 release.
v1: https://patchew.org/Xen/ba4f779d68c54efc80c4a566dca38ac2e6f9a073.1783675708.git.mykola._5Fkvach@epam.com/
---
xen/arch/arm/arm64/vsysreg.c | 21 ++++++++++++++++++++-
xen/arch/arm/include/asm/arm64/sysregs.h | 9 ---------
xen/arch/arm/include/asm/sysregs.h | 9 +++++++++
xen/arch/arm/include/asm/vreg.h | 20 ++++++++++++++++++++
xen/arch/arm/vcpreg.c | 12 +++++++++++-
5 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index 66f4f23bb3..d8f380ccb7 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -299,7 +299,22 @@ void do_sysreg(struct cpu_user_regs *regs,
* to identify the processor features
*/
GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
- GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
+ case HSR_SYSREG_ID_PFR1_EL1:
+ {
+ register_t guest_reg_value = domain_cpuinfo.pfr32.bits[1];
+
+ /*
+ * Preserve the sanitized ID_PFR1_EL1 value when AArch32 EL0
+ * is not supported, as for the other AArch32 ID registers.
+ */
+ if ( cpu_feature64_has_el0_32(&domain_cpuinfo) )
+ guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
+ ID_PFR1_GIC_SHIFT,
+ v->domain);
+
+ return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
+ guest_reg_value);
+ }
GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
case HSR_SYSREG_ID_DFR0_EL1:
@@ -362,6 +377,10 @@ void do_sysreg(struct cpu_user_regs *regs,
guest_reg_value |= (sysval << ID_AA64PFR0_SVE_SHIFT) & mask;
}
+ guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
+ ID_AA64PFR0_GIC_SHIFT,
+ v->domain);
+
return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
guest_reg_value);
}
diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h
index f3c11d871e..f6ece8f972 100644
--- a/xen/arch/arm/include/asm/arm64/sysregs.h
+++ b/xen/arch/arm/include/asm/arm64/sysregs.h
@@ -438,15 +438,6 @@
#define MVFR1_FPDNAN_SHIFT 4
#define MVFR1_FPFTZ_SHIFT 0
-#define ID_PFR1_GIC_SHIFT 28
-#define ID_PFR1_VIRT_FRAC_SHIFT 24
-#define ID_PFR1_SEC_FRAC_SHIFT 20
-#define ID_PFR1_GENTIMER_SHIFT 16
-#define ID_PFR1_VIRTUALIZATION_SHIFT 12
-#define ID_PFR1_MPROGMOD_SHIFT 8
-#define ID_PFR1_SECURITY_SHIFT 4
-#define ID_PFR1_PROGMOD_SHIFT 0
-
#define MVFR2_FPMISC_SHIFT 4
#define MVFR2_SIMDMISC_SHIFT 0
diff --git a/xen/arch/arm/include/asm/sysregs.h b/xen/arch/arm/include/asm/sysregs.h
index f6af987ef5..8dcf82a694 100644
--- a/xen/arch/arm/include/asm/sysregs.h
+++ b/xen/arch/arm/include/asm/sysregs.h
@@ -9,6 +9,15 @@
# error "unknown ARM variant"
#endif
+#define ID_PFR1_GIC_SHIFT 28
+#define ID_PFR1_VIRT_FRAC_SHIFT 24
+#define ID_PFR1_SEC_FRAC_SHIFT 20
+#define ID_PFR1_GENTIMER_SHIFT 16
+#define ID_PFR1_VIRTUALIZATION_SHIFT 12
+#define ID_PFR1_MPROGMOD_SHIFT 8
+#define ID_PFR1_SECURITY_SHIFT 4
+#define ID_PFR1_PROGMOD_SHIFT 0
+
#ifndef __ASSEMBLER__
#include <asm/alternative.h>
diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
index 387ce76e7e..a85fefcc3e 100644
--- a/xen/arch/arm/include/asm/vreg.h
+++ b/xen/arch/arm/include/asm/vreg.h
@@ -4,11 +4,31 @@
#ifndef __ASM_ARM_VREG__
#define __ASM_ARM_VREG__
+#include <xen/bitops.h>
+#include <xen/bug.h>
+#include <xen/sched.h>
+
+#include <asm/gic.h>
+
typedef bool (*vreg_reg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
bool read);
typedef bool (*vreg_reg_fn_t)(struct cpu_user_regs *regs, register_t *r,
bool read);
+#define VREG_ID_REG_GIC_WIDTH 4
+
+static inline register_t vreg_id_reg_set_gic_field(register_t val,
+ unsigned int shift,
+ const struct domain *d)
+{
+ register_t mask = GENMASK(shift + VREG_ID_REG_GIC_WIDTH - 1, shift);
+ enum gic_version vgic_ver = d->arch.vgic.version;
+
+ ASSERT((vgic_ver == GIC_V2) || (vgic_ver == GIC_V3));
+
+ return (val & ~mask) | ((vgic_ver == GIC_V3 ? 1U : 0U) << shift);
+}
+
static inline bool vreg_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr,
vreg_reg_fn_t fn)
{
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index 3c315be9fd..15ccde71da 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -316,7 +316,17 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
* to identify the processor features
*/
GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
- GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
+ case HSR_CPREG32(ID_PFR1):
+ {
+ register_t guest_reg_value = domain_cpuinfo.pfr32.bits[1];
+
+ guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
+ ID_PFR1_GIC_SHIFT,
+ v->domain);
+
+ return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1,
+ guest_reg_value);
+ }
GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
case HSR_CPREG32(ID_DFR0):
--
2.53.0Hi Mykola,
Mykola Kvach <mykola_kvach@epam.com> writes:
> Xen exposes ID_AA64PFR0_EL1.GIC and ID_PFR1.GIC from domain_cpuinfo,
> which is initialized from the sanitized host CPU feature state. This
> does not necessarily match the virtual interrupt controller configured
> for a domain.
>
> A vGICv2 domain can therefore observe a nonzero GIC field when the host
> supports the GIC system register interface, even though Xen disables that
> interface for the domain. On a GICv4.1-capable host, a vGICv3 domain can
> observe encoding 0b0011, although Xen exposes only its vGICv3 model.
>
> Derive the fields from the domain's vGIC version instead. Expose 0b0000
> for vGICv2 and 0b0001 for vGICv3. This covers ID_AA64PFR0_EL1 and the
> ID_PFR1_EL1 alias in AArch64 state, as well as ID_PFR1 accessed through
> CP15 in AArch32 state. Leave the alias unchanged when AArch32 is
> unavailable.
>
> Fixes: 07b9acea116e ("xen/arm: Add handler for ID registers on arm64")
> Fixes: 8f81064a07c6 ("xen/arm: Add handler for cp15 ID registers")
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Changes in v5:
> - Fold the vGIC version mapping into the shared ID register helper.
> - Use the vreg_ and VREG_ prefixes for the helper and field width.
> - Cosmetic changes after review
>
> Changes in v4:
> - Explain why ID_PFR1_EL1 is left unchanged without AArch32 EL0 support.
> - Move all ID_PFR1_*_SHIFT definitions to the common asm/sysregs.h and
> remove the unused cpregs.h includes from the ARM64 files.
> - Return explicit unsigned GIC field values to avoid MISRA C Rule 10.3.
>
> Changes in v3:
> - Add direct dependencies for the shared ID register helpers.
> - Move ID_PFR1_GIC_SHIFT to the common CP15 register header.
> - Apply cosmetic fixes from review.
>
> Changes in v2:
> - Share the GIC ID field helpers between the AArch64 and AArch32 paths.
> - Parenthesize the individual ASSERT conditions.
> - Preserve ID_PFR1_EL1.GIC when AArch32 is unavailable.
> - Target master instead of the 4.22 release.
>
> v1: https://patchew.org/Xen/ba4f779d68c54efc80c4a566dca38ac2e6f9a073.1783675708.git.mykola._5Fkvach@epam.com/
> ---
> xen/arch/arm/arm64/vsysreg.c | 21 ++++++++++++++++++++-
> xen/arch/arm/include/asm/arm64/sysregs.h | 9 ---------
> xen/arch/arm/include/asm/sysregs.h | 9 +++++++++
> xen/arch/arm/include/asm/vreg.h | 20 ++++++++++++++++++++
> xen/arch/arm/vcpreg.c | 12 +++++++++++-
> 5 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index 66f4f23bb3..d8f380ccb7 100644
> --- a/xen/arch/arm/arm64/vsysreg.c
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -299,7 +299,22 @@ void do_sysreg(struct cpu_user_regs *regs,
> * to identify the processor features
> */
> GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
> - GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
> + case HSR_SYSREG_ID_PFR1_EL1:
> + {
> + register_t guest_reg_value = domain_cpuinfo.pfr32.bits[1];
> +
> + /*
> + * Preserve the sanitized ID_PFR1_EL1 value when AArch32 EL0
> + * is not supported, as for the other AArch32 ID registers.
> + */
> + if ( cpu_feature64_has_el0_32(&domain_cpuinfo) )
> + guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
> + ID_PFR1_GIC_SHIFT,
> + v->domain);
> +
> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
> + guest_reg_value);
> + }
> GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
>
> case HSR_SYSREG_ID_DFR0_EL1:
> @@ -362,6 +377,10 @@ void do_sysreg(struct cpu_user_regs *regs,
> guest_reg_value |= (sysval << ID_AA64PFR0_SVE_SHIFT) & mask;
> }
>
> + guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
> + ID_AA64PFR0_GIC_SHIFT,
> + v->domain);
> +
> return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
> guest_reg_value);
> }
> diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h
> index f3c11d871e..f6ece8f972 100644
> --- a/xen/arch/arm/include/asm/arm64/sysregs.h
> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h
> @@ -438,15 +438,6 @@
> #define MVFR1_FPDNAN_SHIFT 4
> #define MVFR1_FPFTZ_SHIFT 0
>
> -#define ID_PFR1_GIC_SHIFT 28
> -#define ID_PFR1_VIRT_FRAC_SHIFT 24
> -#define ID_PFR1_SEC_FRAC_SHIFT 20
> -#define ID_PFR1_GENTIMER_SHIFT 16
> -#define ID_PFR1_VIRTUALIZATION_SHIFT 12
> -#define ID_PFR1_MPROGMOD_SHIFT 8
> -#define ID_PFR1_SECURITY_SHIFT 4
> -#define ID_PFR1_PROGMOD_SHIFT 0
> -
> #define MVFR2_FPMISC_SHIFT 4
> #define MVFR2_SIMDMISC_SHIFT 0
>
> diff --git a/xen/arch/arm/include/asm/sysregs.h b/xen/arch/arm/include/asm/sysregs.h
> index f6af987ef5..8dcf82a694 100644
> --- a/xen/arch/arm/include/asm/sysregs.h
> +++ b/xen/arch/arm/include/asm/sysregs.h
> @@ -9,6 +9,15 @@
> # error "unknown ARM variant"
> #endif
>
> +#define ID_PFR1_GIC_SHIFT 28
> +#define ID_PFR1_VIRT_FRAC_SHIFT 24
> +#define ID_PFR1_SEC_FRAC_SHIFT 20
> +#define ID_PFR1_GENTIMER_SHIFT 16
> +#define ID_PFR1_VIRTUALIZATION_SHIFT 12
> +#define ID_PFR1_MPROGMOD_SHIFT 8
> +#define ID_PFR1_SECURITY_SHIFT 4
> +#define ID_PFR1_PROGMOD_SHIFT 0
> +
> #ifndef __ASSEMBLER__
>
> #include <asm/alternative.h>
> diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
> index 387ce76e7e..a85fefcc3e 100644
> --- a/xen/arch/arm/include/asm/vreg.h
> +++ b/xen/arch/arm/include/asm/vreg.h
> @@ -4,11 +4,31 @@
> #ifndef __ASM_ARM_VREG__
> #define __ASM_ARM_VREG__
>
> +#include <xen/bitops.h>
> +#include <xen/bug.h>
> +#include <xen/sched.h>
> +
> +#include <asm/gic.h>
> +
> typedef bool (*vreg_reg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
> bool read);
> typedef bool (*vreg_reg_fn_t)(struct cpu_user_regs *regs, register_t *r,
> bool read);
>
> +#define VREG_ID_REG_GIC_WIDTH 4
Should this go into sysregs.h?
Something like ID_AArchx_PFR_GIC_SHIFT? I am open to alternatives in naming.
> +
> +static inline register_t vreg_id_reg_set_gic_field(register_t val,
> + unsigned int shift,
> + const struct domain *d)
> +{
> + register_t mask = GENMASK(shift + VREG_ID_REG_GIC_WIDTH - 1, shift);
> + enum gic_version vgic_ver = d->arch.vgic.version;
> +
> + ASSERT((vgic_ver == GIC_V2) || (vgic_ver == GIC_V3));
> +
> + return (val & ~mask) | ((vgic_ver == GIC_V3 ? 1U : 0U) << shift);
Probably better to use constants?
Something like ID_AArchx_PFR_GIC_V3 and ID_AArchx_PFR_GIC_NO_CPU_INTF.
> +}
> +
> static inline bool vreg_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr,
> vreg_reg_fn_t fn)
> {
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index 3c315be9fd..15ccde71da 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -316,7 +316,17 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
> * to identify the processor features
> */
> GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
> - GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
> + case HSR_CPREG32(ID_PFR1):
> + {
> + register_t guest_reg_value = domain_cpuinfo.pfr32.bits[1];
> +
> + guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
> + ID_PFR1_GIC_SHIFT,
> + v->domain);
> +
> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1,
> + guest_reg_value);
> + }
> GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>
> case HSR_CPREG32(ID_DFR0):
--
WBR, Volodymyr
On 25-Sep-26 00:45, Volodymyr Babchuk wrote:
> Hi Mykola,
>
> Mykola Kvach <mykola_kvach@epam.com> writes:
>
>> Xen exposes ID_AA64PFR0_EL1.GIC and ID_PFR1.GIC from domain_cpuinfo,
>> which is initialized from the sanitized host CPU feature state. This
>> does not necessarily match the virtual interrupt controller configured
>> for a domain.
>>
>> A vGICv2 domain can therefore observe a nonzero GIC field when the host
>> supports the GIC system register interface, even though Xen disables that
>> interface for the domain. On a GICv4.1-capable host, a vGICv3 domain can
>> observe encoding 0b0011, although Xen exposes only its vGICv3 model.
>>
>> Derive the fields from the domain's vGIC version instead. Expose 0b0000
>> for vGICv2 and 0b0001 for vGICv3. This covers ID_AA64PFR0_EL1 and the
>> ID_PFR1_EL1 alias in AArch64 state, as well as ID_PFR1 accessed through
>> CP15 in AArch32 state. Leave the alias unchanged when AArch32 is
>> unavailable.
>>
>> Fixes: 07b9acea116e ("xen/arm: Add handler for ID registers on arm64")
>> Fixes: 8f81064a07c6 ("xen/arm: Add handler for cp15 ID registers")
>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>> Changes in v5:
>> - Fold the vGIC version mapping into the shared ID register helper.
>> - Use the vreg_ and VREG_ prefixes for the helper and field width.
>> - Cosmetic changes after review
>>
>> Changes in v4:
>> - Explain why ID_PFR1_EL1 is left unchanged without AArch32 EL0 support.
>> - Move all ID_PFR1_*_SHIFT definitions to the common asm/sysregs.h and
>> remove the unused cpregs.h includes from the ARM64 files.
>> - Return explicit unsigned GIC field values to avoid MISRA C Rule 10.3.
>>
>> Changes in v3:
>> - Add direct dependencies for the shared ID register helpers.
>> - Move ID_PFR1_GIC_SHIFT to the common CP15 register header.
>> - Apply cosmetic fixes from review.
>>
>> Changes in v2:
>> - Share the GIC ID field helpers between the AArch64 and AArch32 paths.
>> - Parenthesize the individual ASSERT conditions.
>> - Preserve ID_PFR1_EL1.GIC when AArch32 is unavailable.
>> - Target master instead of the 4.22 release.
>>
>> v1: https://patchew.org/Xen/ba4f779d68c54efc80c4a566dca38ac2e6f9a073.1783675708.git.mykola._5Fkvach@epam.com/
>> ---
>> xen/arch/arm/arm64/vsysreg.c | 21 ++++++++++++++++++++-
>> xen/arch/arm/include/asm/arm64/sysregs.h | 9 ---------
>> xen/arch/arm/include/asm/sysregs.h | 9 +++++++++
>> xen/arch/arm/include/asm/vreg.h | 20 ++++++++++++++++++++
>> xen/arch/arm/vcpreg.c | 12 +++++++++++-
>> 5 files changed, 60 insertions(+), 11 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>> index 66f4f23bb3..d8f380ccb7 100644
>> --- a/xen/arch/arm/arm64/vsysreg.c
>> +++ b/xen/arch/arm/arm64/vsysreg.c
>> @@ -299,7 +299,22 @@ void do_sysreg(struct cpu_user_regs *regs,
>> * to identify the processor features
>> */
>> GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
>> - GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
>> + case HSR_SYSREG_ID_PFR1_EL1:
>> + {
>> + register_t guest_reg_value = domain_cpuinfo.pfr32.bits[1];
>> +
>> + /*
>> + * Preserve the sanitized ID_PFR1_EL1 value when AArch32 EL0
>> + * is not supported, as for the other AArch32 ID registers.
>> + */
>> + if ( cpu_feature64_has_el0_32(&domain_cpuinfo) )
>> + guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
>> + ID_PFR1_GIC_SHIFT,
>> + v->domain);
>> +
>> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
>> + guest_reg_value);
>> + }
>> GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
>>
>> case HSR_SYSREG_ID_DFR0_EL1:
>> @@ -362,6 +377,10 @@ void do_sysreg(struct cpu_user_regs *regs,
>> guest_reg_value |= (sysval << ID_AA64PFR0_SVE_SHIFT) & mask;
>> }
>>
>> + guest_reg_value = vreg_id_reg_set_gic_field(guest_reg_value,
>> + ID_AA64PFR0_GIC_SHIFT,
>> + v->domain);
>> +
>> return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1,
>> guest_reg_value);
>> }
>> diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h
>> index f3c11d871e..f6ece8f972 100644
>> --- a/xen/arch/arm/include/asm/arm64/sysregs.h
>> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h
>> @@ -438,15 +438,6 @@
>> #define MVFR1_FPDNAN_SHIFT 4
>> #define MVFR1_FPFTZ_SHIFT 0
>>
>> -#define ID_PFR1_GIC_SHIFT 28
>> -#define ID_PFR1_VIRT_FRAC_SHIFT 24
>> -#define ID_PFR1_SEC_FRAC_SHIFT 20
>> -#define ID_PFR1_GENTIMER_SHIFT 16
>> -#define ID_PFR1_VIRTUALIZATION_SHIFT 12
>> -#define ID_PFR1_MPROGMOD_SHIFT 8
>> -#define ID_PFR1_SECURITY_SHIFT 4
>> -#define ID_PFR1_PROGMOD_SHIFT 0
>> -
>> #define MVFR2_FPMISC_SHIFT 4
>> #define MVFR2_SIMDMISC_SHIFT 0
>>
>> diff --git a/xen/arch/arm/include/asm/sysregs.h b/xen/arch/arm/include/asm/sysregs.h
>> index f6af987ef5..8dcf82a694 100644
>> --- a/xen/arch/arm/include/asm/sysregs.h
>> +++ b/xen/arch/arm/include/asm/sysregs.h
>> @@ -9,6 +9,15 @@
>> # error "unknown ARM variant"
>> #endif
>>
>> +#define ID_PFR1_GIC_SHIFT 28
>> +#define ID_PFR1_VIRT_FRAC_SHIFT 24
>> +#define ID_PFR1_SEC_FRAC_SHIFT 20
>> +#define ID_PFR1_GENTIMER_SHIFT 16
>> +#define ID_PFR1_VIRTUALIZATION_SHIFT 12
>> +#define ID_PFR1_MPROGMOD_SHIFT 8
>> +#define ID_PFR1_SECURITY_SHIFT 4
>> +#define ID_PFR1_PROGMOD_SHIFT 0
>> +
>> #ifndef __ASSEMBLER__
>>
>> #include <asm/alternative.h>
>> diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
>> index 387ce76e7e..a85fefcc3e 100644
>> --- a/xen/arch/arm/include/asm/vreg.h
>> +++ b/xen/arch/arm/include/asm/vreg.h
>> @@ -4,11 +4,31 @@
>> #ifndef __ASM_ARM_VREG__
>> #define __ASM_ARM_VREG__
>>
>> +#include <xen/bitops.h>
>> +#include <xen/bug.h>
>> +#include <xen/sched.h>
>> +
>> +#include <asm/gic.h>
>> +
>> typedef bool (*vreg_reg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
>> bool read);
>> typedef bool (*vreg_reg_fn_t)(struct cpu_user_regs *regs, register_t *r,
>> bool read);
>>
>> +#define VREG_ID_REG_GIC_WIDTH 4
>
> Should this go into sysregs.h?
>
> Something like ID_AArchx_PFR_GIC_SHIFT? I am open to alternatives in naming.
I don't think that's necessary, given that we don't have any _WIDTH macros in
there and other places hard code widths (e.g. SVE). Especially that we are now
at v5 and we should avoid subjective NITs.
>
>> +
>> +static inline register_t vreg_id_reg_set_gic_field(register_t val,
>> + unsigned int shift,
>> + const struct domain *d)
>> +{
>> + register_t mask = GENMASK(shift + VREG_ID_REG_GIC_WIDTH - 1, shift);
>> + enum gic_version vgic_ver = d->arch.vgic.version;
>> +
>> + ASSERT((vgic_ver == GIC_V2) || (vgic_ver == GIC_V3));
>> +
>> + return (val & ~mask) | ((vgic_ver == GIC_V3 ? 1U : 0U) << shift);
>
> Probably better to use constants?
>
> Something like ID_AArchx_PFR_GIC_V3 and ID_AArchx_PFR_GIC_NO_CPU_INTF.
This suggestion is valid. Please add in common sysregs.h:
/* GIC field encodings, common to ID_PFR1{,_EL1} and ID_AA64PFR0_EL1 */
#define ID_PFR_GIC_NI 0x0
#define ID_PFR_GIC_V3 0x1
You can retain my Rb.
~Michal
© 2016 - 2026 Red Hat, Inc.