Re: [PATCH] RFC: platform/x86: wmi: Fix check for method instance number

From: Pali RohÃr
Date: Tue Jun 13 2017 - 14:05:06 EST


On Tuesday 13 June 2017 18:49:51 Darren Hart wrote:
> On Sat, Jun 10, 2017 at 09:15:57PM +0200, Pali RohÃr wrote:
> > On Saturday 27 May 2017 13:55:34 Pali RohÃr wrote:
> > > instance_count defines number of instances of data block and
> > > instance itself is indexed from zero, which means first instance
> > > has number 0. Therefore check for invalid instance should be
> > > non-strict inequality.
> > >
> > > Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
> > > ---
> > > I'm marking this patch as RFC because it is not tested at all and
> > > probably could break existing WMI drivers. Some WMI drivers pass
> > > instance number 1 and I'm not sure if ACPI-WMI bytecode for those
> > > machines has really two instances. In more cases ACPI-WMI
> > > bytecode does not check instance number if supports only one
> > > instance. So then any instance id can be used for correct
> > > execution of ACPI-WMI method.
> > >
> > > So this patch is open for discussion.
> >
> > Hi! Any comments?
>
> Hi Pali,
>
> This change appears correct to me, but your comment about this
> parameter being ignored by ACPI-WMI is definitely concerning. Since
> this doesn't address a specific failure report, and it has the
> potential to break functional drivers, I wouldn't want to merge it
> without some evidence that those drivers still work.

I agree that it should not be merged without any other testing or
discussion. Reason why I marked it as RFC.

ACPI bytecode (which implements WMI functions) is often ignoring
instance method if there is only one instance. So it does not have to
decide which instance to call based on parameter.

IIRC it is also stated in that MSDN documentation.

> I'd suggest reaching out to the maintainers and contributors to the
> drivers you mention to request some help in testing.

Seems sane. Grep for all methods with instance number different as zero
(or just number one -- which can be suspicious as somebody could thought
that indexing is from one, not zer) and try to receive ACPI/BMOF data
and verify it.

--
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.