On Tue, Jun 28, 2016 at 05:37:45PM +0800, Xiao Guangrong wrote:
On 06/28/2016 02:58 AM, Dan Williams wrote:
> On Mon, Jun 27, 2016 at 11:47 AM, Linda Knippers <linda.knippers(a)hpe.com>
> > On 6/27/2016 2:06 PM, Dan Williams wrote:
> > > On Mon, Jun 27, 2016 at 10:40 AM, Linda Knippers
> > > >
> > > > On 6/24/2016 1:44 PM, Dan Williams wrote:
> > > > > QEMU 2.6 implements nascent support for nvdimm DSMs. Depending
> > > > > configuration it may only implement the function0 dsm to
> > > > > no other DSMs are available. Commit 31eca76ba2fc "nfit,
> > > > > limited/whitelisted dimm command marshaling mechanism"
breaks QEMU, but
> > > > > QEMU is spec compliant. Per the spec the way to indicate that
> > > > > 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
> > > > > there is support for any functions other than function 0
> > > > > specified UUID and Revision ID. If set to zero, no
> > > > > supported (other than function zero) for the specified UUID
> > > > > Revision ID.
> > > >
> > > > The rest of that paragraph is:
> > > >
> > > > If set to one, at least one additional function is supported. For all
> > > > 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
> > > > Revision ID.)
> > > >
> > > > >
> > > > > Update the nfit driver to determine the family (interface UUID)
> > > > > requiring the implementation to define any other functions,
> > > > > short-circuit acpi_check_dsm() to succeed per the spec. The
> > > > > appears to be the only user passing funcs==0 to
> > > > > this behavior change of the common routine should be limited to
> > > > > 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
> > > > That means returning the correct value for all spec compliant
> > > > 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 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)".
The currently upstream version of acpi_check_dsm doesn't handle this
case, it assumes at least one function other than zero is also being
returned (as it assumes bit 0 is set.)
Instead of the proposed change we should have something like:
index 22c0995..3ecf462 100644
@@ -702,6 +702,9 @@ bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64
if ((mask & 0x1) && (mask & funcs) == funcs)
+ if ((mask == 0) && (funcs == 1))
+ return true;
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jerry Hoemann Software Engineer Hewlett Packard Enterprise