Re: [PATCH] acpi, nfit: fix acpi_check_dsm() vs zero functions implemented

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

 





On 06/28/2016 02:58 AM, Dan Williams wrote:
On Mon, Jun 27, 2016 at 11:47 AM, Linda Knippers <linda.knippers@xxxxxxx> wrote:


On 6/27/2016 2:06 PM, Dan Williams wrote:
On Mon, Jun 27, 2016 at 10:40 AM, Linda Knippers <linda.knippers@xxxxxxx> wrote:

On 6/24/2016 1:44 PM, Dan Williams wrote:
QEMU 2.6 implements nascent support for nvdimm DSMs. Depending on
configuration it may only implement the function0 dsm to indicate that
no other DSMs are available. Commit 31eca76ba2fc "nfit, libnvdimm:
limited/whitelisted dimm command marshaling mechanism" breaks QEMU, but
QEMU is spec compliant.  Per the spec the way to indicate that no
functions are supported is:

     If Function Index is zero, the return is a buffer containing one bit
     for each function index, starting with zero. Bit 0 indicates whether
     there is support for any functions other than function 0 for the
     specified UUID and Revision ID. If set to zero, no functions are
     supported (other than function zero) for the specified UUID and
     Revision ID.

The rest of that paragraph is:

If set to one, at least one additional function is supported. For all other bits
in the buffer, a bit is set to zero to indicate if that function index is not
supported for the specific UUID and Revision ID. (For example, bit 1 set to 0
indicates that function index 1 is not supported for the specific UUID and
Revision ID.)


Update the nfit driver to determine the family (interface UUID) without
requiring the implementation to define any other functions, i.e.
short-circuit acpi_check_dsm() to succeed per the spec.  The nfit driver
appears to be the only user passing funcs==0 to acpi_check_dsm(), so
this behavior change of the common routine should be limited to the
probing done by the nfit driver.

I don't understand why we're special casing this to support QEMU only when
there are no DSM functions supported.  If we want to implement the
spec and support function zero, I think we should support it correctly.
That means returning the correct value for all spec compliant callers,
even when there are functions that are supported.

QEMU 2.6 already shipped, so whatever we do we should not regress
those binaries.  The QEMU behavior could be argued as not spec
compliant, but they've implemented enough of function0 to answer the
"which family" probe.

How would you argue that?

acpi_evaluate_dsm() returns data instead of an error.

Yes, if an implementation supports function0 it
should say so in the returned bitmask,

But in other places you explicitly prevent function 0 from
being executed.

Right, for the whitelist-filtered result to userspace, but this patch
is about the initial kernel probe.

but by the time we've
determined that function0 is "not supported" we've already
successfully executed a function0 request.

If they advertise a _DSM, I think they have to support function 0.

They should, but didn't.  Kernel v4.6 works with qemu 2.6, kernel v4.7
does not, so the kernel needs to be fixed.


Sorry.

I do not know why you guys think QEMU does not support function 0. It
already returns 0 to indicates "no functions are supported
(other than function zero)".




--
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