Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1622232 > unrolled thread

[PATCH] ACPICA: Export mutex functions

Started byGuenter Roeck <linux@roeck-us.net>
First post2017-04-12 17:20 +0200
Last post2017-04-18 01:40 +0200
Articles 13 on this page of 33 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-12 17:20 +0200
    RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-12 17:40 +0200
      Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-12 23:30 +0200
        RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-13 00:00 +0200
          Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-13 03:00 +0200
          Re: [PATCH] ACPICA: Export mutex functions "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-15 00:40 +0200
            RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-15 01:40 +0200
        RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-17 11:40 +0200
          RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-17 11:50 +0200
            Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-17 16:10 +0200
              RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 01:40 +0200
          Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-17 18:00 +0200
            RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-17 19:20 +0200
              RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-17 21:30 +0200
                Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-17 21:50 +0200
                  RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-17 22:50 +0200
                    Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-17 23:10 +0200
                      Re: [PATCH] ACPICA: Export mutex functions "Rafael J. Wysocki" <rafael@kernel.org> - 2017-04-17 23:30 +0200
                        Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-18 00:40 +0200
                          Re: [PATCH] ACPICA: Export mutex functions "Rafael J. Wysocki" <rafael@kernel.org> - 2017-04-18 01:00 +0200
                          RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 02:00 +0200
                            Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-18 06:40 +0200
                              RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 09:10 +0200
                              RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 09:20 +0200
                                Re: [PATCH] ACPICA: Export mutex functions Guenter Roeck <linux@roeck-us.net> - 2017-04-18 16:00 +0200
                                  Re: [PATCH] ACPICA: Export mutex functions "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-04-18 16:30 +0200
                                    RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-18 18:10 +0200
                                  RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-19 03:30 +0200
                                    RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-19 03:40 +0200
                  RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 02:00 +0200
                RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 01:50 +0200
              RE: [PATCH] ACPICA: Export mutex functions "Moore, Robert" <robert.moore@intel.com> - 2017-04-17 21:40 +0200
            RE: [PATCH] ACPICA: Export mutex functions "Zheng, Lv" <lv.zheng@intel.com> - 2017-04-18 01:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1624959

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 02:00 +0200
Message-ID<txoIO-7c0-29@gated-at.bofh.it>
In reply to#1624891
Hi,

> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > > On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
> > >>
> > >>
> > >> > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >> > Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >> >
> > >> > On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> > >> > >
> > >> > > > From: Moore, Robert
> > >> > > > Subject: RE: [PATCH] ACPICA: Export mutex functions
> > >> > > >
> > >> > > > There is a model for the drivers to directly acquire an AML mutex
> > >> > > > object. That is why the acquire/release public interfaces were added
> > >> > > > to ACPICA.
> > >> > > >
> > >> > > > I forget all of the details, but the model was developed with MS and
> > >> > > > others during the ACPI 6.0 timeframe.
> > >> > > >
> > >> > > >
> > >> > > [Moore, Robert]
> > >> > >
> > >> > >
> > >> > > Here is the case where the OS may need to directly acquire an AML
> > >> > mutex:
> > >> > >
> > >> > > From the ACPI spec:
> > >> > >
> > >> > > 19.6.2 Acquire (Acquire a Mutex)
> > >> > >
> > >> > > Note: For Mutex objects referenced by a _DLM object, the host OS may
> > >> > also contend for ownership.
> > >> > >
> > >> > From the context in the dsdt, and from description of expected use cases
> > >> > for _DLM objects I can find, this is what the mutex is used for (to
> > >> > serialize access to a resource on a low pin count serial interconnect,
> > >> > aka LPC).
> > >> >
> > >> > What does that mean in practice ? That I am not supposed to use it
> > >> > because it doesn't follow standard ACPI mutex declaration rules ?
> > >> >
> > >> > Thanks,
> > >> > Guenter
> > >> >
> > >> > >
> > >> [Moore, Robert]
> > >>
> > >> I'm not an expert on the _DLM method, but I would point you to the description section in the
> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
> > >>
> > >
> > > I did. However, not being an ACPI expert, that doesn't tell me anything.
> >
> > Basically, if the kernel and AML need to access a device concurrently,
> > there should be a _DLM object under that device in the ACPI tables.
> > In that case it is expected to return a list of (AML) mutexes that can
> > be acquired by the kernel in order to synchronize device access with
> > respect to AML (and for each mutex it may also return a description of
> > the specific resources to be protected by it).
> >
> > Bottom line: without _DLM, the kernel cannot synchronize things with
> > respect to AML properly, because it has no information how to do that
> > then.
> 
> That is all quite interesting. I do see the mutex in question used on various
> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
> Intel). Interestingly, the naming seems to be consistent - it is always named
> "MUT0". For the most part, it seems to be available on more recent
> motherboards; older motherboards tend to use the resource without locking.
> However, I don't see any mention of "_DLM" in any of the DSDTs.
> 

OK, then you might be having problems in your opregion driver.

> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
> drivers, but also in parallel port and infrared driver code. Effectively
> that means that all this code is inherently unsafe on systems with ACPI
> support.
> 
> I had thought about implementing a set of utility functions to make the kernel
> code safer to use if the mutex is found to exist.

As what you've mentioned, there are already lots of parallel accesses in kernel without enabling ACPI.
Are these accesses mutually exclusive (safe)?
If so, why do you need to invent a new synchronization mechanism?

> Right now I wonder, though,
> if such code would have a chance to be accepted. Any thoughts on that ?

Is that possible to make it safe in the opregion driver?

Thanks
Lv

[toc] | [prev] | [next] | [standalone]

#1625013

FromGuenter Roeck <linux@roeck-us.net>
Date2017-04-18 06:40 +0200
Message-ID<txt5L-1FB-1@gated-at.bofh.it>
In reply to#1624959
On 04/17/2017 04:53 PM, Zheng, Lv wrote:
> Hi,
>
>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>> Subject: Re: [PATCH] ACPICA: Export mutex functions
>>
>> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
>>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
>>>>>
>>>>>
>>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
>>>>>>
>>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
>>>>>>>
>>>>>>>> From: Moore, Robert
>>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
>>>>>>>>
>>>>>>>> There is a model for the drivers to directly acquire an AML mutex
>>>>>>>> object. That is why the acquire/release public interfaces were added
>>>>>>>> to ACPICA.
>>>>>>>>
>>>>>>>> I forget all of the details, but the model was developed with MS and
>>>>>>>> others during the ACPI 6.0 timeframe.
>>>>>>>>
>>>>>>>>
>>>>>>> [Moore, Robert]
>>>>>>>
>>>>>>>
>>>>>>> Here is the case where the OS may need to directly acquire an AML
>>>>>> mutex:
>>>>>>>
>>>>>>> From the ACPI spec:
>>>>>>>
>>>>>>> 19.6.2 Acquire (Acquire a Mutex)
>>>>>>>
>>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
>>>>>> also contend for ownership.
>>>>>>>
>>>>>> From the context in the dsdt, and from description of expected use cases
>>>>>> for _DLM objects I can find, this is what the mutex is used for (to
>>>>>> serialize access to a resource on a low pin count serial interconnect,
>>>>>> aka LPC).
>>>>>>
>>>>>> What does that mean in practice ? That I am not supposed to use it
>>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
>>>>>>
>>>>>> Thanks,
>>>>>> Guenter
>>>>>>
>>>>>>>
>>>>> [Moore, Robert]
>>>>>
>>>>> I'm not an expert on the _DLM method, but I would point you to the description section in the
>> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
>>>>>
>>>>
>>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
>>>
>>> Basically, if the kernel and AML need to access a device concurrently,
>>> there should be a _DLM object under that device in the ACPI tables.
>>> In that case it is expected to return a list of (AML) mutexes that can
>>> be acquired by the kernel in order to synchronize device access with
>>> respect to AML (and for each mutex it may also return a description of
>>> the specific resources to be protected by it).
>>>
>>> Bottom line: without _DLM, the kernel cannot synchronize things with
>>> respect to AML properly, because it has no information how to do that
>>> then.
>>
>> That is all quite interesting. I do see the mutex in question used on various
>> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
>> Intel). Interestingly, the naming seems to be consistent - it is always named
>> "MUT0". For the most part, it seems to be available on more recent
>> motherboards; older motherboards tend to use the resource without locking.
>> However, I don't see any mention of "_DLM" in any of the DSDTs.
>>
>
> OK, then you might be having problems in your opregion driver.
>
>> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
>> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
>> drivers, but also in parallel port and infrared driver code. Effectively
>> that means that all this code is inherently unsafe on systems with ACPI
>> support.
>>
>> I had thought about implementing a set of utility functions to make the kernel
>> code safer to use if the mutex is found to exist.
>
> As what you've mentioned, there are already lots of parallel accesses in kernel without enabling ACPI.
> Are these accesses mutually exclusive (safe)?

In-kernel, yes (using request_muxed_region). Against ACPI, no.

> If so, why do you need to invent a new synchronization mechanism?
>

Because I am seeing a problem with the current code (more specifically,
with the it87 hwmon driver) on new Gigabyte boards.

>> Right now I wonder, though,
>> if such code would have a chance to be accepted. Any thoughts on that ?
>
> Is that possible to make it safe in the opregion driver?
>

Sorry, I don't think I understand what you mean with "opregion driver".
Do you refer to the drivers accessing the memory region in question,
or in other words replicating the necessary code in every driver accessing
that region ? Sure, I can do that, if that is the preferred solution;
I have no problem with that. However, that would require exporting
the ACPI mutex functions. My understanding is that you are opposed to
exporting those, so I assume that is not what you refer to.
Can you clarify ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1625090

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 09:10 +0200
Message-ID<txvqW-3eI-17@gated-at.bofh.it>
In reply to#1625013
Hi,

> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> On 04/17/2017 04:53 PM, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Guenter Roeck [mailto:linux@roeck-us.net]
> >> Subject: Re: [PATCH] ACPICA: Export mutex functions
> >>
> >> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
> >>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
> >>>>>
> >>>>>
> >>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> >>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> >>>>>>
> >>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> >>>>>>>
> >>>>>>>> From: Moore, Robert
> >>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
> >>>>>>>>
> >>>>>>>> There is a model for the drivers to directly acquire an AML mutex
> >>>>>>>> object. That is why the acquire/release public interfaces were added
> >>>>>>>> to ACPICA.
> >>>>>>>>
> >>>>>>>> I forget all of the details, but the model was developed with MS and
> >>>>>>>> others during the ACPI 6.0 timeframe.
> >>>>>>>>
> >>>>>>>>
> >>>>>>> [Moore, Robert]
> >>>>>>>
> >>>>>>>
> >>>>>>> Here is the case where the OS may need to directly acquire an AML
> >>>>>> mutex:
> >>>>>>>
> >>>>>>> From the ACPI spec:
> >>>>>>>
> >>>>>>> 19.6.2 Acquire (Acquire a Mutex)
> >>>>>>>
> >>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
> >>>>>> also contend for ownership.
> >>>>>>>
> >>>>>> From the context in the dsdt, and from description of expected use cases
> >>>>>> for _DLM objects I can find, this is what the mutex is used for (to
> >>>>>> serialize access to a resource on a low pin count serial interconnect,
> >>>>>> aka LPC).
> >>>>>>
> >>>>>> What does that mean in practice ? That I am not supposed to use it
> >>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Guenter
> >>>>>>
> >>>>>>>
> >>>>> [Moore, Robert]
> >>>>>
> >>>>> I'm not an expert on the _DLM method, but I would point you to the description section in the
> >> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
> >>>>>
> >>>>
> >>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
> >>>
> >>> Basically, if the kernel and AML need to access a device concurrently,
> >>> there should be a _DLM object under that device in the ACPI tables.
> >>> In that case it is expected to return a list of (AML) mutexes that can
> >>> be acquired by the kernel in order to synchronize device access with
> >>> respect to AML (and for each mutex it may also return a description of
> >>> the specific resources to be protected by it).
> >>>
> >>> Bottom line: without _DLM, the kernel cannot synchronize things with
> >>> respect to AML properly, because it has no information how to do that
> >>> then.
> >>
> >> That is all quite interesting. I do see the mutex in question used on various
> >> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
> >> Intel). Interestingly, the naming seems to be consistent - it is always named
> >> "MUT0". For the most part, it seems to be available on more recent
> >> motherboards; older motherboards tend to use the resource without locking.
> >> However, I don't see any mention of "_DLM" in any of the DSDTs.
> >>
> >
> > OK, then you might be having problems in your opregion driver.
> >
> >> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
> >> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
> >> drivers, but also in parallel port and infrared driver code. Effectively
> >> that means that all this code is inherently unsafe on systems with ACPI
> >> support.
> >>
> >> I had thought about implementing a set of utility functions to make the kernel
> >> code safer to use if the mutex is found to exist.
> >
> > As what you've mentioned, there are already lots of parallel accesses in kernel without enabling
> ACPI.
> > Are these accesses mutually exclusive (safe)?
> 
> In-kernel, yes (using request_muxed_region). Against ACPI, no.
> 
> > If so, why do you need to invent a new synchronization mechanism?
> >
> 
> Because I am seeing a problem with the current code (more specifically,
> with the it87 hwmon driver) on new Gigabyte boards.

I checked superio_enter()/superio_exit(), IMO, the mutual exclusion
might be handled using 1 of the following 2 solutions:

1. _DLM, then you can find superio related mutex from _DLM and
   acquire/release it in superio_enter()/superio_exit().
   You really need a set of new APIs to acquire the _DLM related mutex.
   If you don't have _DLM in your DSDT, directly exporting ACPICA mutex
   functions seem to be a reasonable solution.
2. Normally, AML developer should abstract superio accesses into customized
   opregion, then you can prepare a superio opregion driver.

> 
> >> Right now I wonder, though,
> >> if such code would have a chance to be accepted. Any thoughts on that ?
> >
> > Is that possible to make it safe in the opregion driver?
> >
> 
> Sorry, I don't think I understand what you mean with "opregion driver".
> Do you refer to the drivers accessing the memory region in question,
> or in other words replicating the necessary code in every driver accessing
> that region ? Sure, I can do that, if that is the preferred solution;
> I have no problem with that. However, that would require exporting
> the ACPI mutex functions. My understanding is that you are opposed to
> exporting those, so I assume that is not what you refer to.
> Can you clarify ?

I mean solution 2.
From it87_find() I really couldn't see a possibility to convert superio
accesses into opregions. Could you paste some example superio access AML
code from your DSDT here.

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1625094

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 09:20 +0200
Message-ID<txvAC-3i3-9@gated-at.bofh.it>
In reply to#1625013
Hi,

> From: Zheng, Lv
> Subject: RE: [PATCH] ACPICA: Export mutex functions
> 
> Hi,
> 
> > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > Subject: Re: [PATCH] ACPICA: Export mutex functions
> >
> > On 04/17/2017 04:53 PM, Zheng, Lv wrote:
> > > Hi,
> > >
> > >> From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >> Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >>
> > >> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
> > >>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > >>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
> > >>>>>
> > >>>>>
> > >>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >>>>>>
> > >>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> > >>>>>>>
> > >>>>>>>> From: Moore, Robert
> > >>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
> > >>>>>>>>
> > >>>>>>>> There is a model for the drivers to directly acquire an AML mutex
> > >>>>>>>> object. That is why the acquire/release public interfaces were added
> > >>>>>>>> to ACPICA.
> > >>>>>>>>
> > >>>>>>>> I forget all of the details, but the model was developed with MS and
> > >>>>>>>> others during the ACPI 6.0 timeframe.
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>> [Moore, Robert]
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> Here is the case where the OS may need to directly acquire an AML
> > >>>>>> mutex:
> > >>>>>>>
> > >>>>>>> From the ACPI spec:
> > >>>>>>>
> > >>>>>>> 19.6.2 Acquire (Acquire a Mutex)
> > >>>>>>>
> > >>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
> > >>>>>> also contend for ownership.
> > >>>>>>>
> > >>>>>> From the context in the dsdt, and from description of expected use cases
> > >>>>>> for _DLM objects I can find, this is what the mutex is used for (to
> > >>>>>> serialize access to a resource on a low pin count serial interconnect,
> > >>>>>> aka LPC).
> > >>>>>>
> > >>>>>> What does that mean in practice ? That I am not supposed to use it
> > >>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Guenter
> > >>>>>>
> > >>>>>>>
> > >>>>> [Moore, Robert]
> > >>>>>
> > >>>>> I'm not an expert on the _DLM method, but I would point you to the description section in the
> > >> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
> > >>>>>
> > >>>>
> > >>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
> > >>>
> > >>> Basically, if the kernel and AML need to access a device concurrently,
> > >>> there should be a _DLM object under that device in the ACPI tables.
> > >>> In that case it is expected to return a list of (AML) mutexes that can
> > >>> be acquired by the kernel in order to synchronize device access with
> > >>> respect to AML (and for each mutex it may also return a description of
> > >>> the specific resources to be protected by it).
> > >>>
> > >>> Bottom line: without _DLM, the kernel cannot synchronize things with
> > >>> respect to AML properly, because it has no information how to do that
> > >>> then.
> > >>
> > >> That is all quite interesting. I do see the mutex in question used on various
> > >> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
> > >> Intel). Interestingly, the naming seems to be consistent - it is always named
> > >> "MUT0". For the most part, it seems to be available on more recent
> > >> motherboards; older motherboards tend to use the resource without locking.
> > >> However, I don't see any mention of "_DLM" in any of the DSDTs.
> > >>
> > >
> > > OK, then you might be having problems in your opregion driver.
> > >
> > >> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
> > >> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
> > >> drivers, but also in parallel port and infrared driver code. Effectively
> > >> that means that all this code is inherently unsafe on systems with ACPI
> > >> support.
> > >>
> > >> I had thought about implementing a set of utility functions to make the kernel
> > >> code safer to use if the mutex is found to exist.
> > >
> > > As what you've mentioned, there are already lots of parallel accesses in kernel without enabling
> > ACPI.
> > > Are these accesses mutually exclusive (safe)?
> >
> > In-kernel, yes (using request_muxed_region). Against ACPI, no.
> >
> > > If so, why do you need to invent a new synchronization mechanism?
> > >
> >
> > Because I am seeing a problem with the current code (more specifically,
> > with the it87 hwmon driver) on new Gigabyte boards.
> 
> I checked superio_enter()/superio_exit(), IMO, the mutual exclusion
> might be handled using 1 of the following 2 solutions:
> 
> 1. _DLM, then you can find superio related mutex from _DLM and
>    acquire/release it in superio_enter()/superio_exit().
>    You really need a set of new APIs to acquire the _DLM related mutex.
>    If you don't have _DLM in your DSDT, directly exporting ACPICA mutex
>    functions seem to be a reasonable solution.
> 2. Normally, AML developer should abstract superio accesses into customized
>    opregion, then you can prepare a superio opregion driver.
> 
> >
> > >> Right now I wonder, though,
> > >> if such code would have a chance to be accepted. Any thoughts on that ?
> > >
> > > Is that possible to make it safe in the opregion driver?
> > >
> >
> > Sorry, I don't think I understand what you mean with "opregion driver".
> > Do you refer to the drivers accessing the memory region in question,
> > or in other words replicating the necessary code in every driver accessing
> > that region ? Sure, I can do that, if that is the preferred solution;
> > I have no problem with that. However, that would require exporting
> > the ACPI mutex functions. My understanding is that you are opposed to
> > exporting those, so I assume that is not what you refer to.
> > Can you clarify ?
> 
> I mean solution 2.

Maybe I should provide more detailed examples for this solution.

For example:
OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100)
Field (SIOT, ByteAcc, Lock, Preserve)
{
    FNC1, 8,
    FNC2, 8,
    ...
}

Acquire (MUX0)
Store (0, FNC1)
Release (MUX0)

Then you can call (let me use casual pseudo code)
acpi_install_operation_region(SuperIOAddressSpace, superio_opregion_handler) from OS side.
In its callback superio_opregion_handler(), you can:

superio_enter();
If (address == 0) {
   /* mean FNC1 */
   Perform the locked superior accesses
} else if (address == 1) {
   /* mean FNC2 */
   Perform the locked superior accesses
}
superio_exit();

Are there similar approach in your DSDT?

Thanks and best regards
Lv

> From it87_find() I really couldn't see a possibility to convert superio
> accesses into opregions. Could you paste some example superio access AML
> code from your DSDT here.
> 
> Thanks and best regards
> Lv

[toc] | [prev] | [next] | [standalone]

#1625338

FromGuenter Roeck <linux@roeck-us.net>
Date2017-04-18 16:00 +0200
Message-ID<txBPH-6OW-11@gated-at.bofh.it>
In reply to#1625094
On 04/18/2017 12:14 AM, Zheng, Lv wrote:
> Hi,
>
>> From: Zheng, Lv
>> Subject: RE: [PATCH] ACPICA: Export mutex functions
>>
>> Hi,
>>
>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
>>>
>>> On 04/17/2017 04:53 PM, Zheng, Lv wrote:
>>>> Hi,
>>>>
>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
>>>>>
>>>>> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
>>>>>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>>>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>>>>>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
>>>>>>>>>
>>>>>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
>>>>>>>>>>
>>>>>>>>>>> From: Moore, Robert
>>>>>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
>>>>>>>>>>>
>>>>>>>>>>> There is a model for the drivers to directly acquire an AML mutex
>>>>>>>>>>> object. That is why the acquire/release public interfaces were added
>>>>>>>>>>> to ACPICA.
>>>>>>>>>>>
>>>>>>>>>>> I forget all of the details, but the model was developed with MS and
>>>>>>>>>>> others during the ACPI 6.0 timeframe.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> [Moore, Robert]
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Here is the case where the OS may need to directly acquire an AML
>>>>>>>>> mutex:
>>>>>>>>>>
>>>>>>>>>> From the ACPI spec:
>>>>>>>>>>
>>>>>>>>>> 19.6.2 Acquire (Acquire a Mutex)
>>>>>>>>>>
>>>>>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
>>>>>>>>> also contend for ownership.
>>>>>>>>>>
>>>>>>>>> From the context in the dsdt, and from description of expected use cases
>>>>>>>>> for _DLM objects I can find, this is what the mutex is used for (to
>>>>>>>>> serialize access to a resource on a low pin count serial interconnect,
>>>>>>>>> aka LPC).
>>>>>>>>>
>>>>>>>>> What does that mean in practice ? That I am not supposed to use it
>>>>>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Guenter
>>>>>>>>>
>>>>>>>>>>
>>>>>>>> [Moore, Robert]
>>>>>>>>
>>>>>>>> I'm not an expert on the _DLM method, but I would point you to the description section in the
>>>>> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
>>>>>>>>
>>>>>>>
>>>>>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
>>>>>>
>>>>>> Basically, if the kernel and AML need to access a device concurrently,
>>>>>> there should be a _DLM object under that device in the ACPI tables.
>>>>>> In that case it is expected to return a list of (AML) mutexes that can
>>>>>> be acquired by the kernel in order to synchronize device access with
>>>>>> respect to AML (and for each mutex it may also return a description of
>>>>>> the specific resources to be protected by it).
>>>>>>
>>>>>> Bottom line: without _DLM, the kernel cannot synchronize things with
>>>>>> respect to AML properly, because it has no information how to do that
>>>>>> then.
>>>>>
>>>>> That is all quite interesting. I do see the mutex in question used on various
>>>>> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
>>>>> Intel). Interestingly, the naming seems to be consistent - it is always named
>>>>> "MUT0". For the most part, it seems to be available on more recent
>>>>> motherboards; older motherboards tend to use the resource without locking.
>>>>> However, I don't see any mention of "_DLM" in any of the DSDTs.
>>>>>
>>>>
>>>> OK, then you might be having problems in your opregion driver.
>>>>
>>>>> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
>>>>> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
>>>>> drivers, but also in parallel port and infrared driver code. Effectively
>>>>> that means that all this code is inherently unsafe on systems with ACPI
>>>>> support.
>>>>>
>>>>> I had thought about implementing a set of utility functions to make the kernel
>>>>> code safer to use if the mutex is found to exist.
>>>>
>>>> As what you've mentioned, there are already lots of parallel accesses in kernel without enabling
>>> ACPI.
>>>> Are these accesses mutually exclusive (safe)?
>>>
>>> In-kernel, yes (using request_muxed_region). Against ACPI, no.
>>>
>>>> If so, why do you need to invent a new synchronization mechanism?
>>>>
>>>
>>> Because I am seeing a problem with the current code (more specifically,
>>> with the it87 hwmon driver) on new Gigabyte boards.
>>
>> I checked superio_enter()/superio_exit(), IMO, the mutual exclusion
>> might be handled using 1 of the following 2 solutions:
>>
>> 1. _DLM, then you can find superio related mutex from _DLM and
>>    acquire/release it in superio_enter()/superio_exit().
>>    You really need a set of new APIs to acquire the _DLM related mutex.
>>    If you don't have _DLM in your DSDT, directly exporting ACPICA mutex
>>    functions seem to be a reasonable solution.
>> 2. Normally, AML developer should abstract superio accesses into customized
>>    opregion, then you can prepare a superio opregion driver.
>>
>>>
>>>>> Right now I wonder, though,
>>>>> if such code would have a chance to be accepted. Any thoughts on that ?
>>>>
>>>> Is that possible to make it safe in the opregion driver?
>>>>
>>>
>>> Sorry, I don't think I understand what you mean with "opregion driver".
>>> Do you refer to the drivers accessing the memory region in question,
>>> or in other words replicating the necessary code in every driver accessing
>>> that region ? Sure, I can do that, if that is the preferred solution;
>>> I have no problem with that. However, that would require exporting
>>> the ACPI mutex functions. My understanding is that you are opposed to
>>> exporting those, so I assume that is not what you refer to.
>>> Can you clarify ?
>>
>> I mean solution 2.
>
> Maybe I should provide more detailed examples for this solution.
>
> For example:
> OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100)
> Field (SIOT, ByteAcc, Lock, Preserve)
> {
>     FNC1, 8,
>     FNC2, 8,
>     ...
> }
>
> Acquire (MUX0)
> Store (0, FNC1)
> Release (MUX0)
>
> Then you can call (let me use casual pseudo code)
> acpi_install_operation_region(SuperIOAddressSpace, superio_opregion_handler) from OS side.
> In its callback superio_opregion_handler(), you can:
>
> superio_enter();
> If (address == 0) {
>    /* mean FNC1 */
>    Perform the locked superior accesses
> } else if (address == 1) {
>    /* mean FNC2 */
>    Perform the locked superior accesses
> }
> superio_exit();
>
> Are there similar approach in your DSDT?
>

Some snippets from the DSDT:

	Device (SIO1)
            {
         	Name (_HID, EisaId ("PNP0C02") /* PNP Motherboard Resources */)  // _HID: Hardware ID
         	Name (_UID, Zero)  // _UID: Unique ID
		...
		Mutex (MUT0, 0x00)
		Method (ENFG, 1, NotSerialized)
                     {
                         Acquire (MUT0, 0x0FFF)
                         INDX = 0x87
                         INDX = One
                         INDX = 0x55
                         If ((SP1O == 0x2E))
                         {
                             INDX = 0x55
                         }
                         Else
                         {
                             INDX = 0xAA
                         }

                         LDN = Arg0
                     }

                     Method (EXFG, 0, NotSerialized)
                     {
                         INDX = 0x02
                         DATA = 0x02
                         Release (MUT0)
                     }

		    OperationRegion (IOID, SystemIO, SP1O, 0x02)	/* SP1O is 0x2e */
                     Field (IOID, ByteAcc, NoLock, Preserve)
                     {
                         INDX,   8,
                         DATA,   8
                     }
		    ...

Example for use:
		Method (DCNT, 2, NotSerialized)
                     {
                         ENFG (CGLD (Arg0))
                         If (((DMCH < 0x04) && ((Local1 = (DMCH & 0x03)) != Zero)))
                         {
                             RDMA (Arg0, Arg1, Local1++)
                         }

                         ACTR = Arg1
                         Local1 = (IOAH << 0x08)
                         Local1 |= IOAL
                         RRIO (Arg0, Arg1, Local1, 0x08)
                         EXFG ()
                     }

Can there be more than one address space handler for a given region ?
Each driver accessing the resource would need that handler.

Thanks,
Guenter

> Thanks and best regards
> Lv
>
>> From it87_find() I really couldn't see a possibility to convert superio
>> accesses into opregions. Could you paste some example superio access AML
>> code from your DSDT here.
>>
>> Thanks and best regards
>> Lv
>

[toc] | [prev] | [next] | [standalone]

#1625354

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2017-04-18 16:30 +0200
Message-ID<txCiJ-7dQ-7@gated-at.bofh.it>
In reply to#1625338
On Tuesday, April 18, 2017 06:50:26 AM Guenter Roeck wrote:
> On 04/18/2017 12:14 AM, Zheng, Lv wrote:
> > Hi,

[cut]

> >
> > Maybe I should provide more detailed examples for this solution.
> >
> > For example:
> > OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100)
> > Field (SIOT, ByteAcc, Lock, Preserve)
> > {
> >     FNC1, 8,
> >     FNC2, 8,
> >     ...
> > }
> >
> > Acquire (MUX0)
> > Store (0, FNC1)
> > Release (MUX0)
> >
> > Then you can call (let me use casual pseudo code)
> > acpi_install_operation_region(SuperIOAddressSpace, superio_opregion_handler) from OS side.
> > In its callback superio_opregion_handler(), you can:
> >
> > superio_enter();
> > If (address == 0) {
> >    /* mean FNC1 */
> >    Perform the locked superior accesses
> > } else if (address == 1) {
> >    /* mean FNC2 */
> >    Perform the locked superior accesses
> > }
> > superio_exit();
> >
> > Are there similar approach in your DSDT?
> >
> 
> Some snippets from the DSDT:
> 
> 	Device (SIO1)
>             {
>          	Name (_HID, EisaId ("PNP0C02") /* PNP Motherboard Resources */)  // _HID: Hardware ID
>          	Name (_UID, Zero)  // _UID: Unique ID
> 		...
> 		Mutex (MUT0, 0x00)
> 		Method (ENFG, 1, NotSerialized)
>                      {
>                          Acquire (MUT0, 0x0FFF)
>                          INDX = 0x87
>                          INDX = One
>                          INDX = 0x55
>                          If ((SP1O == 0x2E))
>                          {
>                              INDX = 0x55
>                          }
>                          Else
>                          {
>                              INDX = 0xAA
>                          }
> 
>                          LDN = Arg0
>                      }
> 
>                      Method (EXFG, 0, NotSerialized)
>                      {
>                          INDX = 0x02
>                          DATA = 0x02
>                          Release (MUT0)
>                      }
> 
> 		    OperationRegion (IOID, SystemIO, SP1O, 0x02)	/* SP1O is 0x2e */
>                      Field (IOID, ByteAcc, NoLock, Preserve)
>                      {
>                          INDX,   8,
>                          DATA,   8
>                      }
> 		    ...
> 
> Example for use:
> 		Method (DCNT, 2, NotSerialized)
>                      {
>                          ENFG (CGLD (Arg0))
>                          If (((DMCH < 0x04) && ((Local1 = (DMCH & 0x03)) != Zero)))
>                          {
>                              RDMA (Arg0, Arg1, Local1++)
>                          }
> 
>                          ACTR = Arg1
>                          Local1 = (IOAH << 0x08)
>                          Local1 |= IOAL
>                          RRIO (Arg0, Arg1, Local1, 0x08)
>                          EXFG ()
>                      }
> 
> Can there be more than one address space handler for a given region ?
> Each driver accessing the resource would need that handler.

Rather, every driver accessing the resource would need to be aware of the
existence of the operation region handler and would need to use the mutual
exclusion mechanism used by that handler, if my understanding here is correct.

The existence of an operation region for a specific section of address space is
a declaration that AML is going to access locations in that section.  It allows
the OS to install a handler for that region to intercept AML accesses and do
what it likes with them.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1625423

From"Moore, Robert" <robert.moore@intel.com>
Date2017-04-18 18:10 +0200
Message-ID<txDRw-8e8-3@gated-at.bofh.it>
In reply to#1625354

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Tuesday, April 18, 2017 7:15 AM
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: Zheng, Lv <lv.zheng@intel.com>; Rafael J. Wysocki
> <rafael@kernel.org>; Moore, Robert <robert.moore@intel.com>; Wysocki,
> Rafael J <rafael.j.wysocki@intel.com>; Len Brown <lenb@kernel.org>;
> linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> kernel@vger.kernel.org; Box, David E <david.e.box@intel.com>
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> On Tuesday, April 18, 2017 06:50:26 AM Guenter Roeck wrote:
> > On 04/18/2017 12:14 AM, Zheng, Lv wrote:
> > > Hi,
> 
> [cut]
> 
> > >
> > > Maybe I should provide more detailed examples for this solution.
> > >
> > > For example:
> > > OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100) Field (SIOT,
> > > ByteAcc, Lock, Preserve) {
> > >     FNC1, 8,
> > >     FNC2, 8,
> > >     ...
> > > }
> > >
> > > Acquire (MUX0)
> > > Store (0, FNC1)
> > > Release (MUX0)
> > >
> > > Then you can call (let me use casual pseudo code)
> > > acpi_install_operation_region(SuperIOAddressSpace,
> superio_opregion_handler) from OS side.
> > > In its callback superio_opregion_handler(), you can:
> > >
> > > superio_enter();
> > > If (address == 0) {
> > >    /* mean FNC1 */
> > >    Perform the locked superior accesses } else if (address == 1) {
> > >    /* mean FNC2 */
> > >    Perform the locked superior accesses } superio_exit();
> > >
> > > Are there similar approach in your DSDT?
> > >
> >
> > Some snippets from the DSDT:
> >
> > 	Device (SIO1)
> >             {
> >          	Name (_HID, EisaId ("PNP0C02") /* PNP Motherboard
> Resources */)  // _HID: Hardware ID
> >          	Name (_UID, Zero)  // _UID: Unique ID
> > 		...
> > 		Mutex (MUT0, 0x00)
> > 		Method (ENFG, 1, NotSerialized)
> >                      {
> >                          Acquire (MUT0, 0x0FFF)
> >                          INDX = 0x87
> >                          INDX = One
> >                          INDX = 0x55
> >                          If ((SP1O == 0x2E))
> >                          {
> >                              INDX = 0x55
> >                          }
> >                          Else
> >                          {
> >                              INDX = 0xAA
> >                          }
> >
> >                          LDN = Arg0
> >                      }
> >
> >                      Method (EXFG, 0, NotSerialized)
> >                      {
> >                          INDX = 0x02
> >                          DATA = 0x02
> >                          Release (MUT0)
> >                      }
> >
> > 		    OperationRegion (IOID, SystemIO, SP1O, 0x02)	/* SP1O
> is 0x2e */
> >                      Field (IOID, ByteAcc, NoLock, Preserve)
> >                      {
> >                          INDX,   8,
> >                          DATA,   8
> >                      }
> > 		    ...
> >
> > Example for use:
> > 		Method (DCNT, 2, NotSerialized)
> >                      {
> >                          ENFG (CGLD (Arg0))
> >                          If (((DMCH < 0x04) && ((Local1 = (DMCH &
> 0x03)) != Zero)))
> >                          {
> >                              RDMA (Arg0, Arg1, Local1++)
> >                          }
> >
> >                          ACTR = Arg1
> >                          Local1 = (IOAH << 0x08)
> >                          Local1 |= IOAL
> >                          RRIO (Arg0, Arg1, Local1, 0x08)
> >                          EXFG ()
> >                      }
> >
> > Can there be more than one address space handler for a given region ?
> > Each driver accessing the resource would need that handler.
> 
> Rather, every driver accessing the resource would need to be aware of
> the existence of the operation region handler and would need to use the
> mutual exclusion mechanism used by that handler, if my understanding
> here is correct.
> 
> The existence of an operation region for a specific section of address
> space is a declaration that AML is going to access locations in that
> section.  It allows the OS to install a handler for that region to
> intercept AML accesses and do what it likes with them.
> 
> Thanks,
> Rafael

Yes, agreed.
Bob

[toc] | [prev] | [next] | [standalone]

#1625773

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-19 03:30 +0200
Message-ID<txMBr-5g9-17@gated-at.bofh.it>
In reply to#1625338
Hi,

> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> On 04/18/2017 12:14 AM, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Zheng, Lv
> >> Subject: RE: [PATCH] ACPICA: Export mutex functions
> >>
> >> Hi,
> >>
> >>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> >>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> >>>
> >>> On 04/17/2017 04:53 PM, Zheng, Lv wrote:
> >>>> Hi,
> >>>>
> >>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> >>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> >>>>>
> >>>>> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
> >>>>>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> >>>>>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> >>>>>>>>>
> >>>>>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> >>>>>>>>>>
> >>>>>>>>>>> From: Moore, Robert
> >>>>>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
> >>>>>>>>>>>
> >>>>>>>>>>> There is a model for the drivers to directly acquire an AML mutex
> >>>>>>>>>>> object. That is why the acquire/release public interfaces were added
> >>>>>>>>>>> to ACPICA.
> >>>>>>>>>>>
> >>>>>>>>>>> I forget all of the details, but the model was developed with MS and
> >>>>>>>>>>> others during the ACPI 6.0 timeframe.
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>> [Moore, Robert]
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Here is the case where the OS may need to directly acquire an AML
> >>>>>>>>> mutex:
> >>>>>>>>>>
> >>>>>>>>>> From the ACPI spec:
> >>>>>>>>>>
> >>>>>>>>>> 19.6.2 Acquire (Acquire a Mutex)
> >>>>>>>>>>
> >>>>>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
> >>>>>>>>> also contend for ownership.
> >>>>>>>>>>
> >>>>>>>>> From the context in the dsdt, and from description of expected use cases
> >>>>>>>>> for _DLM objects I can find, this is what the mutex is used for (to
> >>>>>>>>> serialize access to a resource on a low pin count serial interconnect,
> >>>>>>>>> aka LPC).
> >>>>>>>>>
> >>>>>>>>> What does that mean in practice ? That I am not supposed to use it
> >>>>>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Guenter
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>> [Moore, Robert]
> >>>>>>>>
> >>>>>>>> I'm not an expert on the _DLM method, but I would point you to the description section in the
> >>>>> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
> >>>>>>>>
> >>>>>>>
> >>>>>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
> >>>>>>
> >>>>>> Basically, if the kernel and AML need to access a device concurrently,
> >>>>>> there should be a _DLM object under that device in the ACPI tables.
> >>>>>> In that case it is expected to return a list of (AML) mutexes that can
> >>>>>> be acquired by the kernel in order to synchronize device access with
> >>>>>> respect to AML (and for each mutex it may also return a description of
> >>>>>> the specific resources to be protected by it).
> >>>>>>
> >>>>>> Bottom line: without _DLM, the kernel cannot synchronize things with
> >>>>>> respect to AML properly, because it has no information how to do that
> >>>>>> then.
> >>>>>
> >>>>> That is all quite interesting. I do see the mutex in question used on various
> >>>>> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
> >>>>> Intel). Interestingly, the naming seems to be consistent - it is always named
> >>>>> "MUT0". For the most part, it seems to be available on more recent
> >>>>> motherboards; older motherboards tend to use the resource without locking.
> >>>>> However, I don't see any mention of "_DLM" in any of the DSDTs.
> >>>>>
> >>>>
> >>>> OK, then you might be having problems in your opregion driver.
> >>>>
> >>>>> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
> >>>>> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
> >>>>> drivers, but also in parallel port and infrared driver code. Effectively
> >>>>> that means that all this code is inherently unsafe on systems with ACPI
> >>>>> support.
> >>>>>
> >>>>> I had thought about implementing a set of utility functions to make the kernel
> >>>>> code safer to use if the mutex is found to exist.
> >>>>
> >>>> As what you've mentioned, there are already lots of parallel accesses in kernel without enabling
> >>> ACPI.
> >>>> Are these accesses mutually exclusive (safe)?
> >>>
> >>> In-kernel, yes (using request_muxed_region). Against ACPI, no.
> >>>
> >>>> If so, why do you need to invent a new synchronization mechanism?
> >>>>
> >>>
> >>> Because I am seeing a problem with the current code (more specifically,
> >>> with the it87 hwmon driver) on new Gigabyte boards.
> >>
> >> I checked superio_enter()/superio_exit(), IMO, the mutual exclusion
> >> might be handled using 1 of the following 2 solutions:
> >>
> >> 1. _DLM, then you can find superio related mutex from _DLM and
> >>    acquire/release it in superio_enter()/superio_exit().
> >>    You really need a set of new APIs to acquire the _DLM related mutex.
> >>    If you don't have _DLM in your DSDT, directly exporting ACPICA mutex
> >>    functions seem to be a reasonable solution.
> >> 2. Normally, AML developer should abstract superio accesses into customized
> >>    opregion, then you can prepare a superio opregion driver.
> >>
> >>>
> >>>>> Right now I wonder, though,
> >>>>> if such code would have a chance to be accepted. Any thoughts on that ?
> >>>>
> >>>> Is that possible to make it safe in the opregion driver?
> >>>>
> >>>
> >>> Sorry, I don't think I understand what you mean with "opregion driver".
> >>> Do you refer to the drivers accessing the memory region in question,
> >>> or in other words replicating the necessary code in every driver accessing
> >>> that region ? Sure, I can do that, if that is the preferred solution;
> >>> I have no problem with that. However, that would require exporting
> >>> the ACPI mutex functions. My understanding is that you are opposed to
> >>> exporting those, so I assume that is not what you refer to.
> >>> Can you clarify ?
> >>
> >> I mean solution 2.
> >
> > Maybe I should provide more detailed examples for this solution.
> >
> > For example:
> > OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100)
> > Field (SIOT, ByteAcc, Lock, Preserve)
> > {
> >     FNC1, 8,
> >     FNC2, 8,
> >     ...
> > }
> >
> > Acquire (MUX0)
> > Store (0, FNC1)
> > Release (MUX0)
> >
> > Then you can call (let me use casual pseudo code)
> > acpi_install_operation_region(SuperIOAddressSpace, superio_opregion_handler) from OS side.
> > In its callback superio_opregion_handler(), you can:
> >
> > superio_enter();
> > If (address == 0) {
> >    /* mean FNC1 */
> >    Perform the locked superior accesses
> > } else if (address == 1) {
> >    /* mean FNC2 */
> >    Perform the locked superior accesses
> > }
> > superio_exit();
> >
> > Are there similar approach in your DSDT?
> >
> 
> Some snippets from the DSDT:
> 
> 	Device (SIO1)
>             {
>          	Name (_HID, EisaId ("PNP0C02") /* PNP Motherboard Resources */)  // _HID: Hardware ID
>          	Name (_UID, Zero)  // _UID: Unique ID
> 		...
> 		Mutex (MUT0, 0x00)
> 		Method (ENFG, 1, NotSerialized)
>                      {
>                          Acquire (MUT0, 0x0FFF)
>                          INDX = 0x87
>                          INDX = One
>                          INDX = 0x55
>                          If ((SP1O == 0x2E))
>                          {
>                              INDX = 0x55
>                          }
>                          Else
>                          {
>                              INDX = 0xAA
>                          }
> 
>                          LDN = Arg0
>                      }
> 
>                      Method (EXFG, 0, NotSerialized)
>                      {
>                          INDX = 0x02
>                          DATA = 0x02
>                          Release (MUT0)
>                      }
> 
> 		    OperationRegion (IOID, SystemIO, SP1O, 0x02)	/* SP1O is 0x2e */
>                      Field (IOID, ByteAcc, NoLock, Preserve)
>                      {
>                          INDX,   8,
>                          DATA,   8
>                      }
> 		    ...
> 
> Example for use:
> 		Method (DCNT, 2, NotSerialized)
>                      {
>                          ENFG (CGLD (Arg0))
>                          If (((DMCH < 0x04) && ((Local1 = (DMCH & 0x03)) != Zero)))
>                          {
>                              RDMA (Arg0, Arg1, Local1++)
>                          }
> 
>                          ACTR = Arg1
>                          Local1 = (IOAH << 0x08)
>                          Local1 |= IOAL
>                          RRIO (Arg0, Arg1, Local1, 0x08)
>                          EXFG ()
>                      }
> 
> Can there be more than one address space handler for a given region ?
> Each driver accessing the resource would need that handler.

From the above AML code, the solution 2 is not possible for ensuring the
mutual exclusion of accessing the resources.
Because the mutual exclusion must be ensured for the entire transaction
including ENFG() and EXFG() rather than a single SystemIo operation
region field access.

So you really need the solution 1 and the new API.

Thanks and best regards
Lv

> 
> Thanks,
> Guenter
> 
> > Thanks and best regards
> > Lv
> >
> >> From it87_find() I really couldn't see a possibility to convert superio
> >> accesses into opregions. Could you paste some example superio access AML
> >> code from your DSDT here.
> >>
> >> Thanks and best regards
> >> Lv

[toc] | [prev] | [next] | [standalone]

#1625776

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-19 03:40 +0200
Message-ID<txML8-5kl-5@gated-at.bofh.it>
In reply to#1625773
Hi,

> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
> Subject: Re: [Devel] [PATCH] ACPICA: Export mutex functions
> 
> Hi,
> 
> > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > Subject: Re: [PATCH] ACPICA: Export mutex functions
> >
> > On 04/18/2017 12:14 AM, Zheng, Lv wrote:
> > > Hi,
> > >
> > >> From: Zheng, Lv
> > >> Subject: RE: [PATCH] ACPICA: Export mutex functions
> > >>
> > >> Hi,
> > >>
> > >>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >>>
> > >>> On 04/17/2017 04:53 PM, Zheng, Lv wrote:
> > >>>> Hi,
> > >>>>
> > >>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >>>>>
> > >>>>> On Mon, Apr 17, 2017 at 11:29:38PM +0200, Rafael J. Wysocki wrote:
> > >>>>>> On Mon, Apr 17, 2017 at 11:03 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > >>>>>>> On Mon, Apr 17, 2017 at 08:40:38PM +0000, Moore, Robert wrote:
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>> From: Guenter Roeck [mailto:linux@roeck-us.net]
> > >>>>>>>>> Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >>>>>>>>>
> > >>>>>>>>> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> > >>>>>>>>>>
> > >>>>>>>>>>> From: Moore, Robert
> > >>>>>>>>>>> Subject: RE: [PATCH] ACPICA: Export mutex functions
> > >>>>>>>>>>>
> > >>>>>>>>>>> There is a model for the drivers to directly acquire an AML mutex
> > >>>>>>>>>>> object. That is why the acquire/release public interfaces were added
> > >>>>>>>>>>> to ACPICA.
> > >>>>>>>>>>>
> > >>>>>>>>>>> I forget all of the details, but the model was developed with MS and
> > >>>>>>>>>>> others during the ACPI 6.0 timeframe.
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>> [Moore, Robert]
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> Here is the case where the OS may need to directly acquire an AML
> > >>>>>>>>> mutex:
> > >>>>>>>>>>
> > >>>>>>>>>> From the ACPI spec:
> > >>>>>>>>>>
> > >>>>>>>>>> 19.6.2 Acquire (Acquire a Mutex)
> > >>>>>>>>>>
> > >>>>>>>>>> Note: For Mutex objects referenced by a _DLM object, the host OS may
> > >>>>>>>>> also contend for ownership.
> > >>>>>>>>>>
> > >>>>>>>>> From the context in the dsdt, and from description of expected use cases
> > >>>>>>>>> for _DLM objects I can find, this is what the mutex is used for (to
> > >>>>>>>>> serialize access to a resource on a low pin count serial interconnect,
> > >>>>>>>>> aka LPC).
> > >>>>>>>>>
> > >>>>>>>>> What does that mean in practice ? That I am not supposed to use it
> > >>>>>>>>> because it doesn't follow standard ACPI mutex declaration rules ?
> > >>>>>>>>>
> > >>>>>>>>> Thanks,
> > >>>>>>>>> Guenter
> > >>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>> [Moore, Robert]
> > >>>>>>>>
> > >>>>>>>> I'm not an expert on the _DLM method, but I would point you to the description section in
> the
> > >>>>> ACPI spec, 5.7.5 _DLM (DeviceLock Mutex).
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>> I did. However, not being an ACPI expert, that doesn't tell me anything.
> > >>>>>>
> > >>>>>> Basically, if the kernel and AML need to access a device concurrently,
> > >>>>>> there should be a _DLM object under that device in the ACPI tables.
> > >>>>>> In that case it is expected to return a list of (AML) mutexes that can
> > >>>>>> be acquired by the kernel in order to synchronize device access with
> > >>>>>> respect to AML (and for each mutex it may also return a description of
> > >>>>>> the specific resources to be protected by it).
> > >>>>>>
> > >>>>>> Bottom line: without _DLM, the kernel cannot synchronize things with
> > >>>>>> respect to AML properly, because it has no information how to do that
> > >>>>>> then.
> > >>>>>
> > >>>>> That is all quite interesting. I do see the mutex in question used on various
> > >>>>> motherboards from various vendors (I checked boards from Gigabyte, MSI, and
> > >>>>> Intel). Interestingly, the naming seems to be consistent - it is always named
> > >>>>> "MUT0". For the most part, it seems to be available on more recent
> > >>>>> motherboards; older motherboards tend to use the resource without locking.
> > >>>>> However, I don't see any mention of "_DLM" in any of the DSDTs.
> > >>>>>
> > >>>>
> > >>>> OK, then you might be having problems in your opregion driver.
> > >>>>
> > >>>>> At the same time, access to ports 0x2e/0x2f is widely used in the kernel.
> > >>>>> As mentioned before, it is used in watchdog, hardware monitoring, and gpio
> > >>>>> drivers, but also in parallel port and infrared driver code. Effectively
> > >>>>> that means that all this code is inherently unsafe on systems with ACPI
> > >>>>> support.
> > >>>>>
> > >>>>> I had thought about implementing a set of utility functions to make the kernel
> > >>>>> code safer to use if the mutex is found to exist.
> > >>>>
> > >>>> As what you've mentioned, there are already lots of parallel accesses in kernel without
> enabling
> > >>> ACPI.
> > >>>> Are these accesses mutually exclusive (safe)?
> > >>>
> > >>> In-kernel, yes (using request_muxed_region). Against ACPI, no.
> > >>>
> > >>>> If so, why do you need to invent a new synchronization mechanism?
> > >>>>
> > >>>
> > >>> Because I am seeing a problem with the current code (more specifically,
> > >>> with the it87 hwmon driver) on new Gigabyte boards.
> > >>
> > >> I checked superio_enter()/superio_exit(), IMO, the mutual exclusion
> > >> might be handled using 1 of the following 2 solutions:
> > >>
> > >> 1. _DLM, then you can find superio related mutex from _DLM and
> > >>    acquire/release it in superio_enter()/superio_exit().
> > >>    You really need a set of new APIs to acquire the _DLM related mutex.
> > >>    If you don't have _DLM in your DSDT, directly exporting ACPICA mutex
> > >>    functions seem to be a reasonable solution.
> > >> 2. Normally, AML developer should abstract superio accesses into customized
> > >>    opregion, then you can prepare a superio opregion driver.
> > >>
> > >>>
> > >>>>> Right now I wonder, though,
> > >>>>> if such code would have a chance to be accepted. Any thoughts on that ?
> > >>>>
> > >>>> Is that possible to make it safe in the opregion driver?
> > >>>>
> > >>>
> > >>> Sorry, I don't think I understand what you mean with "opregion driver".
> > >>> Do you refer to the drivers accessing the memory region in question,
> > >>> or in other words replicating the necessary code in every driver accessing
> > >>> that region ? Sure, I can do that, if that is the preferred solution;
> > >>> I have no problem with that. However, that would require exporting
> > >>> the ACPI mutex functions. My understanding is that you are opposed to
> > >>> exporting those, so I assume that is not what you refer to.
> > >>> Can you clarify ?
> > >>
> > >> I mean solution 2.
> > >
> > > Maybe I should provide more detailed examples for this solution.
> > >
> > > For example:
> > > OperationRegion (SIOT, SuperIOAddressSpace, Zero, 100)
> > > Field (SIOT, ByteAcc, Lock, Preserve)
> > > {
> > >     FNC1, 8,
> > >     FNC2, 8,
> > >     ...
> > > }
> > >
> > > Acquire (MUX0)
> > > Store (0, FNC1)
> > > Release (MUX0)
> > >
> > > Then you can call (let me use casual pseudo code)
> > > acpi_install_operation_region(SuperIOAddressSpace, superio_opregion_handler) from OS side.
> > > In its callback superio_opregion_handler(), you can:
> > >
> > > superio_enter();
> > > If (address == 0) {
> > >    /* mean FNC1 */
> > >    Perform the locked superior accesses
> > > } else if (address == 1) {
> > >    /* mean FNC2 */
> > >    Perform the locked superior accesses
> > > }
> > > superio_exit();
> > >
> > > Are there similar approach in your DSDT?
> > >
> >
> > Some snippets from the DSDT:
> >
> > 	Device (SIO1)
> >             {
> >          	Name (_HID, EisaId ("PNP0C02") /* PNP Motherboard Resources */)  // _HID: Hardware ID
> >          	Name (_UID, Zero)  // _UID: Unique ID
> > 		...
> > 		Mutex (MUT0, 0x00)
> > 		Method (ENFG, 1, NotSerialized)
> >                      {
> >                          Acquire (MUT0, 0x0FFF)
> >                          INDX = 0x87
> >                          INDX = One
> >                          INDX = 0x55
> >                          If ((SP1O == 0x2E))
> >                          {
> >                              INDX = 0x55
> >                          }
> >                          Else
> >                          {
> >                              INDX = 0xAA
> >                          }
> >
> >                          LDN = Arg0
> >                      }
> >
> >                      Method (EXFG, 0, NotSerialized)
> >                      {
> >                          INDX = 0x02
> >                          DATA = 0x02
> >                          Release (MUT0)
> >                      }
> >
> > 		    OperationRegion (IOID, SystemIO, SP1O, 0x02)	/* SP1O is 0x2e */
> >                      Field (IOID, ByteAcc, NoLock, Preserve)
> >                      {
> >                          INDX,   8,
> >                          DATA,   8
> >                      }
> > 		    ...
> >
> > Example for use:
> > 		Method (DCNT, 2, NotSerialized)
> >                      {
> >                          ENFG (CGLD (Arg0))
> >                          If (((DMCH < 0x04) && ((Local1 = (DMCH & 0x03)) != Zero)))
> >                          {
> >                              RDMA (Arg0, Arg1, Local1++)
> >                          }
> >
> >                          ACTR = Arg1
> >                          Local1 = (IOAH << 0x08)
> >                          Local1 |= IOAL
> >                          RRIO (Arg0, Arg1, Local1, 0x08)
> >                          EXFG ()
> >                      }
> >
> > Can there be more than one address space handler for a given region ?
> > Each driver accessing the resource would need that handler.
> 
> From the above AML code, the solution 2 is not possible for ensuring the
> mutual exclusion of accessing the resources.
> Because the mutual exclusion must be ensured for the entire transaction
> including ENFG() and EXFG() rather than a single SystemIo operation
> region field access.
> 
> So you really need the solution 1 and the new API.

Sorry, there is still another solution besides of the above 2 that
Can possibly solve your problem.

Let me explain.
Here for the DCNT method, there must be entry methods (those with
underscore prefixed, let me use _XXXX as an example) invoking it.
And OS will invoke those entry methods. So is that possible to add
request_muxed_region() before invoking those entry control methods.

For example:
superio_enter()
acpi_evaluate_object(_XXXX)
superio_exit()

Thanks and best regards
Lv

> 
> Thanks and best regards
> Lv
> 
> >
> > Thanks,
> > Guenter
> >
> > > Thanks and best regards
> > > Lv
> > >
> > >> From it87_find() I really couldn't see a possibility to convert superio
> > >> accesses into opregions. Could you paste some example superio access AML
> > >> code from your DSDT here.
> > >>
> > >> Thanks and best regards
> > >> Lv
> 
> _______________________________________________
> Devel mailing list
> Devel@acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

[toc] | [prev] | [next] | [standalone]

#1624947

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 02:00 +0200
Message-ID<txoIN-7c0-1@gated-at.bofh.it>
In reply to#1624859
Hi,

> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Sent: Tuesday, April 18, 2017 3:45 AM
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> On Mon, Apr 17, 2017 at 07:27:37PM +0000, Moore, Robert wrote:
> >
> > > -----Original Message-----
> > > From: Moore, Robert
> > > Sent: Monday, April 17, 2017 10:13 AM
> > > To: Guenter Roeck <linux@roeck-us.net>; Zheng, Lv <lv.zheng@intel.com>
> > > Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Len Brown
> > > <lenb@kernel.org>; linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > > kernel@vger.kernel.org
> > > Subject: RE: [PATCH] ACPICA: Export mutex functions
> > >
> > > There is a model for the drivers to directly acquire an AML mutex
> > > object. That is why the acquire/release public interfaces were added to
> > > ACPICA.
> > >
> > > I forget all of the details, but the model was developed with MS and
> > > others during the ACPI 6.0 timeframe.
> > >
> > >
> > [Moore, Robert]
> >
> >
> > Here is the case where the OS may need to directly acquire an AML mutex:
> >
> > From the ACPI spec:
> >
> > 19.6.2 Acquire (Acquire a Mutex)
> >
> > Note: For Mutex objects referenced by a _DLM object, the host OS may also contend for ownership.
> >
> From the context in the dsdt, and from description of expected use cases for
> _DLM objects I can find, this is what the mutex is used for (to serialize
> access to a resource on a low pin count serial interconnect, aka LPC).
> 
> What does that mean in practice ? That I am not supposed to use it because
> it doesn't follow standard ACPI mutex declaration rules ?
> 

Could you find related _DLMs in your DSDT?
If there is any, could you please post it here for reference?

Thanks
Lv

> Thanks,
> Guenter
> 
> >
> >
> >
> > Other than this case, the OS/drivers should never need to directly acquire an AML mutex.
> > Bob
> >

[toc] | [prev] | [next] | [standalone]

#1624922

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 01:50 +0200
Message-ID<txoz7-789-1@gated-at.bofh.it>
In reply to#1624847
Hi,

> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, April 18, 2017 3:28 AM
> To: 'Guenter Roeck' <linux@roeck-us.net>; Zheng, Lv <lv.zheng@intel.com>
> Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; 'Len Brown' <lenb@kernel.org>; 'linux-
> acpi@vger.kernel.org' <linux-acpi@vger.kernel.org>; 'devel@acpica.org' <devel@acpica.org>; 'linux-
> kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; Box, David E <david.e.box@intel.com>
> Subject: RE: [PATCH] ACPICA: Export mutex functions
> 
> 
> > -----Original Message-----
> > From: Moore, Robert
> > Sent: Monday, April 17, 2017 10:13 AM
> > To: Guenter Roeck <linux@roeck-us.net>; Zheng, Lv <lv.zheng@intel.com>
> > Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Len Brown
> > <lenb@kernel.org>; linux-acpi@vger.kernel.org; devel@acpica.org; linux-
> > kernel@vger.kernel.org
> > Subject: RE: [PATCH] ACPICA: Export mutex functions
> >
> > There is a model for the drivers to directly acquire an AML mutex
> > object. That is why the acquire/release public interfaces were added to
> > ACPICA.
> >
> > I forget all of the details, but the model was developed with MS and
> > others during the ACPI 6.0 timeframe.
> >
> >
> [Moore, Robert]
> 
> 
> Here is the case where the OS may need to directly acquire an AML mutex:
> 
> From the ACPI spec:
> 
> 19.6.2 Acquire (Acquire a Mutex)
> 
> Note: For Mutex objects referenced by a _DLM object, the host OS may also contend for ownership.
> 
> 
> 
> 
> Other than this case, the OS/drivers should never need to directly acquire an AML mutex.

That sounds reasonable but the driver might invoke an ACPICA API accessing the _DLM returned mutexes.

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1624855

From"Moore, Robert" <robert.moore@intel.com>
Date2017-04-17 21:40 +0200
Message-ID<txkFb-4Gv-13@gated-at.bofh.it>
In reply to#1624749

> -----Original Message-----
> From: Moore, Robert
> Sent: Monday, April 17, 2017 12:28 PM
> To: 'Guenter Roeck' <linux@roeck-us.net>; Zheng, Lv <lv.zheng@intel.com>
> Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; 'Len Brown'
> <lenb@kernel.org>; 'linux-acpi@vger.kernel.org' <linux-
> acpi@vger.kernel.org>; 'devel@acpica.org' <devel@acpica.org>; 'linux-
> kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; Box, David E
> (david.e.box@intel.com) <david.e.box@intel.com>
> Subject: RE: [PATCH] ACPICA: Export mutex functions
> 
> 
> > -----Original Message-----
> > From: Moore, Robert
> > Sent: Monday, April 17, 2017 10:13 AM
> > To: Guenter Roeck <linux@roeck-us.net>; Zheng, Lv <lv.zheng@intel.com>
> > Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Len Brown
> > <lenb@kernel.org>; linux-acpi@vger.kernel.org; devel@acpica.org;
> > linux- kernel@vger.kernel.org
> > Subject: RE: [PATCH] ACPICA: Export mutex functions
> >
> > There is a model for the drivers to directly acquire an AML mutex
> > object. That is why the acquire/release public interfaces were added
> > to ACPICA.
> >
> > I forget all of the details, but the model was developed with MS and
> > others during the ACPI 6.0 timeframe.
> >
> >
> [Moore, Robert]
> 
> 
> Here is the case where the OS may need to directly acquire an AML mutex:
> 
> From the ACPI spec:
> 
> 19.6.2 Acquire (Acquire a Mutex)
> 
> Note: For Mutex objects referenced by a _DLM object, the host OS may
> also contend for ownership.
> 
> Other than this case, the OS/drivers should never need to directly
> acquire an AML mutex.
> Bob

[Moore, Robert] 
Here is more information, from the ACPICA reference:

8.3.15 AcpiAcquireMutex

This function is intended to be used in conjunction with the _DLM (Device Lock Method) predefined name to directly acquire a mutex object that is defined in the ACPI namespace. The purpose of this is to provide a mutual exclusion mechanism between the AML interpreter and an ACPI-related device driver, in order to support multiple-operation transactions.

From the ACPI Specification: "The _DLM object appears in a device scope when AML access to the device must be synchronized with the OS environment. It is used in conjunction with a standard Mutex object. With _DLM, the standard Mutex provides synchronization within the AML environment as usual, but also synchronizes with the OS environment."

The AML mutex node is pointed to by Parent:Pathname. Either the Parent or the Pathname can be NULL, but not both.

If the operation fails an appropriate status will be returned.

[toc] | [prev] | [next] | [standalone]

#1624918

From"Zheng, Lv" <lv.zheng@intel.com>
Date2017-04-18 01:40 +0200
Message-ID<txops-74j-17@gated-at.bofh.it>
In reply to#1624727
Hi,

> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Subject: Re: [PATCH] ACPICA: Export mutex functions
> 
> Hi,
> 
> On Mon, Apr 17, 2017 at 09:39:35AM +0000, Zheng, Lv wrote:
> > Hi,
> >
> > > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > > Subject: Re: [PATCH] ACPICA: Export mutex functions
> > >
> > > On Wed, Apr 12, 2017 at 03:29:55PM +0000, Moore, Robert wrote:
> > > > The ACPICA mutex functions are based on the host OS functions, so they don't really buy you
> anything.
> > > You should just use the native Linux functions.
> > > >
> > >
> > > You mean they don't really acquire the requested ACPI mutex,
> > > and the underlying DSDT which declares and uses the mutex
> > > just ignores if the mutex was acquired by acpi_acquire_mutex() ?
> > >
> > > To clarify: You are saying that code such as
> > >
> > > 	acpi_status status;
> > >
> > > 	status = acpi_acquire_mutex(NULL, "\\_SB.PCI0.SBRG.SIO1.MUT0", 0x10);
> > > 	if (ACPI_FAILURE(status)) {
> > > 		pr_err("Failed to acquire ACPI mutex\n");
> > > 		return -EBUSY;
> > > 	}
> >
> > Why do you need to access \_SB.PCI0.SBRG.SIO1.MUT0?
> > OSPM should only invoke entry methods predefined by ACPI spec or whatever specs.
> > There shouldn't be any needs that a driver acquires an arbitrary AML mutex.
> > You do not seem to have justified the usage model, IMO.
> >
> 
> I am sorry, I have no idea how to do that. I can see that the resource in
> question (IO address 0x2e/0x2f) is accessed from the DSDT, that the resource
> is mutex protected, and that accesses to the same IO address from the Linux
> kernel are unreliable unless I acquire the mutex in question. At the same time,
> I can see that request_muxed_region() succeeds, so presumably ACPI does not
> reserve the region for its exclusive use.

Please refer to my previous reply.
Why don't you make it safe in super IO driver and the opregion driver in first place.
Then check if you still need to acquire the mutex.

> 
> It may well be that the "official" response to this problem is "you must
> not instantiate a watchdog, environmental monitor, or gpio driver (or anything
> else provided by the Super-IO chip that requires access to those ports) on this
> platform in Linux". Is that what you are suggesting ?
> 

I didn't mean that.
Probably you have synchronization problems in the OS driver.

Thanks and best regards
Lv

> Thanks,
> Guenter

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web