On 6/27/2016 2:58 PM, Dan Williams wrote: > On Mon, Jun 27, 2016 at 11:47 AM, Linda Knippers <linda.knipp...@hpe.com> > wrote: >> >> >> On 6/27/2016 2:06 PM, Dan Williams wrote: >>> On Mon, Jun 27, 2016 at 10:40 AM, Linda Knippers <linda.knipp...@hpe.com> >>> 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.
I'm not actually arguing against this fix. I'm arguing that we should support function 0 more generally. -- ljk _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
