| Message ID | 539cc9c817a80e35a2532dba5bc01e9b2533ff56.1606742184.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Emulate ID registers | expand |
Bertrand Marquis writes: > Create a cpuinfo structure for guest and mask into it the features that > we do not support in Xen or that we do not want to publish to guests. > > Modify some values in the cpuinfo structure for guests to mask some > features which we do not want to allow to guests (like AMU) or we do not > support (like SVE). > > The code is trying to group together registers modifications for the > same feature to be able in the long term to easily enable/disable a > feature depending on user parameters or add other registers modification > in the same place (like enabling/disabling HCR bits). > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > --- > Changes in V2: rebase > --- > xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++ > xen/include/asm-arm/cpufeature.h | 2 ++ > 2 files changed, 53 insertions(+) > > diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c > index 204be9b084..309941ff37 100644 > --- a/xen/arch/arm/cpufeature.c > +++ b/xen/arch/arm/cpufeature.c > @@ -24,6 +24,8 @@ > > DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); > > +struct cpuinfo_arm __read_mostly guest_cpuinfo; > + > void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, > const char *info) > { > @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c) > #endif > } > > +/* > + * This function is creating a cpuinfo structure with values modified to mask > + * all cpu features that should not be published to guest. > + * The created structure is then used to provide ID registers values to guests. > + */ > +static int __init create_guest_cpuinfo(void) > +{ > + /* > + * TODO: The code is currently using only the features detected on the boot > + * core. In the long term we should try to compute values containing only > + * features supported by all cores. > + */ > + identify_cpu(&guest_cpuinfo); > + > +#ifdef CONFIG_ARM_64 > + /* Disable MPAM as xen does not support it */ > + guest_cpuinfo.pfr64.mpam = 0; > + guest_cpuinfo.pfr64.mpam_frac = 0; > + > + /* Disable SVE as Xen does not support it */ > + guest_cpuinfo.pfr64.sve = 0; > + guest_cpuinfo.zfr64.bits[0] = 0; > + > + /* Disable MTE as Xen does not support it */ > + guest_cpuinfo.pfr64.mte = 0; > +#endif > + > + /* Disable AMU */ > +#ifdef CONFIG_ARM_64 > + guest_cpuinfo.pfr64.amu = 0; > +#endif > + guest_cpuinfo.pfr32.amu = 0; > + > + /* Disable RAS as Xen does not support it */ > +#ifdef CONFIG_ARM_64 > + guest_cpuinfo.pfr64.ras = 0; > + guest_cpuinfo.pfr64.ras_frac = 0; > +#endif > + guest_cpuinfo.pfr32.ras = 0; > + guest_cpuinfo.pfr32.ras_frac = 0; > + > + return 0; > +} > +/* > + * This function needs to be run after all smp are started to have > + * cpuinfo structures for all cores. > + */ This comment contradicts with TODO at the beginning of create_guest_cpuinfo(). > +__initcall(create_guest_cpuinfo); > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h > index 64354c3f19..0ab6dd42a0 100644 > --- a/xen/include/asm-arm/cpufeature.h > +++ b/xen/include/asm-arm/cpufeature.h > @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *); > extern struct cpuinfo_arm cpu_data[]; > #define current_cpu_data cpu_data[smp_processor_id()] > > +extern struct cpuinfo_arm guest_cpuinfo; > + > #endif /* __ASSEMBLY__ */ > > #endif
Hi Volodymyr, > On 30 Nov 2020, at 20:15, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote: > > > Bertrand Marquis writes: > >> Create a cpuinfo structure for guest and mask into it the features that >> we do not support in Xen or that we do not want to publish to guests. >> >> Modify some values in the cpuinfo structure for guests to mask some >> features which we do not want to allow to guests (like AMU) or we do not >> support (like SVE). >> >> The code is trying to group together registers modifications for the >> same feature to be able in the long term to easily enable/disable a >> feature depending on user parameters or add other registers modification >> in the same place (like enabling/disabling HCR bits). >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> --- >> Changes in V2: rebase >> --- >> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++ >> xen/include/asm-arm/cpufeature.h | 2 ++ >> 2 files changed, 53 insertions(+) >> >> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c >> index 204be9b084..309941ff37 100644 >> --- a/xen/arch/arm/cpufeature.c >> +++ b/xen/arch/arm/cpufeature.c >> @@ -24,6 +24,8 @@ >> >> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); >> >> +struct cpuinfo_arm __read_mostly guest_cpuinfo; >> + >> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, >> const char *info) >> { >> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c) >> #endif >> } >> >> +/* >> + * This function is creating a cpuinfo structure with values modified to mask >> + * all cpu features that should not be published to guest. >> + * The created structure is then used to provide ID registers values to guests. >> + */ >> +static int __init create_guest_cpuinfo(void) >> +{ >> + /* >> + * TODO: The code is currently using only the features detected on the boot >> + * core. In the long term we should try to compute values containing only >> + * features supported by all cores. >> + */ >> + identify_cpu(&guest_cpuinfo); >> + >> +#ifdef CONFIG_ARM_64 >> + /* Disable MPAM as xen does not support it */ >> + guest_cpuinfo.pfr64.mpam = 0; >> + guest_cpuinfo.pfr64.mpam_frac = 0; >> + >> + /* Disable SVE as Xen does not support it */ >> + guest_cpuinfo.pfr64.sve = 0; >> + guest_cpuinfo.zfr64.bits[0] = 0; >> + >> + /* Disable MTE as Xen does not support it */ >> + guest_cpuinfo.pfr64.mte = 0; >> +#endif >> + >> + /* Disable AMU */ >> +#ifdef CONFIG_ARM_64 >> + guest_cpuinfo.pfr64.amu = 0; >> +#endif >> + guest_cpuinfo.pfr32.amu = 0; >> + >> + /* Disable RAS as Xen does not support it */ >> +#ifdef CONFIG_ARM_64 >> + guest_cpuinfo.pfr64.ras = 0; >> + guest_cpuinfo.pfr64.ras_frac = 0; >> +#endif >> + guest_cpuinfo.pfr32.ras = 0; >> + guest_cpuinfo.pfr32.ras_frac = 0; >> + >> + return 0; >> +} >> +/* >> + * This function needs to be run after all smp are started to have >> + * cpuinfo structures for all cores. >> + */ > > This comment contradicts with TODO at the beginning of > create_guest_cpuinfo(). > I think the comment is coherent as it is a prerequisite to solve the TODO. I made it this way so that nothing would need to be modified there to handle the TODO. So I do not really see a contradiction there, what would you suggest to say instead ? Regards Bertrand >> +__initcall(create_guest_cpuinfo); >> + >> /* >> * Local variables: >> * mode: C >> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h >> index 64354c3f19..0ab6dd42a0 100644 >> --- a/xen/include/asm-arm/cpufeature.h >> +++ b/xen/include/asm-arm/cpufeature.h >> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *); >> extern struct cpuinfo_arm cpu_data[]; >> #define current_cpu_data cpu_data[smp_processor_id()] >> >> +extern struct cpuinfo_arm guest_cpuinfo; >> + >> #endif /* __ASSEMBLY__ */ >> >> #endif > > > -- > Volodymyr Babchuk at EPAM
On Mon, 30 Nov 2020, Bertrand Marquis wrote: > Create a cpuinfo structure for guest and mask into it the features that > we do not support in Xen or that we do not want to publish to guests. > > Modify some values in the cpuinfo structure for guests to mask some > features which we do not want to allow to guests (like AMU) or we do not > support (like SVE). The first two sentences seem to say the same thing in two different ways. > The code is trying to group together registers modifications for the > same feature to be able in the long term to easily enable/disable a > feature depending on user parameters or add other registers modification > in the same place (like enabling/disabling HCR bits). > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > --- > Changes in V2: rebase > --- > xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++ > xen/include/asm-arm/cpufeature.h | 2 ++ > 2 files changed, 53 insertions(+) > > diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c > index 204be9b084..309941ff37 100644 > --- a/xen/arch/arm/cpufeature.c > +++ b/xen/arch/arm/cpufeature.c > @@ -24,6 +24,8 @@ > > DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); > > +struct cpuinfo_arm __read_mostly guest_cpuinfo; > + > void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, > const char *info) > { > @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c) > #endif > } > > +/* > + * This function is creating a cpuinfo structure with values modified to mask > + * all cpu features that should not be published to guest. > + * The created structure is then used to provide ID registers values to guests. > + */ > +static int __init create_guest_cpuinfo(void) > +{ > + /* > + * TODO: The code is currently using only the features detected on the boot > + * core. In the long term we should try to compute values containing only > + * features supported by all cores. > + */ > + identify_cpu(&guest_cpuinfo); Given that we already have boot_cpu_data and current_cpu_data, which should be already initialized at this point, we could simply: guest_cpuinfo = current_cpu_data; or guest_cpuinfo = boot_cpu_data; ? > +#ifdef CONFIG_ARM_64 > + /* Disable MPAM as xen does not support it */ > + guest_cpuinfo.pfr64.mpam = 0; > + guest_cpuinfo.pfr64.mpam_frac = 0; > + > + /* Disable SVE as Xen does not support it */ > + guest_cpuinfo.pfr64.sve = 0; > + guest_cpuinfo.zfr64.bits[0] = 0; > + > + /* Disable MTE as Xen does not support it */ > + guest_cpuinfo.pfr64.mte = 0; > +#endif > + > + /* Disable AMU */ > +#ifdef CONFIG_ARM_64 > + guest_cpuinfo.pfr64.amu = 0; > +#endif > + guest_cpuinfo.pfr32.amu = 0; > + > + /* Disable RAS as Xen does not support it */ > +#ifdef CONFIG_ARM_64 > + guest_cpuinfo.pfr64.ras = 0; > + guest_cpuinfo.pfr64.ras_frac = 0; > +#endif > + guest_cpuinfo.pfr32.ras = 0; > + guest_cpuinfo.pfr32.ras_frac = 0; > + > + return 0; > +} > +/* > + * This function needs to be run after all smp are started to have > + * cpuinfo structures for all cores. > + */ > +__initcall(create_guest_cpuinfo); > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h > index 64354c3f19..0ab6dd42a0 100644 > --- a/xen/include/asm-arm/cpufeature.h > +++ b/xen/include/asm-arm/cpufeature.h > @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *); > extern struct cpuinfo_arm cpu_data[]; > #define current_cpu_data cpu_data[smp_processor_id()] > > +extern struct cpuinfo_arm guest_cpuinfo; > + > #endif /* __ASSEMBLY__ */ > > #endif > -- > 2.17.1 >
Hi Stefano, > On 4 Dec 2020, at 23:57, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Mon, 30 Nov 2020, Bertrand Marquis wrote: >> Create a cpuinfo structure for guest and mask into it the features that >> we do not support in Xen or that we do not want to publish to guests. >> >> Modify some values in the cpuinfo structure for guests to mask some >> features which we do not want to allow to guests (like AMU) or we do not >> support (like SVE). > > The first two sentences seem to say the same thing in two different > ways. > > >> The code is trying to group together registers modifications for the >> same feature to be able in the long term to easily enable/disable a >> feature depending on user parameters or add other registers modification >> in the same place (like enabling/disabling HCR bits). >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> --- >> Changes in V2: rebase >> --- >> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++ >> xen/include/asm-arm/cpufeature.h | 2 ++ >> 2 files changed, 53 insertions(+) >> >> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c >> index 204be9b084..309941ff37 100644 >> --- a/xen/arch/arm/cpufeature.c >> +++ b/xen/arch/arm/cpufeature.c >> @@ -24,6 +24,8 @@ >> >> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); >> >> +struct cpuinfo_arm __read_mostly guest_cpuinfo; >> + >> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, >> const char *info) >> { >> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c) >> #endif >> } >> >> +/* >> + * This function is creating a cpuinfo structure with values modified to mask >> + * all cpu features that should not be published to guest. >> + * The created structure is then used to provide ID registers values to guests. >> + */ >> +static int __init create_guest_cpuinfo(void) >> +{ >> + /* >> + * TODO: The code is currently using only the features detected on the boot >> + * core. In the long term we should try to compute values containing only >> + * features supported by all cores. >> + */ >> + identify_cpu(&guest_cpuinfo); > > Given that we already have boot_cpu_data and current_cpu_data, which > should be already initialized at this point, we could simply: > > guest_cpuinfo = current_cpu_data; > > or > > guest_cpuinfo = boot_cpu_data; > > ? Ack, I will do that. Cheers Bertrand > > >> +#ifdef CONFIG_ARM_64 >> + /* Disable MPAM as xen does not support it */ >> + guest_cpuinfo.pfr64.mpam = 0; >> + guest_cpuinfo.pfr64.mpam_frac = 0; >> + >> + /* Disable SVE as Xen does not support it */ >> + guest_cpuinfo.pfr64.sve = 0; >> + guest_cpuinfo.zfr64.bits[0] = 0; >> + >> + /* Disable MTE as Xen does not support it */ >> + guest_cpuinfo.pfr64.mte = 0; >> +#endif >> + >> + /* Disable AMU */ >> +#ifdef CONFIG_ARM_64 >> + guest_cpuinfo.pfr64.amu = 0; >> +#endif >> + guest_cpuinfo.pfr32.amu = 0; >> + >> + /* Disable RAS as Xen does not support it */ >> +#ifdef CONFIG_ARM_64 >> + guest_cpuinfo.pfr64.ras = 0; >> + guest_cpuinfo.pfr64.ras_frac = 0; >> +#endif >> + guest_cpuinfo.pfr32.ras = 0; >> + guest_cpuinfo.pfr32.ras_frac = 0; >> + >> + return 0; >> +} >> +/* >> + * This function needs to be run after all smp are started to have >> + * cpuinfo structures for all cores. >> + */ >> +__initcall(create_guest_cpuinfo); >> + >> /* >> * Local variables: >> * mode: C >> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h >> index 64354c3f19..0ab6dd42a0 100644 >> --- a/xen/include/asm-arm/cpufeature.h >> +++ b/xen/include/asm-arm/cpufeature.h >> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *); >> extern struct cpuinfo_arm cpu_data[]; >> #define current_cpu_data cpu_data[smp_processor_id()] >> >> +extern struct cpuinfo_arm guest_cpuinfo; >> + >> #endif /* __ASSEMBLY__ */ >> >> #endif >> -- >> 2.17.1
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c index 204be9b084..309941ff37 100644 --- a/xen/arch/arm/cpufeature.c +++ b/xen/arch/arm/cpufeature.c @@ -24,6 +24,8 @@ DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS); +struct cpuinfo_arm __read_mostly guest_cpuinfo; + void update_cpu_capabilities(const struct arm_cpu_capabilities *caps, const char *info) { @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c) #endif } +/* + * This function is creating a cpuinfo structure with values modified to mask + * all cpu features that should not be published to guest. + * The created structure is then used to provide ID registers values to guests. + */ +static int __init create_guest_cpuinfo(void) +{ + /* + * TODO: The code is currently using only the features detected on the boot + * core. In the long term we should try to compute values containing only + * features supported by all cores. + */ + identify_cpu(&guest_cpuinfo); + +#ifdef CONFIG_ARM_64 + /* Disable MPAM as xen does not support it */ + guest_cpuinfo.pfr64.mpam = 0; + guest_cpuinfo.pfr64.mpam_frac = 0; + + /* Disable SVE as Xen does not support it */ + guest_cpuinfo.pfr64.sve = 0; + guest_cpuinfo.zfr64.bits[0] = 0; + + /* Disable MTE as Xen does not support it */ + guest_cpuinfo.pfr64.mte = 0; +#endif + + /* Disable AMU */ +#ifdef CONFIG_ARM_64 + guest_cpuinfo.pfr64.amu = 0; +#endif + guest_cpuinfo.pfr32.amu = 0; + + /* Disable RAS as Xen does not support it */ +#ifdef CONFIG_ARM_64 + guest_cpuinfo.pfr64.ras = 0; + guest_cpuinfo.pfr64.ras_frac = 0; +#endif + guest_cpuinfo.pfr32.ras = 0; + guest_cpuinfo.pfr32.ras_frac = 0; + + return 0; +} +/* + * This function needs to be run after all smp are started to have + * cpuinfo structures for all cores. + */ +__initcall(create_guest_cpuinfo); + /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h index 64354c3f19..0ab6dd42a0 100644 --- a/xen/include/asm-arm/cpufeature.h +++ b/xen/include/asm-arm/cpufeature.h @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *); extern struct cpuinfo_arm cpu_data[]; #define current_cpu_data cpu_data[smp_processor_id()] +extern struct cpuinfo_arm guest_cpuinfo; + #endif /* __ASSEMBLY__ */ #endif
Create a cpuinfo structure for guest and mask into it the features that we do not support in Xen or that we do not want to publish to guests. Modify some values in the cpuinfo structure for guests to mask some features which we do not want to allow to guests (like AMU) or we do not support (like SVE). The code is trying to group together registers modifications for the same feature to be able in the long term to easily enable/disable a feature depending on user parameters or add other registers modification in the same place (like enabling/disabling HCR bits). Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> --- Changes in V2: rebase --- xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++ xen/include/asm-arm/cpufeature.h | 2 ++ 2 files changed, 53 insertions(+)