[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 3/4] xen/arm: Sanitize cpuinfo ID registers fields
Hi Julien, > On 19 Jul 2021, at 09:58, Julien Grall <julien@xxxxxxx> wrote: > > > > On 16/07/2021 18:14, Bertrand Marquis wrote: >> Hi Julien > > Hi Bertrand, > >> […] >>>> >>>> + >>>> + if ( old_reg != new_reg ) >>>> + printk(XENLOG_DEBUG "SANITY DIF: %s 0x%"PRIx64" -> 0x%"PRIx64"\n", >>>> + reg_name, old_reg, new_reg); >>>> + if ( old_reg != *cur_reg ) >>>> + printk(XENLOG_DEBUG "SANITY FIX: %s 0x%"PRIx64" -> 0x%"PRIx64"\n", >>>> + reg_name, old_reg, *cur_reg); >>>> + >>>> + if ( taint ) >>>> + { >>>> + printk(XENLOG_WARNING "SANITY CHECK: Unexpected variation in >>>> %s.\n", >>>> + reg_name); >>>> + add_taint(TAINT_CPU_OUT_OF_SPEC); >>>> + } >>>> +} >>>> + >>>> + >>>> +/* >>>> + * This function should be called on secondary cores to sanitize the boot >>>> cpu >>>> + * cpuinfo. >>> >>> Can we renamed boot_cpu_data to system_cpuinfo (or something similar)? This >>> would make clear this is not only the features for the boot CPU? >> While looking at this request, I checked a bit how boot_cpu_data and >> cpu_data overall are used: >> - boot_cpu_data is only used in setup.c, by boot_cpu_features macros, in >> smpboot to retrieve the bootcpu midr, in p2m and by cpufeatures >> - cpu_data[] is used in smpboot, in errata handling to test for csv2, and in >> vcpreg to access the midr >> So we have a bunch of cpuinfo structures as global variables but most of >> them are not really used or did I miss something ? > > While I agree this is not useful today, the idea is we can find easily what > features each processor supports. This could be useful if we wanted to expose > big.LITTLE to the guest. > > For instance, imagine you have a system where some processor may support > 32-bit EL1 only on some processor. With a global approach, we would say > "32-bit EL1 is not supported". That would prevent a user to use the system to > its full advantage. > > Note that I am not asking to implement such things today... This is more to > show that we will likely want to keep the per-CPU info around. > > The system_cpuinfo could be used for system wide decision in Xen (e.g. P2M > size, cacheline size....) while the per-CPU could be used to enable features > only used by a couple of CPUs. I understand the potential need (even though supporting to assign guest CPUs depending on features needed would be something complex to achieve). > >> So I am wondering if we should not reduce a bit the amount of global data >> and: > > How much are we talking? We are declaring cpuinfo for all possible cores in smpboot: struct cpuinfo_arm cpu_data[NR_CPUS]; And default value for NR_CPUS is 128 so that makes around 9k of data. This is not that much I agree. > >> - introduce a global system_cpuinfo >> - remove cpu_data[] and use a temp structure in the stack of the cpu booting >> - read midr directly in vcpreg >> - use boot_cpu_data in errata for csv2 > > This would not be quite the same. You may have a system where not all > processors have ID_AA64PFR0_EL1.CSV2 is set, yet we want to avoid setting the > hardening vector on process with the bit set to reduce the overhead. Agree and with the actual size reduction being quite small this does not make sense. Thanks a lot for the feedback, I will go on with the system_cpuinfo and is. Cheers Bertrand > > Cheers, > > -- > Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |