在 2026/10/26 21:05, Markus Armbruster 写道:
> Jinjie Ruan <[email protected]> writes:
>
>> From: Jonathan Cameron <[email protected]>
>>
>> Add the query-mpam-cache QMP command schema: the MpamBm bitmap
>> type, the per-RIS MpamRegs state and the MpamCacheInfo summary of
>> a cache MSC, plus an optional cache level filter. The command is
>> stubbed to return NULL so the generated marshal links; the real
>> implementation walks the device tree in the next patch.
>>
>> Based on an RFC series by Jonathan Cameron, adapted to the current
>> qapi documentation rules.
>>
>> Signed-off-by: Jonathan Cameron <[email protected]>
>> Signed-off-by: Jinjie Ruan <[email protected]>
>
> [...]
>
>> diff --git a/qapi/meson.build b/qapi/meson.build
>> index a46269b..c6663e5 100644
>> --- a/qapi/meson.build
>> +++ b/qapi/meson.build
>> @@ -42,6 +42,7 @@ qapi_all_modules = [
>> 'machine-s390x',
>> 'migration',
>> 'misc',
>> + 'mpam',
>> 'net',
>> 'pragma',
>> 'qom',
>> diff --git a/qapi/mpam.json b/qapi/mpam.json
>> new file mode 100644
>> index 0000000..ff476a1
>> --- /dev/null
>> +++ b/qapi/mpam.json
>> @@ -0,0 +1,120 @@
>> +# -*- Mode: Python -*-
>> +# vim: filetype=python
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +##
>> +# = ARM MPAM State Introspection
>> +##
>
> Special QAPI doc markup that hasn't been recognized in more than a year.
> Please build with --enable-docs, and examine
> docs/manual/interop/qemu-qmp-ref.html to guard against such mishaps.
>
> Use standard reST markup instead:
Hi Markus,
Will fix it in v2 and build with --enable-docs.
>
> ##
> # ****************************
> # ARM MPAM State Introspection
> # ****************************
> ##
>
> Are you *sure* this ought to be between "Miscellanea" and "Audio" and at
> the same heading level? Or should it be deeper and/or elsewhere?
Yes, it should be placed in arm64 related chapter.
>
>> +
>> +##
>> +# @MpamBm:
>> +#
>> +# A bitmap control value, as used by MPAM cache-portion (CPBM) and
>> +# memory-bandwidth-portion (MBW_PBM) controls. The value is
>> +# carried as an array of 32-bit words, least significant first.
>> +#
>
> Anti-pattern: the introduction (above) discusses the value of a member
> (below).
>
>> +# @words: The bitmap, one element per 32-bits of the control
>> +#
>> +# Since: 10.1
>
> 11.2 now. Same elsewhere, not noting it again.
Will update it.
>
>> +##
>> +{ 'struct': 'MpamBm',
>> + 'data' : { 'words' : [ 'int' ]
>
> The description says it's 32 bit words. Shouldn't you use 'uint32'
> then?
>
>> + }
>> +}
>> +
>> +##
>> +# @MpamRegs:
>> +#
>> +# Per RIS register state of an emulated MPAM Memory System Component.
>
> Please use the current syntax for introductions:
Ok, will update it.
>
> ##
> # @name: introduction, best to keep it brief, but you may continue it
> # like this if necessary. If your text need to refer to
> # arguments, it's almost certainly not brief introduction
> # anymore.
>
> Same elsewhere, not noting it again.
Ok, will update all the same comment.
>
>> +#
>> +# @idr: MPAMF_IDR, the MPAM features identification register
>> +#
>> +# @iidr: MPAMF_IIDR, the MPAM implementer identification register
>> +#
>> +# @aidr: MPAMF_AIDR, the MPAM architecture identification register
>> +#
>> +# @cpor-idr: MPAMF_CPOR_IDR, cache-portion feature register
>> +#
>> +# @ccap-idr: MPAMF_CCAP_IDR, cache-capacity feature register
>> +#
>> +# @mbw-idr: MPAMF_MBW_IDR, memory-bandwidth feature register
>> +#
>> +# @pri-idr: MPAMF_PRI_IDR, priority feature register
>> +#
>> +# @partid-nrw-idr: MPAMF_PARTID_NRW_IDR, PARTID narrowing register
>> +#
>> +# @msmon-idr: MPAMF_MSMON_IDR, monitor feature register
>> +#
>> +# @csumon-idr: MPAMF_CSUMON_IDR, cache-usage monitor register
>> +#
>> +# @mbwumon-idr: MPAMF_MBWUMON_IDR, memory-bandwidth monitor register
>> +#
>> +# @ecr: MPAMF_ECR, the error control register
>> +#
>> +# @esr: MPAMF_ESR, the error status register
>> +#
>> +# @cfg-part-sel: MPAMCFG_PART_SEL, the partition-selection register
>> +#
>> +# @cfg-cpbm: The configured cache-portion bitmaps, one per
>> +# internal PARTID
>> +#
>> +# Since: 10.1
>> +##
>> +{ 'struct' : 'MpamRegs',
>> + 'data' : { 'idr' : 'int',
>> + 'iidr' : 'int',
>> + 'aidr' : 'int',
>> + 'cpor-idr': 'int',
>> + 'ccap-idr': 'int',
>> + 'mbw-idr': 'int',
>> + 'pri-idr': 'int',
>> + 'partid-nrw-idr': 'int',
>> + 'msmon-idr': 'int',
>> + 'csumon-idr': 'int',
>> + 'mbwumon-idr': 'int',
>> + 'ecr': 'int',
>> + 'esr': 'int',
>> + 'cfg-part-sel': 'int',
>> + 'cfg-cpbm': ['MpamBm']#lazy
>
> Is the comment an accident?
>
> How wide are these registers? Is 'int' the appropriate type?
>
>> + }
>> +}
>> +
>> +##
>> +# @MpamCacheInfo:
>> +#
>> +# Information about an MPAM cache Memory System Component.
>> +#
>> +# @cpu: First CPU of the set associated with the cache
>> +#
>> +# @level: Level of cache
>> +#
>> +# @type: Type of cache
>> +#
>> +# @regs: The per RIS register state
>> +#
>> +# Since: 10.1
>> +##
>> +{ 'struct': 'MpamCacheInfo',
>> + 'data' : { 'cpu' : 'int',
>> + 'level' : 'int',
>> + 'type' : 'int',
>> + 'regs' : ['MpamRegs']
>> + }
>> +}
>> +
>> +##
>> +# @query-mpam-cache:
>> +#
>> +# Get a list of MpamCacheInfo for all Cache related MSC
>> +#
>> +# @level: Provide a cache level to filter against
>
> What exactly does this argument filter out?
>
> Is it really needed? How large can unfiltered output become?
>
>> +#
>> +# Returns: a list of @MpamCacheInfo describing all
>> +# the MPAM cache MSC instances
>
> This gets rendered like
>
> Return: [MpamCacheInfo] – a list of MpamCacheInfo describing all the
> MPAM cache MSC instances
>
> As is, the description is mostly redundant. Do you need a description?
> Try building without it, and see how it comes out.
>
>> +#
>> +# Since: 10.1
>> +##
>> +{ 'command': 'query-mpam-cache',
>> + 'data': { '*level': 'int' },
>> + 'returns' : ['MpamCacheInfo'],
>> + 'allow-preconfig': true }
>> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
>> index b93dd68..cef175d 100644
>> --- a/qapi/qapi-schema.json
>> +++ b/qapi/qapi-schema.json
>> @@ -64,6 +64,7 @@
>> { 'include': 'replay.json' }
>> { 'include': 'yank.json' }
>> { 'include': 'misc.json' }
>> +{ 'include': 'mpam.json' }
>> { 'include': 'misc-arm.json' }
>> { 'include': 'misc-i386.json' }
>> { 'include': 'audio.json' }
>
--
Best regards,
Jinjie
