Re: [PATCH v9 11/12] target/arm: add an experimental mpidr arm cpu property object

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Sun, 25 Aug 2024 12:34:14 +0100
Peter Maydell <peter.maydell@linaro.org> escreveu:

> On Sun, 25 Aug 2024 at 04:46, Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Accurately injecting an ARM Processor error ACPI/APEI GHES
> > error record requires the value of the ARM Multiprocessor
> > Affinity Register (mpidr).
> >
> > While ARM implements it, this is currently not visible.
> >
> > Add a field at CPU storing it, and place it at arm_cpu_properties
> > as experimental, thus allowing it to be queried via QMP using
> > qom-get function.  
> 
> >  static Property arm_cpu_properties[] = {
> >      DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0),
> > +    DEFINE_PROP_UINT64("x-mpidr", ARMCPU, mpidr, 0),
> >      DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
> >                          mp_affinity, ARM64_AFFINITY_INVALID),
> >      DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),  
> 
> Why do we need this? 

The ACPI HEST tables, in particular when using GHESv2 provide
several kinds of errors. Among them, we have ARM Processor Error,
as defined at UEFI 2.10 spec (and earlier versions), the Common
Platform Error Record (CPER) is defined as:

   https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html?highlight=ghes#arm-processor-error-section

There are two fields that are part of the CPER record. One of them is
mandatory (MIDR); the other one is optional, but needed to decode another
field.

So, basically those errors need them.

> Why is it experimental? 

This was a suggestion from Igor. As for now the QAPI for external
error injection is experimental, It makes sense to me to keep it
experimental as well.

> The later patch
> seems to use it via QMP, which I'm not super enthusiastic
> about -- the preexisting mpidr and mp-affinity properties are
> there for code that is creating CPU objects to configure
> the CPU object, not as a query interface for QOM.

I saw that. Basically the decoding by OS guest depends on MPIDR,
as explained at the description of Error affinity level field:

	"For errors that can be attributed to a specific affinity level, 
	this field defines the affinity level at which the error was 
	produced, detected, and/or consumed. This is a value between 0
	and 3. All other values (4-255) are reserved

	For example, a vendor may choose to define affinity levels as
	follows:
	Level 0: errors that can be precisely attributed to a specific CPU
	(e.g. due to a synchronous external abort)
	Level 1: Cache parity and/or ECC errors detected at cache of affinity
	level 1 (e.g. only attributed to higher level cache due to 
	prefetching and/or error propagation)

	NOTE: Detailed meanings and groupings of affinity level are chip 
	and/or platform specific. The affinity level described here must 
	be consistent with the platform definitions used MPIDR. For
	cache/TLB errors, the cache/TLB level is provided by the cache/TLB
	error structure, which may differ from affinity level."

Regards,
Mauro