[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 3/4] xen/arm: Sanitize cpuinfo ID registers fields

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.

So I am wondering if we should not reduce a bit the amount of global data and:

How much are we talking?

- 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.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.