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 Tue, Jul 19, 2016 at 11:50 AM, Linda Knippers <linda.knippers@xxxxxxx> wrote:
>
>
> On 7/19/2016 1:11 PM, Jerry Hoemann wrote:
>> On Fri, Jun 24, 2016 at 10:44:25AM -0700, 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.
>>
>>
>> Dan,
>>
>> This breaks calling DSM on HPE platforms and is a regression.
>>
>> E-mail context can be found at:
>>
>>       https://lists.01.org/pipermail/linux-nvdimm/2016-June/006099.html
>>
>> The problem with this change is that it assumes that ACPI returning an
>> object means that the UUID is supported on that platform.
>>
>> However, looking at ACPI v 6.1 section 9.1.1, the example for
>> evaluating a _DSM shows that if the UUID is not supported at all,
>> a zeroed out buffer of length 1 is returned:
>>
>>       //
>>       // If not one of the UUIDs we recognize, then return a buffer
>>       // with bit 0 set to 0 indicating no functions supported.
>>       //
>>       return(Buffer(){0})
>>
>> HPE firmware has been following this practice for a long long time.
>> I suspect other manufacturer's firmware do so as well.
>>
>> The problem is that this creates an ambiguity and the linux code
>> is no longer differentiating the case where the DSM/UUID is
>> supported but only implements function 0 (the QEMU case you're
>> trying to accommodate) and the case that the DSM/UUID is not
>> supported at all.
>>
>> The result is that the code in acpi_nfit_add_dimm:
>>
>>       for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_HPE2; i++)
>> -             if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1))
>> +             if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 0))
>>                       break;
>>
>>
>> always matches NVDIMM_FAMILY_INTEL.  This is either because its
>> is an Intel nvdimm, or because the unsupported UUID returns back
>> a zeroed out buffer of length 1.
>>
>>
>> As nfit_mem->family always equals NVDIMM_FAMILY_INTEL, no subsequent
>> DSM call will succeed for NVDIMM_FAMILY_HPE1 or any other
>> family.
>>
>> I don't have a fix as of yet, but wanted to make you aware of
>> the problem.
>
> Could we try the all known UUIDs looking for one that returns a non-zero
> value?
>

Yes, that seems like the way forward, and also make not finding a DSM
family non-fatal.
--
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