Re: [PATCH 2/8] qapi/qom: Introduce smp-cache object

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu,  4 Jul 2024 11:15:57 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> Introduce smp-cache object so that user could define cache properties.
> 
> In smp-cache object, define cache topology based on CPU topology level
> with two reasons:
> 
> 1. In practice, a cache will always be bound to the CPU container
>    (either private in the CPU container or shared among multiple
>    containers), and CPU container is often expressed in terms of CPU
>    topology level.
> 2. The x86's cache-related CPUIDs encode cache topology based on APIC
>    ID's CPU topology layout. And the ACPI PPTT table that ARM/RISCV
>    relies on also requires CPU containers to help indicate the private
>    shared hierarchy of the cache. Therefore, for SMP systems, it is
>    natural to use the CPU topology hierarchy directly in QEMU to define
>    the cache topology.
> 
> Currently, separated L1 cache (L1 data cache and L1 instruction cache)
> with unified higher-level cache (e.g., unified L2 and L3 caches), is the
> most common cache architectures.
> 
> Therefore, enumerate the L1 D-cache, L1 I-cache, L2 cache and L3 cache
> with smp-cache object to add the basic cache topology support.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Suggested by credit:
>  * Referred to Daniel's suggestion to introduce cache JSON list, though
>    as a standalone object since -smp/-machine can't support JSON.

A comment and a query on the code movement.  Whilst not particularly
elegant (as would be nice if -smp or machine supported json) using
an object isn't too bad.

Given both comments are trivial.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> index 82413c668bdb..8b8c0e9eeb86 100644
> --- a/qapi/machine-common.json
> +++ b/qapi/machine-common.json

> +
> +##
> +# @SMPCacheName:
> +#
> +# An enumeration of cache for SMP systems.  The cache name here is
> +# a combination of cache level and cache type.
> +#
> +# @l1d: L1 data cache.
> +#
> +# @l1i: L1 instruction cache.
> +#
> +# @l2: L2 (unified) cache.
> +#
> +# @l3: L3 (unified) cache
> +#
> +# Since: 9.1
> +##
> +{ 'enum': 'SMPCacheName',
> +  'prefix': 'SMP_CACHE',
> +  'data': [ 'l1d', 'l1i', 'l2', 'l3' ] }

I guess we can extend this as necessary for other designs
with more caches or split to higher levels. This is a reasonable
place to start.  Splitting and L2 is going to require more
complex code elsewhere but we can deal with that if anyone
needs to do so.


> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index b1581988e4eb..25394f2cda50 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -64,11 +64,11 @@
>  { 'include': 'compat.json' }
>  { 'include': 'control.json' }
>  { 'include': 'introspect.json' }
> -{ 'include': 'qom.json' }
> -{ 'include': 'qdev.json' }
>  { 'include': 'machine-common.json' }
>  { 'include': 'machine.json' }
>  { 'include': 'machine-target.json' }
> +{ 'include': 'qom.json' }
> +{ 'include': 'qdev.json' }
If this is a necessary bit of movement, then a comment on that
in the patch introduction would be good to have.

>  { 'include': 'replay.json' }
>  { 'include': 'yank.json' }
>  { 'include': 'misc.json' }