Hi Volodymyr,
> On 30 Nov 2020, at 20:15, Volodymyr Babchuk <volodymyr_babc...@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.marq...@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
