> -----Original Message----- > From: Qemu-devel > [mailto:qemu-devel-bounces+shameerali.kolothum.thodi=huawei.com@nongn > u.org] On Behalf Of Igor Mammedov > Sent: 09 January 2020 17:13 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: peter.maydell@linaro.org; drjones@redhat.com; > xiaoguangrong.eric@gmail.com; Auger Eric <eric.auger@redhat.com>; > qemu-devel@nongnu.org; Linuxarm <linuxarm@huawei.com>; > shannon.zhaosl@gmail.com; qemu-arm@nongnu.org; xuwei (O) > <xuwei5@huawei.com>; Jonathan Cameron > <jonathan.cameron@huawei.com>; lersek@redhat.com > Subject: Re: [PATCH 0/5] ARM virt: Add NVDIMM support > > On Mon, 6 Jan 2020 17:06:32 +0000 > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > Hi Igor, [...] > > (+Jonathan) > > > > Thanks to Jonathan for taking a fresh look at this issue and spotting this, > > > https://elixir.bootlin.com/linux/v5.5-rc5/source/drivers/acpi/acpica/utmisc.c#L > 109 > > > > And, from ACPI 6.3, table 19-419 > > > > "If the Buffer Field is smaller than or equal to the size of an Integer (in bits), it > > will be treated as an Integer. Otherwise, it will be treated as a Buffer. The > size > > of an Integer is indicated by the Definition Block table header's Revision field. > > A Revision field value less than 2 indicates that the size of an Integer is 32 > bits. > > A value greater than or equal to 2 signifies that the size of an Integer is 64 > bits." > > > > It looks like the main reason for the difference in behavior of the buffer object > > size between x86 and ARM/virt, is because of the Revision number used in > the > > DSDT table. On x86 it is 1 and ARM/virt it is 2. > > > > So most likely, > > > > > CreateField (ODAT, Zero, Local1, OBUF) > > You are right, that's where it goes wrong, since OBUF > is implicitly converted to integer if size is less than 64bits. > > > > Concatenate (Buffer (Zero){}, OBUF, Local7) > > see more below > > [...] > > > > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > > index 64eacfad08..621f9ffd41 100644 > > --- a/hw/acpi/nvdimm.c > > +++ b/hw/acpi/nvdimm.c > > @@ -1192,15 +1192,18 @@ static void nvdimm_build_fit(Aml *dev) > > aml_append(method, ifctx); > > > > aml_append(method, aml_store(aml_sizeof(buf), buf_size)); > > - aml_append(method, aml_subtract(buf_size, > > - aml_int(4) /* the size of > "STAU" */, > > - buf_size)); > > > > /* if we read the end of fit. */ > > - ifctx = aml_if(aml_equal(buf_size, aml_int(0))); > > + ifctx = aml_if(aml_equal(aml_subtract(buf_size, > > + aml_sizeof(aml_int(0)), NULL), > > + aml_int(0))); > > aml_append(ifctx, aml_return(aml_buffer(0, NULL))); > > aml_append(method, ifctx); > > > > + aml_append(method, aml_subtract(buf_size, > > + aml_int(4) /* the size of > "STAU" */, > > + buf_size)); > > + > > aml_append(method, aml_create_field(buf, > > aml_int(4 * BITS_PER_BYTE), /* offset > at byte 4.*/ > > aml_shiftleft(buf_size, aml_int(3)), > "BUFF")); > > Instead of covering up error in NCAL, I'd prefer original issue fixed. > How about something like this pseudocode: > > NTFI = Local6 > Local1 = (RLEN - 0x04) > - Local1 = (Local1 << 0x03) > - CreateField (ODAT, Zero, Local1, OBUF) > - Concatenate (Buffer (Zero) {}, OBUF, Local7) > > If (Local1 < IntegerSize) > { > Local7 = Buffer(0) // output buffer > Local8 = 0 // index for being copied byte > // build byte by byte output buffer > while (Local8 < Local1) { > Local9 = Buffer(1) > // copy 1 byte at Local8 offset from ODAT to > temporary buffer Local9 > Store(DeRef(Index(ODAT, Local8)), Index(Local9, > 0)) > Concatenate (Local7, Local9, Local7) > Increment(Local8) > } > return Local7 > } else { > CreateField (ODAT, Zero, Local1, OBUF) > return OBUF > } > Ok. This looks much better. I will test this and sent out a v2 soon addressing other comments on this series as well. Thanks, Shameer
© 2016 - 2024 Red Hat, Inc.