Linux Virtualization - May 2016 - [PATCH v4 6/6] hwmon: use smp_call_on_cpu() for dell-smm i8k

On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
> On 21/04/16 12:57, Pali Roh?r wrote:
> > On Tuesday 05 April 2016 21:31:52 Pali Roh?r wrote:
> >> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> >>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross wrote:
> >>>> Use the smp_call_on_cpu() function to call system
management
> >>>> mode on cpu 0.
> >>>> Make call secure by adding get_online_cpus() to avoid e.g.
suspend
> >>>> resume cycles in between.
> >>>>
> >>>> Signed-off-by: Juergen Gross <jgross at suse.com>
> >>>> ---
> >>>> V4: add call to get_online_cpus()
> >>>
> >>> Pali, any chance to test this ?
> >>
> >> I can test it, but just on machine where (probably) smm calls can
be
> >> send from any cpu... Need some time for testing and I believe I
can do
> >> that at the end of the week.
> > 
> > Sorry I had absolutely no more free time last weekend :-( And same
> > prediction is for this weekend and also next one...
> 
> Pali, I've got a Dell laptop (Latitude E6440) here. Would this device
be
> okay for a test?
Hi!

Proper regression test should check if this patch does not break any
function or drivers dependent on dcdbas.ko. And should be done on both
notebook devices: which needs to issue that smm call on cpu 0 and also
on which it is not needed.

Some notebooks which needs smm call to issued from cpu 0 can be found in
git commit messages of i8k, dell-laptop or dcdbas kernel drivers.
> What would you do for testing? In case you can give me
> some hints how to do a sensible test I'd do it.
Test e.g. dell-laptop.ko driver. It provides /sys interface for changing
keyboard backlight or changing rfkill switches (bluetooth wifi).

Also test tools from libsmbios (userspace) package.

There must be no difference in output/functionality with or without your
patches.
> I've verified by adding a printk() to smp_call_on_cpu() that at least
> one of the modified drivers has been used during system boot.
Also you can patch i8k/dcdbas smm function to print cpu number on which
is code running (to verify that it was really called on cpu 0 as
needed).

-- 
Pali Roh?r
pali.rohar at gmail.com

On 21/04/16 15:27, Pali Roh?r wrote:
> On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
>> On 21/04/16 12:57, Pali Roh?r wrote:
>>> On Tuesday 05 April 2016 21:31:52 Pali Roh?r wrote:
>>>> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
>>>>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen Gross
wrote:
>>>>>> Use the smp_call_on_cpu() function to call system
management
>>>>>> mode on cpu 0.
>>>>>> Make call secure by adding get_online_cpus() to avoid
e.g. suspend
>>>>>> resume cycles in between.
>>>>>>
>>>>>> Signed-off-by: Juergen Gross <jgross at suse.com>
>>>>>> ---
>>>>>> V4: add call to get_online_cpus()
>>>>>
>>>>> Pali, any chance to test this ?
>>>>
>>>> I can test it, but just on machine where (probably) smm calls
can be
>>>> send from any cpu... Need some time for testing and I believe I
can do
>>>> that at the end of the week.
>>>
>>> Sorry I had absolutely no more free time last weekend :-( And same
>>> prediction is for this weekend and also next one...
>>
>> Pali, I've got a Dell laptop (Latitude E6440) here. Would this
device be
>> okay for a test?
> 
> Hi!
> 
> Proper regression test should check if this patch does not break any
> function or drivers dependent on dcdbas.ko. And should be done on both
> notebook devices: which needs to issue that smm call on cpu 0 and also
> on which it is not needed.
Hmm, couldn't get one which needs smm to be called on cpu 0.
OTOH I've done various tests and added a printk() in raise_smm()
and i8k_smm_func() issuing the cpu number it was called on.
> Some notebooks which needs smm call to issued from cpu 0 can be found in
> git commit messages of i8k, dell-laptop or dcdbas kernel drivers.
> 
>> What would you do for testing? In case you can give me
>> some hints how to do a sensible test I'd do it.
> 
> Test e.g. dell-laptop.ko driver. It provides /sys interface for changing
> keyboard backlight or changing rfkill switches (bluetooth wifi).
Done.
> Also test tools from libsmbios (userspace) package.
Done.
> There must be no difference in output/functionality with or without your
> patches.
Verified.
>> I've verified by adding a printk() to smp_call_on_cpu() that at
least
>> one of the modified drivers has been used during system boot.
> 
> Also you can patch i8k/dcdbas smm function to print cpu number on which
> is code running (to verify that it was really called on cpu 0 as
> needed).
Done.

I tested suspend/resume, too, as adding get_online_cpus() might have
changed behavior. Worked like a charm. :-)


Juergen

On Monday 09 May 2016 16:37:01 Juergen Gross wrote:
> On 21/04/16 15:27, Pali Roh?r wrote:
> > On Thursday 21 April 2016 15:12:52 Juergen Gross wrote:
> >> On 21/04/16 12:57, Pali Roh?r wrote:
> >>> On Tuesday 05 April 2016 21:31:52 Pali Roh?r wrote:
> >>>> On Tuesday 05 April 2016 16:54:14 Guenter Roeck wrote:
> >>>>> On Tue, Apr 05, 2016 at 07:10:07AM +0200, Juergen
Gross wrote:
> >>>>>> Use the smp_call_on_cpu() function to call system
management
> >>>>>> mode on cpu 0.
> >>>>>> Make call secure by adding get_online_cpus() to
avoid e.g. suspend
> >>>>>> resume cycles in between.
> >>>>>>
> >>>>>> Signed-off-by: Juergen Gross <jgross at
suse.com>
> >>>>>> ---
> >>>>>> V4: add call to get_online_cpus()
> >>>>>
> >>>>> Pali, any chance to test this ?
> >>>>
> >>>> I can test it, but just on machine where (probably) smm
calls can be
> >>>> send from any cpu... Need some time for testing and I
believe I can do
> >>>> that at the end of the week.
> >>>
> >>> Sorry I had absolutely no more free time last weekend :-( And
same
> >>> prediction is for this weekend and also next one...
> >>
> >> Pali, I've got a Dell laptop (Latitude E6440) here. Would this
device be
> >> okay for a test?
> > 
> > Hi!
> > 
> > Proper regression test should check if this patch does not break any
> > function or drivers dependent on dcdbas.ko. And should be done on both
> > notebook devices: which needs to issue that smm call on cpu 0 and also
> > on which it is not needed.
> 
> Hmm, couldn't get one which needs smm to be called on cpu 0.
> OTOH I've done various tests and added a printk() in raise_smm()
> and i8k_smm_func() issuing the cpu number it was called on.
Understood, those machines are old and probably rare now.
> > Some notebooks which needs smm call to issued from cpu 0 can be found
in
> > git commit messages of i8k, dell-laptop or dcdbas kernel drivers.
> > 
> >> What would you do for testing? In case you can give me
> >> some hints how to do a sensible test I'd do it.
> > 
> > Test e.g. dell-laptop.ko driver. It provides /sys interface for
changing
> > keyboard backlight or changing rfkill switches (bluetooth wifi).
> 
> Done.
> 
> > Also test tools from libsmbios (userspace) package.
> 
> Done.
> 
> > There must be no difference in output/functionality with or without
your
> > patches.
> 
> Verified.
> 
> >> I've verified by adding a printk() to smp_call_on_cpu() that
at least
> >> one of the modified drivers has been used during system boot.
> > 
> > Also you can patch i8k/dcdbas smm function to print cpu number on
which
> > is code running (to verify that it was really called on cpu 0 as
> > needed).
> 
> Done.
> 
> I tested suspend/resume, too, as adding get_online_cpus() might have
> changed behavior. Worked like a charm. :-)
Ok, I think this should be enough. You can add my Acked-by.

-- 
Pali Roh?r
pali.rohar at gmail.com