Re: [PATCH] hw/cxl: Add QTG _DSM support for ACPI0017 device

[Dave Jiang]

On 1/26/23 11:24 AM, Jonathan Cameron wrote:
> On Thu, 26 Jan 2023 11:07:37 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
> Hi Dave,
>
> That was quick!
>
> > Add a simple _DSM call support for the ACPI0017 device to return a fake QTG
> > ID value. Given the current CXL implementation does not involve switches,
>
> I don't follow.  What part doesn't involve switches?
>
> > a faked value of 0 can be returned for the QTG ID. The enabling is for _DSM
> > plumbing testing from the OS.
>
> Can you include a dump iasl -d for the DSDT chunk this generates. Much
> easier to review with that available.

        Device (CXLM)
        {
            Name (_HID, "ACPI0017")  // _HID: Hardware ID
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (One)
            }

            Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
            {
                If ((Arg0 == ToUUID 
("f365f9a6-a7de-4071-a66a-b40c0b4f8e52") /* Unknown UUID */))
                {
                    If ((Arg2 == Zero))
                    {
                        Return (Buffer (One)
                        {
                             0x01 
      // .
                        })
                    }

                    If ((Arg2 == One))
                    {
                        Return (Package (0x02)
                        {
                            Buffer (0x02)
                            {
                                 0x01, 0x00 
          // ..
                            },

                            Package (0x01)
                            {
                                Buffer (0x02)
                                {
                                     0x00, 0x00 
              // ..
                                }
                            }
                        })
                    }
                }


> On that note, tests need updating I think
> tests/qtest/bios-tables-test.c data which is in
> tests/data/acpi/q35/DSDT.cxl
>
> We should update that test code as part of the volatile series as well
> as it's using the deprecated memdev parameter - not critical
> but never a good thing to leave old examples of what not to use in
> the tests.
>
> Thanks,
>
> Jonathan
>
> p.s. I'm too lazy to look at the code without the AML to compare with
> as I'll review the AML first then look at if there are any oddities
> in the generation code.
>
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> >   hw/acpi/cxl.c         |   57 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   hw/i386/acpi-build.c  |    1 +
> >   include/hw/acpi/cxl.h |    1 +
> >   3 files changed, 59 insertions(+)
> >
> > diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c
> > index 2bf8c0799359..cd6839c24416 100644
> > --- a/hw/acpi/cxl.c
> > +++ b/hw/acpi/cxl.c
> > @@ -30,6 +30,63 @@
> >   #include "qapi/error.h"
> >   #include "qemu/uuid.h"
> >   
> > +void build_cxl_dsm_method(Aml *dev)
> > +{
> > +    Aml *method, *ifctx, *ifctx2;
> > +
> > +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> > +    {
> > +        Aml *function, *uuid;
> > +
> > +        uuid = aml_arg(0);
> > +        function = aml_arg(2);
> > +        /* CXL spec v3.0 9.17.3.1 *, QTG ID _DSM */
> > +        ifctx = aml_if(aml_equal(
> > +            uuid, aml_touuid("F365F9A6-A7DE-4071-A66A-B40C0B4F8E52")));
> > +
> > +        /* Function 0, standard DSM query function */
> > +        ifctx2 = aml_if(aml_equal(function, aml_int(0)));
> > +        {
> > +            uint8_t byte_list[1] = { 0x01 }; /* functions 1 only */
> > +
> > +            aml_append(ifctx2,
> > +                       aml_return(aml_buffer(sizeof(byte_list), byte_list)));
> > +        }
> > +        aml_append(ifctx, ifctx2);
> > +
> > +        /*
> > +         * Function 1
> > +         * A return value of {1, {0}} inciate that
> > +         * max supported QTG ID of 1 and recommended QTG is 0.
> > +         * The values here are faked to simplify emulation.
> > +         */
> > +        ifctx2 = aml_if(aml_equal(function, aml_int(1)));
> > +        {
> > +            uint16_t word_list[1] = { 0x01 };
> > +            uint16_t word_list2[1] = { 0 };
> > +            uint8_t *byte_list = (uint8_t *)word_list;
> > +            uint8_t *byte_list2 = (uint8_t *)word_list2;
> > +            Aml *pak, *pak1;
> > +
> > +            /*
> > +             * The return package is a package of a WORD and another package.
> > +             * The embedded package contains 0 or more WORDs for the
> > +             * recommended QTG IDs.
> > +             */
> > +            pak1 = aml_package(1);
> > +            aml_append(pak1, aml_buffer(sizeof(uint16_t), byte_list2));
> > +            pak = aml_package(2);
> > +            aml_append(pak, aml_buffer(sizeof(uint16_t), byte_list));
> > +            aml_append(pak, pak1);
> > +
> > +            aml_append(ifctx2, aml_return(pak));
> > +        }
> > +        aml_append(ifctx, ifctx2);
> > +    }
> > +    aml_append(method, ifctx);
> > +    aml_append(dev, method);
> > +}
> > +
> >   static void cedt_build_chbs(GArray *table_data, PXBDev *cxl)
> >   {
> >       SysBusDevice *sbd = SYS_BUS_DEVICE(cxl->cxl.cxl_host_bridge);
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 285829802b1a..623f26a16db3 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1313,6 +1313,7 @@ static void build_acpi0017(Aml *table)
> >       method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> >       aml_append(method, aml_return(aml_int(0x01)));
> >       aml_append(dev, method);
> > +    build_cxl_dsm_method(dev);
> >   
> >       aml_append(scope, dev);
> >       aml_append(table, scope);
> > diff --git a/include/hw/acpi/cxl.h b/include/hw/acpi/cxl.h
> > index acf441888683..8f22c71530d8 100644
> > --- a/include/hw/acpi/cxl.h
> > +++ b/include/hw/acpi/cxl.h
> > @@ -25,5 +25,6 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
> >                       BIOSLinker *linker, const char *oem_id,
> >                       const char *oem_table_id, CXLState *cxl_state);
> >   void build_cxl_osc_method(Aml *dev);
> > +void build_cxl_dsm_method(Aml *dev);
> >   
> >   #endif