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.ji...@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.ji...@intel.com>
> > Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
> >
> > --
> > v2: Minor edit to drop reference to switches in patch description.
> > Message-Id: <20230904161847.18468-3-jonathan.came...@huawei.com>
> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <m...@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);
> > +}
>
