[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 Bertrand,

On 12/07/2021 17:29, Bertrand Marquis wrote:
On 12 Jul 2021, at 12:13, Julien Grall <julien@xxxxxxx> wrote:



On 12/07/2021 12:03, Bertrand Marquis wrote:
Hi Julien,

Hi Bertrand,

On 12 Jul 2021, at 11:16, Julien Grall <julien@xxxxxxx> wrote:

Hi Bertrand,

On 29/06/2021 18:08, Bertrand Marquis wrote:
Define a sanitize_cpu function to be called on secondary cores to
sanitize the cpuinfo structure from the boot CPU.
The safest value is taken when possible and the system is marked tainted
if we encounter values which are incompatible with each other.
Call the sanitize_cpu function on all secondary cores and remove the
code disabling secondary cores if they are not the same as the boot core
as we are now able to handle this use case.
This is only supported on arm64 so cpu_sanitize is an empty static
inline on arm32.
This patch also removes the code imported from Linux that will not be
used by Xen.
Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
---
  xen/arch/arm/arm64/cpusanitize.c | 236 ++++++++++++++++++++++++-------
  xen/arch/arm/smpboot.c           |   5 +-
  xen/include/asm-arm/cpufeature.h |   9 ++
  xen/include/xen/lib.h            |   1 +
  4 files changed, 199 insertions(+), 52 deletions(-)
diff --git a/xen/arch/arm/arm64/cpusanitize.c b/xen/arch/arm/arm64/cpusanitize.c
index 4cc8378c14..744006ca7c 100644
--- a/xen/arch/arm/arm64/cpusanitize.c
+++ b/xen/arch/arm/arm64/cpusanitize.c
@@ -97,10 +97,6 @@ struct arm64_ftr_bits {
                .width = 0,                             \
        }
  -static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap);
-
-static bool __system_matches_cap(unsigned int n);
-
  /*
   * NOTE: Any changes to the visibility of features should be kept in
   * sync with the documentation of the CPU feature register ABI.
@@ -277,31 +273,6 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
        ARM64_FTR_END,
  };
  -static const struct arm64_ftr_bits ftr_ctr[] = {
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RES1 */
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, CTR_DIC_SHIFT, 
1, 1),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, CTR_IDC_SHIFT, 
1, 1),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_HIGHER_OR_ZERO_SAFE, 
CTR_CWG_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_HIGHER_OR_ZERO_SAFE, 
CTR_ERG_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
CTR_DMINLINE_SHIFT, 4, 1),
-       /*
-        * Linux can handle differing I-cache policies. Userspace JITs will
-        * make use of *minLine.
-        * If we have differing I-cache policies, report it as the weakest - 
VIPT.
-        */
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_EXACT, CTR_L1IP_SHIFT, 
2, ICACHE_POLICY_VIPT),   /* L1Ip */
-       ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, 
CTR_IMINLINE_SHIFT, 4, 0),
-       ARM64_FTR_END,
-};

I don't think this is should be dropped. Xen will need to know the safest 
cacheline size that can be used for cache maintenance instructions.
I will surround those entries by #if 0 instead

But, why can't this just be sanitized as the other registers? If this is just a 
matter of missing structure in Xen, then we should add one.

The same goes for the other 2 below.

The point of the RFC was to validate the way to go and do the base.

Right... I was commenting on your suggestion to switch to #if 0 for the next version. I assume this will be a non-RFC and...

Those require changes out of the cpuinfo and are on my todo list to
... it wasn't clear to me that the change on the cpuinfo was on your TODO list for the next version.

So I prefer to ask before you spend time working on a change that I may disagree with ;).

Cheers,

--
Julien Grall



 


Rackspace

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