| Message ID | 0bae0aebf44ac89b47ef11c5d101323dd5540951.1696408966.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/63] pci: SLT must be RO | expand |
On 04/10/2023 10.44, Michael S. Tsirkin wrote: > From: Dave Jiang <dave.jiang@intel.com> > > Add a simple _DSM call support for the ACPI0017 device to return a fake QTG > ID value of 0 in all cases. The enabling is for _DSM plumbing testing > from the OS. > > Following edited for readbility only > > Device (CXLM) > { > Name (_HID, "ACPI0017") // _HID: Hardware ID > ... > Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method > { > If ((Arg0 == ToUUID ("f365f9a6-a7de-4071-a66a-b40c0b4f8e52"))) > { > If ((Arg2 == Zero)) > { > Return (Buffer (One) { 0x01 }) > } > > If ((Arg2 == One)) > { > Return (Package (0x02) > { > Buffer (0x02) > { 0x01, 0x00 }, > Package (0x01) > { > Buffer (0x02) > { 0x00, 0x00 } > } > }) > } > } > } > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > -- > v2: Minor edit to drop reference to switches in patch description. > Message-Id: <20230904161847.18468-3-Jonathan.Cameron@huawei.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > include/hw/acpi/cxl.h | 1 + > hw/acpi/cxl.c | 57 +++++++++++++++++++++++++++++++++++++++++++ > hw/i386/acpi-build.c | 1 + > 3 files changed, 59 insertions(+) > > diff --git a/include/hw/acpi/cxl.h b/include/hw/acpi/cxl.h > index acf4418886..8f22c71530 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 > diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c > index 92b46bc932..5e9039785a 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); This looks fishy ... first declaring a uint16_t array in host endian order, then casting it to an uint8_t* byte array for code that likely expects little endian byte order in the guest ... I assume that this won't work and needs some more love for big endian hosts? Thomas > + aml_append(ifctx2, aml_return(pak)); > + } > + aml_append(ifctx, ifctx2); > + } > + aml_append(method, ifctx); > + aml_append(dev, method); > +}
On Wed, Oct 04, 2023 at 07:46:19PM +0200, Thomas Huth wrote: > On 04/10/2023 10.44, Michael S. Tsirkin wrote: > > From: Dave Jiang <dave.jiang@intel.com> > > > > Add a simple _DSM call support for the ACPI0017 device to return a fake QTG > > ID value of 0 in all cases. The enabling is for _DSM plumbing testing > > from the OS. > > > > Following edited for readbility only > > > > Device (CXLM) > > { > > Name (_HID, "ACPI0017") // _HID: Hardware ID > > ... > > Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method > > { > > If ((Arg0 == ToUUID ("f365f9a6-a7de-4071-a66a-b40c0b4f8e52"))) > > { > > If ((Arg2 == Zero)) > > { > > Return (Buffer (One) { 0x01 }) > > } > > > > If ((Arg2 == One)) > > { > > Return (Package (0x02) > > { > > Buffer (0x02) > > { 0x01, 0x00 }, > > Package (0x01) > > { > > Buffer (0x02) > > { 0x00, 0x00 } > > } > > }) > > } > > } > > } > > > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > -- > > v2: Minor edit to drop reference to switches in patch description. > > Message-Id: <20230904161847.18468-3-Jonathan.Cameron@huawei.com> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > include/hw/acpi/cxl.h | 1 + > > hw/acpi/cxl.c | 57 +++++++++++++++++++++++++++++++++++++++++++ > > hw/i386/acpi-build.c | 1 + > > 3 files changed, 59 insertions(+) > > > > diff --git a/include/hw/acpi/cxl.h b/include/hw/acpi/cxl.h > > index acf4418886..8f22c71530 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 > > diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c > > index 92b46bc932..5e9039785a 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); > > This looks fishy ... first declaring a uint16_t array in host endian order, > then casting it to an uint8_t* byte array for code that likely expects > little endian byte order in the guest ... I assume that this won't work and > needs some more love for big endian hosts? > > Thomas > Oh, good catch! So that would be "Add dummy ACPI QTG DSM". I actually also don't like sizeof(uint16_t) here, it should be sizeof array. Maybe it actually can use build_append_int_noprefix in some way? And inciate is a typo. > > + aml_append(ifctx2, aml_return(pak)); > > + } > > + aml_append(ifctx, ifctx2); > > + } > > + aml_append(method, ifctx); > > + aml_append(dev, method); > > +} >
diff --git a/include/hw/acpi/cxl.h b/include/hw/acpi/cxl.h index acf4418886..8f22c71530 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 diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c index 92b46bc932..5e9039785a 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, PXBCXLDev *cxl) { PXBDev *pxb = PXB_DEV(cxl); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 4d2d40bab5..4fb18fee3c 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1422,6 +1422,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);