Re: [Qemu-devel] [PATCH v4 28/33] nvdimm acpi: support DSM_FUN_IMPLEMENTED function

[Xiao Guangrong]

On 10/21/2015 06:49 PM, Stefan Hajnoczi wrote:
> On Wed, Oct 21, 2015 at 12:26:35AM +0800, Xiao Guangrong wrote:
> > On 10/20/2015 11:51 PM, Stefan Hajnoczi wrote:
> > > On Mon, Oct 19, 2015 at 08:54:14AM +0800, Xiao Guangrong wrote:
> > > > +exit:
> > > > +    /* Write our output result to dsm memory. */
> > > > +    ((dsm_out *)dsm_ram_addr)->len = out->len;
> > >
> > > Missing byteswap?
> > >
> > > I thought you were going to remove this field because it wasn't needed
> > > by the guest.
> >
> > The @len is the size of _DSM result buffer, for example, for the function of
> > DSM_FUN_IMPLEMENTED the result buffer is 8 bytes, and for
> > DSM_DEV_FUN_NAMESPACE_LABEL_SIZE the buffer size is 4 bytes. It tells ASL code
> > how much size of memory we need to return to the _DSM caller.
> >
> > In _DSM code, it's handled like this:
> >
> > "RLEN" is @len, “OBUF” is the left memory in DSM page.
> >
> >          /* get @len*/
> >          aml_append(method, aml_store(aml_name("RLEN"), aml_local(6)));
> >          /* @len << 3 to get bits. */
> >          aml_append(method, aml_store(aml_shiftleft(aml_local(6),
> >                                         aml_int(3)), aml_local(6)));
> >
> >          /* get @len << 3 bits from OBUF, and return it to the caller. */
> >          aml_append(method, aml_create_field(aml_name("ODAT"), aml_int(0),
> >                                              aml_local(6) , "OBUF"));
> >
> > Since @len is our internally used, it's not return to guest, so i did not do
> > byteswap here.
>
> I am not familiar with the ACPI details, but I think this emits bytecode
> that will be run by the guest's ACPI interpreter?
>
> You still need to define the endianness of fields since QEMU and the
> guest could have different endianness.
>
> In other words, will the following work if a big-endian ppc host is
> running a little-endian x86 guest?
>
>    ((dsm_out *)dsm_ram_addr)->len = out->len;

Er... If we do byteswap in QEMU then it is also needed in ASL code, however,
ASL lacks this kind of instruction.  I guess ACPI interpreter is smart enough
to change value to Littel-Endian for all 2 bytes / 4 bytes / 8 bytes accesses

I will do the change in next version, thanks for you pointing it out, Stefan!