Re: [PATCH v2 1/3] ACPI/NFIT: Update Control Region Structure to comply ACPI 6.1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2016-03-01 at 17:37 +0000, Moore, Robert wrote:
> 
> > -----Original Message-----
> > From: Toshi Kani [mailto:toshi.kani@xxxxxxx]
> > Sent: Tuesday, March 01, 2016 9:37 AM
> > To: Moore, Robert; rjw@xxxxxxxxxxxxx; Williams, Dan J
> > Cc: Zheng, Lv; elliott@xxxxxxx; linux-nvdimm@xxxxxxxxxxxx; linux-
> > acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx
> > Subject: Re: [PATCH v2 1/3] ACPI/NFIT: Update Control Region Structure
> > to
> > comply ACPI 6.1
> > 
> > On Tue, 2016-03-01 at 16:03 +0000, Moore, Robert wrote:
> > > We have a bunch of macros in include/acmacros.h -- like this:
> > > 
> > > ACPI_MOVE_16_TO_16(d, s)
> > 
> > There is a problem in using the ACPICA byte-swap macros.  ACPI is
> > little-endian arch, so the macros are set to perform byte-swappings
> > when the CPU arch is big-endian.  This case, however, is the other way
> > around.  The fields in question are defined & stored as arrays of
> > bytes.
> 
> That's not what I see in the ACPI spec. The fields are defined like any
> other ACPI table.
> 
> Vendor ID 2 6 
> Identifier indicating the vendor of the NVDIMM.
> This field shall be set to the value of the NVDIMM SPD Module
> Manufacturer ID Code field a with byte 0 set to DDR4 SPD byte
> 320 and byte 1 set to DDR4 SPD byte 321.

They are different from other fields because the spec defines "byte
locations" of the fields.  The above case, Vendor ID, is defined that:
 - byte 0 set to DDR4 SPD byte 320
 - byte 1 set to DDR4 SPD byte 321

For instance, if SPD byte 320 is 0x12 and 321 is 0x34, then this field is
stored as (0x12, 0x34).  If you read this field as a 16-bit int value, you
will get 0x3412 on little-endian CPUs, such as x86.

Section 5.2.25.9 of ACPI 6.1 further clarifies that Vendor ID needs to be
represented as ("%02x%02x", byte0, byte1), which is "1234" in this case.

So, we will need to byte-swap when it is handled as a 16-bit int value on
little-endian CPUs.  This is different from other fields with multi-byte
numeric values, which are stored in little-endian format in ACPI.

Hence, my patch avoids this byte-swapping as I described in the change log
below.

|  - Change 'struct acpi_nfit_control_region' to reflect the update.
|    SPD IDs are defined as arrays of bytes, so that they can be
|    treated in the same way regardless of CPU endianness and are
|    not miss-treated as little-endian numeric values.

I hope this makes it clear now.

> > Another issue is that it is not clear who needs to perform the byte-
> > swapping among ACPICA and drivers.  If ACPICA, drivers must agree that
>
> ACPICA does not ever do anything with the "data tables" like NFIT, other
> than handing off the table when requested by a driver.

So, this means that the alternative is to change the NFIT driver to do the
byte-swapping for the fields when the CPU arch is little-endian.  If we
cannot change the structure, we will have to take this course...

Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux