Re: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier

[Dou Liyang <douly.fnst@cn.fujitsu.com>]

Hi Baoquan,

Thanks for your reply.

At 08/24/2017 04:05 PM, Baoquan He wrote:
> Hi Liyang,
>
> On 08/24/17 at 11:54am, Dou Liyang wrote:
>>>> Test in my own PC(Lenovo M4340).
>>>> Ask help for doing regression testing for the bug said in commit
>>>> c4e1acbb35e4
>>>> ("ACPI / init: Invoke early ACPI initialization later").
>>>>
>>>
>>> Now, I can prove this patch doesn't result in the bug[1] which made the
>>> fast TSC calibration using PIT failed in a Thinkpad x121e (AMD E-450
>>> APU).
>>>
>>> The true reason of the bug is enabling ACPI subsystem earlier than
>>> using PIT, not the SCI setup. invoking acpi_enable_subsystem() later
>
> Seems redhat mail server was down earlier, I didn't receive new mail in
> this thread.  Just curious, do you know why the fast tsc calibration
> using PIT will fail if enabling ACPI subsystem earlier than using PIT?
>
It's related to particular hardware, As you know, I tested in many
kinds of PC and laptop and PIT works well no matter before or after
enabling ACPI subsystem.

In pit_verify_msb(), we use inb(0x42) to read the current MSB,

Normally, the value is continuously, like following shows:

msb = fe
msb = fd
msb = fc
msb = fb
msb = fa
msb = f9
msb = f8
msb = f7
msb = f6
...

But, if in some particular hardware, you will see like that:

msb = fe
msb = f0
msb = ed
msb = e9
msb = e0
msb = db
...

In this case, the count in pit_expect_msb() is always zero.
So we will see "Fast TSC calibration failed" in our dmesg log.

For the further deep reason why the hardware failed, I'm sorry
I can't answer and don't know how to investigate. For hardware,
I usually change a new one directly and know very little.

Thanks,
	dou.


> Thanks
> Baoquan
>
>>> could fix this bug as Julian tested and said[2].
>>>
>>> And, I found that Commit b064a8fa77df (" ACPI / init: Switch over
>>> platform to the ACPI mode later") split the ACPI early initialization
>>> code into acpi_early_init() and acpi_subsystem_init(). executing
>>> acpi_enable_subsystem() at the original early ACPI initialization spot.
>>>
>>> The sequence of them shows below:
>>>
>>>  start_kernel
>>> +---------------+
>>> |
>>> +--> .......
>>> |
>>> |    late_time_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> |    acpi_early_init()
>>> +--> +-------+
>>> |
>>> +--> .......
>>> |
>>> |   acpi_subsystem_init()
>>> +-> +--------+
>>>
>>> We make sure the acpi_subsystem_init() is called later than
>>> late_time_init(), the bug will be avoided.
>>>
>>> This patch changes the sequence of late_time_init() and
>>> acpi_early_init(), doesn't effect acpi_subsystem_init().
>>>
>>> So, this patch is OK.
>>>
>>> Btw, Thanks very much for Borislav Petkov, he will have access to
>>> Thinkpad x121e from Mid-August and will test this series.
>>>
>>
>> Almost one month passed, Borislav have tested this series in Thinkpad
>> x121e and I also have tested in my box and QEmu again. It is OK.
>>
>> BTW,
>> 1) I found your commit b064a8fa77df (" ACPI / init: Switch over
>> platform to the ACPI mode later") split the ACPI early initialization
>> code into acpi_early_init() and acpi_subsystem_init(). Actually enabling
>> the ACPI subsystem is in acpi_subsystem_init().
>>
>> 2) As we discussed earlier, invoking acpi_put_table() is not good for
>> this situation.
>>
>> So I do this patch, Is that goot to you? Any comments will be welcome.
>>
>> If it is OK, As the patches need to be re-based, and I also found
>> several spelling mistake, I will send a new version next week.
>>
>> Thanks,
>> 	dou.
>>
>>> [1] https://lkml.org/lkml/2014/3/10/123
>>> [2] https://lkml.org/lkml/2014/3/12/311
>>>
>>>
>>> Thanks
>>>     dou.
>>>
>>>>  init/main.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index df58a41..7a09467 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -654,12 +654,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>      kmemleak_init();
>>>>      setup_per_cpu_pageset();
>>>>      numa_policy_init();
>>>> +    acpi_early_init();
>>>>      if (late_time_init)
>>>>          late_time_init();
>>>>      calibrate_delay();
>>>>      pidmap_init();
>>>>      anon_vma_init();
>>>> -    acpi_early_init();
>>>>  #ifdef CONFIG_X86
>>>>      if (efi_enabled(EFI_RUNTIME_SERVICES))
>>>>          efi_enter_virtual_mode();
>>>>
>>
>>
>
>
>