Re: [PATCH qemu v2 2/5] hw/cxl/events: Update for rev3.2 common event record format

[Markus Armbruster <armbru@redhat.com>]

Jonathan Cameron <jonathan.cameron@huawei.com> writes:

> On Mon, 12 Jan 2026 13:16:05 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Jonathan Cameron <Jonathan.Cameron@huawei.com> writes:
>> 
>> > From: Shiju Jose <shiju.jose@huawei.com>
>> >
>> > CXL spec 3.2 section 8.2.9.2.1 Table 8-55, Common Event Record
>> > format has updated with optional Maintenance Operation Subclass,
>> > LD ID and ID of the device head information.
>> >
>> > Add updates for the above optional parameters in the related
>> > CXL events reporting and in the QMP commands to inject CXL events.
>> >
>> > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Hi Markus,
> Thanks for taking a look!

You're welcome!

>> > ---
>> >  qapi/cxl.json               | 20 ++++++++---
>> >  include/hw/cxl/cxl_device.h |  7 +++-
>> >  include/hw/cxl/cxl_events.h | 15 ++++++--
>> >  hw/cxl/cxl-events.c         |  3 +-
>> >  hw/cxl/cxl-mailbox-utils.c  |  3 +-
>> >  hw/mem/cxl_type3.c          | 70 ++++++++++++++++++++++++++++++++-----
>> >  hw/mem/cxl_type3_stubs.c    | 24 +++++++++++--
>> >  7 files changed, 121 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/qapi/cxl.json b/qapi/cxl.json
>> > index d5b86159f1..b3c2ac9575 100644
>> > --- a/qapi/cxl.json
>> > +++ b/qapi/cxl.json
>> > @@ -33,20 +33,32 @@
>> >  ##
>> >  # @CXLCommonEventBase:
>> >  #
>> > -# Common event base for a CXL Event (CXL r3.0 8.2.9.2.1
>> > -# Table 8-42 Common Event Record Format).
>> > +# Common event base for a CXL Event (CXL r3.2 8.2.10.2.1
>> > +# Table 8-55 Common Event Record Format).
>> >  #
>> >  # @path: CXL type 3 device canonical QOM path
>> >  #
>> >  # @log: event log to add the event to
>> >  #
>> > -# @flags: Event Record Flags.  See CXL r3.0 Table 8-42 Common Event
>> > +# @flags: Event Record Flags.  See CXL r3.2 Table 8-55 Common Event
>> >  #     Record Format, Event Record Flags for subfield definitions.
>> >  #
>> > +# @maint-op-class: Maintenance operation class the device requests to
>> > +#     initiate.
>> > +#
>> > +# @maint-op-subclass: Maintenance operation subclass the device
>> > +#     requests to initiate.
>> > +#
>> > +# @ld-id: LD ID of LD from where the event originated.  
>> 
>> What's an LD?
>
> Logical Device.  I'll spell it out the first time.

Yes, please.

>                                                    From the glossary:
> "LD: Logical Device. Entity that represents a CXL Endpoint that is bound to
> a VCS.  An SLD contains one LD. An MLD contains multiple LDs."

A line of acronyms marching ponderously across the page... made me laugh
:)

> So what does that actually mean?  It's a number that identifies the
> bit of a device that is presented to a given host at a particular
> point in the PCI topology that we are faking from the underlying CXL
> one. 
>
> I think best I can do is just spell out the acronyms. If people
> want more they need to look at the spec :(

Fair!

> Time for a deeper explanation that probably no one ever wanted :)
>
> From a physical topology point of view think of something like:
>
> vPPDs are virtual downstream ports that assign an LD ID.
>
> 	HOST 0           HOST 1               Host 3
> 	  |                 |                   |
>          RP                RP                  RP
>           |                 |                   |
>           |                 |                   |
>         __|_________________|______             |
>        |  |   SWITCH        |      |            |
>        | _|_____       _____|_     |            |
>        ||       |     |       |    |            |
>        ||       |     |       |    |            |
>        |vPPD0  vPPD1 VPPD2   VPPD3 |            |
>        ||       |     |          | |            |
>        ||_____________|          | |            |
>        |           |             | |            |
>        |___________DSP0________DSP1|            |
>                      |           |              |
>                      |       Some other dev     |
>               Traffic tagged                    | 
>               with an LD_ID associated with     |
>               a given vPPD (so 2 tags here)     |
>                      |                          |
>      ________________|__________________________|______
>     | (CXL Device) HEAD0                     HEAD1     |
>     |           Now splits up based         Only one LD|
>     |           on LD_ID                               |
>     |__________________________________________________|
>
>
> Hosts see:
> Used letters for down stream port numbers to avoid
> any explicit mapping to the physical ports above.
>
>
>           HOST 0                  Host 1             Host3   
>             |                       |                  |
>            RP                       RP                RP
>             |                       |                  |
>     ________|_________     _________|_________         |
>    |        |         |   |         |         |        |
>    |    ____|____     |   |   ______|______   |        |
>    |   |         |    |   |  |             |  |        |
>    |__DSPA______DSPB__|   |__DSPA________DSPB_|        |
>        |          |           |            |           |
>     ___|______  Nothing  _____|_____      Other dev ___|______
>    |CXL Type3 |          |CXL Type3 |              |CXL Type3 |
>    | (H0, LD0)|          | (H0, LD1)|              | (H1, LD0)|
>    |__________|          |__________|              |__________|
>
> These IDs should only be reported for events that are isolated 
> (as appropriate) to a head and/or LD and as you can see the hosts
> don't actually know these IDs even exist hence there is no need to
> report them via in band path.  If they don't take the values
> that reflect the LD we are actually talking to, then we are
> reporting someone else's error! Via out of band / switch CCI we
> see the top diagram where we definitely need LD_ID and HEAD_ID
> to make any sense of errors, particularly as the memory addresses
> in the records are specific to Head/LD pair.
>  
> The recommendation of the spec is don't set them for simple reporting
> over inband mailboxes. Now you might think we don't have out of band
> support upstream yet (my staging CXL tree does have both MCTP over
> I2C and USB) but we do have the switch CCI and that can tunnel to
> out of band interfaces (technically it's over PCI Vendor defined
> messages but in practice it ends up the same from a 'what can I talk
> to' point of view as something truely out of band like an I2C bus.
> We don't actually have event queues on those yet though... When
> we do we'll want this command to queue events there as well as on
> for inband path.

Truly more than I ever wanted ;)

> So we are adding them now for 2 reasons.
> 1) The spec allows it even for boring inband event reporting
>    (recommendation only to not do so) so we need to check the kernel stack
>    reports correctly.
> 2) We will want to add event queues on the out of band interfaces and
>    for those we'll need these IDs.

Makes sense.  Thanks!

>> > +#
>> > +# @head-id: ID of the device head from where the event originated.  
>> 
>> Are these identifiers taken from the CXL spec?
> Yes. Though feels odd to reference the glossary for the definitions.

Since they are from the CXL spec, I don't need to think about names that
are more in line with established QAPI naming conventions.  That's why I
asked.  We're good there.

>> > +#
>> >  # Since: 8.1
>> >  ##
>> >  { 'struct': 'CXLCommonEventBase',
>> > -  'data': { 'path': 'str', 'log': 'CxlEventLog', 'flags': 'uint8' } }
>> > +  'data': { 'path': 'str', 'log': 'CxlEventLog', 'flags': 'uint32',
>> > +            '*maint-op-class':'uint8', '*maint-op-subclass':'uint8',
>> > +            '*ld-id':'uint16', '*head-id':'uint8' } }
>> >  
>> >  ##
>> >  # @CXLGeneralMediaEvent:  
>> 
>> [...]