Hi Jonathan,
Thanks for your review and feedback!
[snip]
> > Note, 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
>
> Really trivial but CPU Containers are a different ACPI concept.
> For PPTT they are referred to as Processor Groups. Wonderfully they
> 'might match a Processor Container in the namespace' which rather implies
> they might not. In QEMU they always will because the next bit of the
> spec matters. "In that case this entry will match the value of the _UID
> method of the associated processor container. Where there is a match it must
> be represented."
>
> So having said all that, CPU container is probably fine as a description.
Thanks for the explanation!
> > 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.
> >
> > Suggested-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > Signed-off-by: Zhao Liu <zhao1.liu@xxxxxxxxx>
> > Tested-by: Yongwei Ma <yongwei.ma@xxxxxxxxx>
>
> Seems fine but my gut would be to combine this and next patch so we can
> see how it is used (assuming no one asked for it to be separate!)
No problem. I intended to make it easier to review the QAPI part, but
these two patches were simple enough that I was happy to combine them.
> Version numbers need an update I guess.
Ah, yes!
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Thanks!
> > +##
> > +# @SmpCachePropertiesWrapper:
> > +#
> > +# List wrapper of SmpCacheProperties.
> > +#
> > +# @caches: the list of SmpCacheProperties.
> > +#
> > +# Since 9.1
>
> Needs updating to 9.2 I guess.
Yes, I think so, too.
Thanks,
Zhao
> > +##
> > +{ 'struct': 'SmpCachePropertiesWrapper',
> > + 'data': { 'caches': ['SmpCacheProperties'] } }
>
