From: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: "peter.maydell@linaro.org" <peter.maydell@linaro.org>,
"drjones@redhat.com" <drjones@redhat.com>,
"xiaoguangrong.eric@gmail.com" <xiaoguangrong.eric@gmail.com>,
"shannon.zhaosl@gmail.com" <shannon.zhaosl@gmail.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
Linuxarm <linuxarm@huawei.com>,
Auger Eric <eric.auger@redhat.com>,
"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
"xuwei \(O\)" <xuwei5@huawei.com>,
Jonathan Cameron <jonathan.cameron@huawei.com>,
"lersek@redhat.com" <lersek@redhat.com>
Subject: RE: [PATCH 0/5] ARM virt: Add NVDIMM support
Date: Mon, 13 Jan 2020 13:11:53 +0000 [thread overview]
Message-ID: <5db1e82970fc4e82a32e9ede4d8d40d0@huawei.com> (raw)
In-Reply-To: <20200109181300.00238828@redhat.com>
> -----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
next prev parent reply other threads:[~2020-01-13 13:13 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 15:52 [PATCH 0/5] ARM virt: Add NVDIMM support Shameer Kolothum
2019-10-04 15:52 ` [PATCH 1/5] hw/arm: Align ACPI blob len to PAGE size Shameer Kolothum
2019-11-08 16:17 ` Igor Mammedov
2019-11-11 12:47 ` Shameerali Kolothum Thodi
2019-12-09 13:04 ` Shameerali Kolothum Thodi
2019-10-04 15:52 ` [PATCH 2/5] nvdimm: Use configurable ACPI IO base and size Shameer Kolothum
2019-11-11 14:24 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 3/5] hw/arm/virt: Add nvdimm hot-plug infrastructure Shameer Kolothum
2019-11-11 14:38 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 4/5] hw/arm/boot: Expose the pmem nodes in the DT Shameer Kolothum
2019-11-11 14:46 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 5/5] hw/arm/virt: Add nvdimm hotplug support Shameer Kolothum
2019-11-12 13:01 ` Igor Mammedov
2019-10-18 16:39 ` [PATCH 0/5] ARM virt: Add NVDIMM support Auger Eric
2019-10-22 14:05 ` Shameerali Kolothum Thodi
2019-11-25 13:20 ` Shameerali Kolothum Thodi
2019-11-25 15:45 ` Igor Mammedov
2019-11-25 16:25 ` Shameerali Kolothum Thodi
2019-11-26 8:56 ` Igor Mammedov
2019-11-26 9:46 ` Andrew Jones
2019-11-28 12:36 ` Shameerali Kolothum Thodi
2019-12-09 17:39 ` Shameerali Kolothum Thodi
2019-12-11 7:57 ` Igor Mammedov
2019-12-13 12:52 ` Shameerali Kolothum Thodi
2020-01-06 17:06 ` Shameerali Kolothum Thodi
2020-01-09 17:13 ` Igor Mammedov
2020-01-13 13:11 ` Shameerali Kolothum Thodi [this message]
2019-11-12 14:39 ` Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5db1e82970fc4e82a32e9ede4d8d40d0@huawei.com \
--to=shameerali.kolothum.thodi@huawei.com \
--cc=drjones@redhat.com \
--cc=eric.auger@redhat.com \
--cc=imammedo@redhat.com \
--cc=jonathan.cameron@huawei.com \
--cc=lersek@redhat.com \
--cc=linuxarm@huawei.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=shannon.zhaosl@gmail.com \
--cc=xiaoguangrong.eric@gmail.com \
--cc=xuwei5@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).