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

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

[Dou Liyang]

Linux uses acpi_early_init() to put the ACPI table management into
the late stage from the early stage where the mapped ACPI tables is
temporary and should be unmapped.

But, now initializing interrupt delivery mode should map and parse the
DMAR table earlier in the early stage. This causes an ACPI error when
Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
the DMAR table after using in the early stage.

Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
be mapped and parsed in late stage like before.

Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Cc: linux-acpi@vger.kernel.org
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Zheng, Lv <lv.zheng@intel.com>
Cc: Julian Wollrath <jwollrath@web.de>
---
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").

 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();
-- 
2.5.5

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

[Zheng, Lv]

Hi,

Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?

Thanks
Lv

> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> Sent: Friday, July 14, 2017 1:53 PM
> To: x86@kernel.org; linux-kernel@vger.kernel.org
> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> 
> Linux uses acpi_early_init() to put the ACPI table management into
> the late stage from the early stage where the mapped ACPI tables is
> temporary and should be unmapped.
> 
> But, now initializing interrupt delivery mode should map and parse the
> DMAR table earlier in the early stage. This causes an ACPI error when
> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> the DMAR table after using in the early stage.
> 
> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> be mapped and parsed in late stage like before.
> 
> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Zheng, Lv <lv.zheng@intel.com>
> Cc: Julian Wollrath <jwollrath@web.de>
> ---
> 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").
> 
>  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();
> --
> 2.5.5
> 
> 

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

[Dou Liyang]

Hi, Zheng

At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> Hi,
>
> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?

Invoking acpi_put_table() is my first choice. But it made the kernel
*panic* when we try to get the table again in intel_iommu_init() in
late stage.

I am also confused that:

There are two places where we used DMAR table in Linux:

1) In detect_intel_iommu() in ACPI early stage:

...
status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
....
if (dmar_tbl) {
	acpi_put_table(dmar_tbl);
	dmar_tbl = NULL;
}

2) In dmar_table_init() in ACPI late stage:

...
status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
...

As we know, dmar_table_init() is called by intel_iommu_init() and
intel_prepare_irq_remapping().

When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in 
early stage like 1) shows, kernel will panic.


Thanks,

	dou.
>
> Thanks
> Lv
>
>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>> Sent: Friday, July 14, 2017 1:53 PM
>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>
>> Linux uses acpi_early_init() to put the ACPI table management into
>> the late stage from the early stage where the mapped ACPI tables is
>> temporary and should be unmapped.
>>
>> But, now initializing interrupt delivery mode should map and parse the
>> DMAR table earlier in the early stage. This causes an ACPI error when
>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>> the DMAR table after using in the early stage.
>>
>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>> be mapped and parsed in late stage like before.
>>
>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>> Cc: linux-acpi@vger.kernel.org
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Zheng, Lv <lv.zheng@intel.com>
>> Cc: Julian Wollrath <jwollrath@web.de>
>> ---
>> 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").
>>
>>  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();
>> --
>> 2.5.5
>>
>>
>
>
>
>

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

[bhe]

On 07/18/17 at 02:08pm, Dou Liyang wrote:
> Hi, Zheng
> 
> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> > Hi,
> > 
> > Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
> 
> Invoking acpi_put_table() is my first choice. But it made the kernel
> *panic* when we try to get the table again in intel_iommu_init() in
> late stage.
> 
> I am also confused that:
> 
> There are two places where we used DMAR table in Linux:
> 
> 1) In detect_intel_iommu() in ACPI early stage:
> 
> ...
> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> ....
> if (dmar_tbl) {
> 	acpi_put_table(dmar_tbl);
> 	dmar_tbl = NULL;
> }
> 
> 2) In dmar_table_init() in ACPI late stage:
> 
> ...
> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> ...
> 
> As we know, dmar_table_init() is called by intel_iommu_init() and
> intel_prepare_irq_remapping().
> 
> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> early stage like 1) shows, kernel will panic.

That's because acpi_put_table() will make the table pointer be NULL,
while dmar_table_init() will skip parse_dmar_table() calling if
dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().

Dmar hardware support interrupt remapping and io remapping separately. But
intel_iommu_init() is called later than intel_prepare_irq_remapping().
So what if make dmar_table_init() a reentrant function? You can just
have a try, but maybe not a good idea, the dmar table will be parsed
twice.

> 
> 
> Thanks,
> 
> 	dou.
> > 
> > Thanks
> > Lv
> > 
> > > From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> > > Sent: Friday, July 14, 2017 1:53 PM
> > > To: x86@kernel.org; linux-kernel@vger.kernel.org
> > > Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
> > > peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
> > > <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
> > > Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
> > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > > 
> > > Linux uses acpi_early_init() to put the ACPI table management into
> > > the late stage from the early stage where the mapped ACPI tables is
> > > temporary and should be unmapped.
> > > 
> > > But, now initializing interrupt delivery mode should map and parse the
> > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > the DMAR table after using in the early stage.
> > > 
> > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > be mapped and parsed in late stage like before.
> > > 
> > > Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> > > Cc: linux-acpi@vger.kernel.org
> > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > Cc: Zheng, Lv <lv.zheng@intel.com>
> > > Cc: Julian Wollrath <jwollrath@web.de>
> > > ---
> > > 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").
> > > 
> > >  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();
> > > --
> > > 2.5.5
> > > 
> > > 
> > 
> > 
> > 
> > 
> 
> 

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

[Dou Liyang]

Hi Baoquan,

At 07/18/2017 04:45 PM, bhe@redhat.com wrote:
> On 07/18/17 at 02:08pm, Dou Liyang wrote:
>> Hi, Zheng
>>
>> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
>>> Hi,
>>>
>>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
>>
>> Invoking acpi_put_table() is my first choice. But it made the kernel
>> *panic* when we try to get the table again in intel_iommu_init() in
>> late stage.
>>
>> I am also confused that:
>>
>> There are two places where we used DMAR table in Linux:
>>
>> 1) In detect_intel_iommu() in ACPI early stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ....
>> if (dmar_tbl) {
>> 	acpi_put_table(dmar_tbl);
>> 	dmar_tbl = NULL;
>> }
>>
>> 2) In dmar_table_init() in ACPI late stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ...
>>
>> As we know, dmar_table_init() is called by intel_iommu_init() and
>> intel_prepare_irq_remapping().
>>
>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>> early stage like 1) shows, kernel will panic.
>
> That's because acpi_put_table() will make the table pointer be NULL,
> while dmar_table_init() will skip parse_dmar_table() calling if
> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>

Correctly.

I have considered and removed the *dmar_table_initialized* in this
situation. So, dmar_table_init() didn't skip parse_dmar_table()
calling.

I didn't dig into the cause, I think it is interesting, I will do it
right now and share with you later.

> Dmar hardware support interrupt remapping and io remapping separately. But
> intel_iommu_init() is called later than intel_prepare_irq_remapping().
> So what if make dmar_table_init() a reentrant function? You can just
> have a try, but maybe not a good idea, the dmar table will be parsed
> twice.

Yes, It is precisely one reason that I gave up invoking
acpi_put_table().

Thanks,

	dou.

>
>>
>>
>> Thanks,
>>
>> 	dou.
>>>
>>> Thanks
>>> Lv
>>>
>>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
>>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>> Cc: linux-acpi@vger.kernel.org
>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>> ---
>>>> 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").
>>>>
>>>>  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();
>>>> --
>>>> 2.5.5
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>

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

[Zheng, Lv]

Hi,

> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> Sent: Tuesday, July 18, 2017 5:44 PM
> Subject: Re: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> 
> Hi Baoquan,
> 
> At 07/18/2017 04:45 PM, bhe@redhat.com wrote:
> > On 07/18/17 at 02:08pm, Dou Liyang wrote:
> >> Hi, Zheng
> >>
> >> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
> >>
> >> Invoking acpi_put_table() is my first choice. But it made the kernel
> >> *panic* when we try to get the table again in intel_iommu_init() in
> >> late stage.
> >>
> >> I am also confused that:
> >>
> >> There are two places where we used DMAR table in Linux:
> >>
> >> 1) In detect_intel_iommu() in ACPI early stage:
> >>
> >> ...
> >> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> >> ....
> >> if (dmar_tbl) {
> >> 	acpi_put_table(dmar_tbl);
> >> 	dmar_tbl = NULL;
> >> }
> >>
> >> 2) In dmar_table_init() in ACPI late stage:
> >>
> >> ...
> >> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> >> ...
> >>
> >> As we know, dmar_table_init() is called by intel_iommu_init() and
> >> intel_prepare_irq_remapping().
> >>
> >> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> >> early stage like 1) shows, kernel will panic.
> >
> > That's because acpi_put_table() will make the table pointer be NULL,
> > while dmar_table_init() will skip parse_dmar_table() calling if
> > dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
> >
> 
> Correctly.
> 
> I have considered and removed the *dmar_table_initialized* in this
> situation. So, dmar_table_init() didn't skip parse_dmar_table()
> calling.
> 
> I didn't dig into the cause, I think it is interesting, I will do it
> right now and share with you later.
> 
> > Dmar hardware support interrupt remapping and io remapping separately. But
> > intel_iommu_init() is called later than intel_prepare_irq_remapping().
> > So what if make dmar_table_init() a reentrant function? You can just
> > have a try, but maybe not a good idea, the dmar table will be parsed
> > twice.
> 
> Yes, It is precisely one reason that I gave up invoking
> acpi_put_table().

Parsing a table twice is not a problem on x86.
If you check the code, there are many examples.
It's actually required if you want to use a table both in early stage and late stage.

Thanks

> 
> Thanks,
> 
> 	dou.
> 
> >
> >>
> >>
> >> Thanks,
> >>
> >> 	dou.
> >>>
> >>> Thanks
> >>> Lv
> >>>
> >>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> >>>> Sent: Friday, July 14, 2017 1:53 PM
> >>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
> >>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
> >>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
> >>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>;
> Zheng,
> >>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
> >>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> >>>>
> >>>> Linux uses acpi_early_init() to put the ACPI table management into
> >>>> the late stage from the early stage where the mapped ACPI tables is
> >>>> temporary and should be unmapped.
> >>>>
> >>>> But, now initializing interrupt delivery mode should map and parse the
> >>>> DMAR table earlier in the early stage. This causes an ACPI error when
> >>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >>>> the DMAR table after using in the early stage.
> >>>>
> >>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >>>> be mapped and parsed in late stage like before.
> >>>>
> >>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >>>> Cc: linux-acpi@vger.kernel.org
> >>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> >>>> Cc: Zheng, Lv <lv.zheng@intel.com>
> >>>> Cc: Julian Wollrath <jwollrath@web.de>
> >>>> ---
> >>>> 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").
> >>>>
> >>>>  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();
> >>>> --
> >>>> 2.5.5
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
> 

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

[Dou Liyang]

Hi, Zheng

At 07/28/2017 09:53 AM, Zheng, Lv wrote:
[...]
>>
>>> Dmar hardware support interrupt remapping and io remapping separately. But
>>> intel_iommu_init() is called later than intel_prepare_irq_remapping().
>>> So what if make dmar_table_init() a reentrant function? You can just
>>> have a try, but maybe not a good idea, the dmar table will be parsed
>>> twice.
>>
>> Yes, It is precisely one reason that I gave up invoking
>> acpi_put_table().
>
> Parsing a table twice is not a problem on x86.
> If you check the code, there are many examples.
> It's actually required if you want to use a table both in early stage and late stage.
>

I am sorry I don't explain clearly.

Yeah, in current kernel, The DMAR table has been parsed twice as well.
one is in detect_intel_iommu(), the other is in dmar_table_init().

Here we focus on the reentrant dmar_table_init(), this func contains
many callback for parsing different DMAR type structures, such as DRHD,
RMRR.

acpi_put_table() can release the mapped DMAR address, but we should
clear the remapping structure types manually. and I think parsing these
DMAR type structures again may have an influence on interrupt remapping.


Thanks,

	dou.

> Thanks
>
>>
>> Thanks,
>>
>> 	dou.
>>
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> 	dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>>>>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>>>>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>>>>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>;
>> Zheng,
>>>>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> Cc: linux-acpi@vger.kernel.org
>>>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>>>> ---
>>>>>> 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").
>>>>>>
>>>>>>  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();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>
>
>
>

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

[Dou Liyang]

Hi Baoquan,

At 07/18/2017 04:45 PM, bhe@redhat.com wrote:
> On 07/18/17 at 02:08pm, Dou Liyang wrote:
>> Hi, Zheng
>>
>> At 07/18/2017 01:18 PM, Zheng, Lv wrote:
>>> Hi,
>>>
>>> Can the problem be fixed by invoking acpi_put_table() for mapped DMAR table?
>>
>> Invoking acpi_put_table() is my first choice. But it made the kernel
>> *panic* when we try to get the table again in intel_iommu_init() in
>> late stage.
>>
>> I am also confused that:
>>
>> There are two places where we used DMAR table in Linux:
>>
>> 1) In detect_intel_iommu() in ACPI early stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ....
>> if (dmar_tbl) {
>> 	acpi_put_table(dmar_tbl);
>> 	dmar_tbl = NULL;
>> }
>>
>> 2) In dmar_table_init() in ACPI late stage:
>>
>> ...
>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>> ...
>>
>> As we know, dmar_table_init() is called by intel_iommu_init() and
>> intel_prepare_irq_remapping().
>>
>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>> early stage like 1) shows, kernel will panic.
>
> That's because acpi_put_table() will make the table pointer be NULL,
> while dmar_table_init() will skip parse_dmar_table() calling if
> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>
> Dmar hardware support interrupt remapping and io remapping separately. But
> intel_iommu_init() is called later than intel_prepare_irq_remapping().
> So what if make dmar_table_init() a reentrant function? You can just
> have a try, but maybe not a good idea, the dmar table will be parsed
> twice.

The true reason why the kernel panic is that acpi_put_table() only
released DMAR table structure, but not released the remapping
structures in DMAR table, such as DRHD, RMRR. So the address of
RMRR parsed in early ACPI stage will be used in late ACPI stage in
intel_iommu_init(), which make the kernel panic.

The solution is invoking the intel_iommu_free_dmars() before
dmar_table_init() in intel_iommu_init() to release the RMRR.
Demo code will show at the bottom.

I prefer to invoke acpi_early_init() earlier. But it needs a regression
test[1].

I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
it in Thinkpad s430, It's OK.

BTY, I am confused how does the ACPI subsystem affect PIT which
will be used to fast calibrate CPU frequency[2].

Do you have any idea?

[1] https://lkml.org/lkml/2014/3/10/123
[2] https://lkml.org/lkml/2014/3/12/3


  drivers/iommu/dmar.c                | 27 +++++++++++----------------
  drivers/iommu/intel-iommu.c         |  2 ++
  drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
  include/linux/dmar.h                |  2 ++
  init/main.c                         |  2 +-
  5 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index c8b0329..e6261b7 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
  LIST_HEAD(dmar_drhd_units);

  struct acpi_table_header * __initdata dmar_tbl;
+struct acpi_table_header * __initdata dmar_tbl_original;
+
  static int dmar_dev_scope_status = 1;
  static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];

@@ -627,6 +629,7 @@ parse_dmar_table(void)
  	 * fixed map.
  	 */
  	dmar_table_detect();
+	dmar_tbl_original = dmar_tbl;

  	/*
  	 * ACPI tables may not be DMA protected by tboot, so use DMAR copy
@@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)

  int __init dmar_table_init(void)
  {
-	static int dmar_table_initialized;
  	int ret;

-	if (dmar_table_initialized == 0) {
-		ret = parse_dmar_table();
-		if (ret < 0) {
-			if (ret != -ENODEV)
-				pr_info("Parse DMAR table failure.\n");
-		} else  if (list_empty(&dmar_drhd_units)) {
-			pr_info("No DMAR devices found\n");
-			ret = -ENODEV;
-		}
-
-		if (ret < 0)
-			dmar_table_initialized = ret;
-		else
-			dmar_table_initialized = 1;
+	ret = parse_dmar_table();
+	if (ret < 0) {
+		if (ret != -ENODEV)
+			pr_info("Parse DMAR table failure.\n");
+	} else  if (list_empty(&dmar_drhd_units)) {
+		pr_info("No DMAR devices found\n");
+		ret = -ENODEV;
  	}

-	return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
+	return ret;
  }

  static void warn_invalid_dmar(u64 addr, const char *message)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 687f18f..90f74f4 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
  	}

  	down_write(&dmar_global_lock);
+
+	intel_iommu_free_dmars();
  	if (dmar_table_init()) {
  		if (force_on)
  			panic("tboot: Failed to initialize DMAR table\n");
diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index a5b89f6..ccaacda 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
  		pr_warn("Failed to enable irq remapping. You are vulnerable to 
irq-injection attacks.\n");
  }

-static int __init intel_prepare_irq_remapping(void)
+static int __init __intel_prepare_irq_remapping(void)
  {
  	struct dmar_drhd_unit *drhd;
  	struct intel_iommu *iommu;
@@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
  	return -ENODEV;
  }

+static int __init intel_prepare_irq_remapping(void)
+{
+	int ret;
+
+	ret = __intel_prepare_irq_remapping();
+
+	if (dmar_tbl_original) {
+		acpi_put_table(dmar_tbl_original);
+		dmar_tbl_original = NULL;
+		dmar_tbl = NULL;
+	}
+
+	return ret;
+}
+
  /*
   * Set Posted-Interrupts capability.
   */
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index e8ffba1..987b076 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -50,6 +50,8 @@ struct dmar_dev_scope {

  #ifdef CONFIG_DMAR_TABLE
  extern struct acpi_table_header *dmar_tbl;
+extern struct acpi_table_header *dmar_tbl_original;
+
  struct dmar_drhd_unit {
  	struct list_head list;		/* list of drhd units	*/
  	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
diff --git a/init/main.c b/init/main.c
index 52dee20..052481f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -655,12 +655,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();

Thanks,
	dou.
>
>>
>>
>> Thanks,
>>
>> 	dou.
>>>
>>> Thanks
>>> Lv
>>>
>>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
>>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>> Cc: linux-acpi@vger.kernel.org
>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>> ---
>>>> 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").
>>>>
>>>>  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();
>>>> --
>>>> 2.5.5
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>

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

[bhe]

On 07/26/17 at 08:19pm, Dou Liyang wrote:
> Hi Baoquan,
> > > There are two places where we used DMAR table in Linux:
> > > 
> > > 1) In detect_intel_iommu() in ACPI early stage:
> > > 
> > > ...
> > > status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> > > ....
> > > if (dmar_tbl) {
> > > 	acpi_put_table(dmar_tbl);
> > > 	dmar_tbl = NULL;
> > > }
> > > 
> > > 2) In dmar_table_init() in ACPI late stage:
> > > 
> > > ...
> > > status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
> > > ...
> > > 
> > > As we know, dmar_table_init() is called by intel_iommu_init() and
> > > intel_prepare_irq_remapping().
> > > 
> > > When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
> > > early stage like 1) shows, kernel will panic.
> > 
> > That's because acpi_put_table() will make the table pointer be NULL,
> > while dmar_table_init() will skip parse_dmar_table() calling if
> > dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
> > 
> > Dmar hardware support interrupt remapping and io remapping separately. But
> > intel_iommu_init() is called later than intel_prepare_irq_remapping().
> > So what if make dmar_table_init() a reentrant function? You can just
> > have a try, but maybe not a good idea, the dmar table will be parsed
> > twice.
> 
> The true reason why the kernel panic is that acpi_put_table() only
> released DMAR table structure, but not released the remapping
> structures in DMAR table, such as DRHD, RMRR. So the address of
> RMRR parsed in early ACPI stage will be used in late ACPI stage in
> intel_iommu_init(), which make the kernel panic.
> 
> The solution is invoking the intel_iommu_free_dmars() before
> dmar_table_init() in intel_iommu_init() to release the RMRR.
> Demo code will show at the bottom.
> 
> I prefer to invoke acpi_early_init() earlier. But it needs a regression
> test[1].

Good work, in fact not only intel iommu matters here, I gues you haven't
tried amd system with a AMD-VI which does the amd iommu work. It's
similar to intel iommu and the same principle but different acpi tables.

So it's fine you want to put acpi_early_init earlier.

> 
> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
> it in Thinkpad s430, It's OK.
> 
> BTY, I am confused how does the ACPI subsystem affect PIT which
> will be used to fast calibrate CPU frequency[2].

I checked code, and have no any idea. Add Joey Lee to list, see if he
can help answer this.

> 
> Do you have any idea?
> 
> [1] https://lkml.org/lkml/2014/3/10/123
> [2] https://lkml.org/lkml/2014/3/12/3
> 
> 
>  drivers/iommu/dmar.c                | 27 +++++++++++----------------
>  drivers/iommu/intel-iommu.c         |  2 ++
>  drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
>  include/linux/dmar.h                |  2 ++
>  init/main.c                         |  2 +-
>  5 files changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index c8b0329..e6261b7 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
>  LIST_HEAD(dmar_drhd_units);
> 
>  struct acpi_table_header * __initdata dmar_tbl;
> +struct acpi_table_header * __initdata dmar_tbl_original;
> +
>  static int dmar_dev_scope_status = 1;
>  static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
> 
> @@ -627,6 +629,7 @@ parse_dmar_table(void)
>  	 * fixed map.
>  	 */
>  	dmar_table_detect();
> +	dmar_tbl_original = dmar_tbl;
> 
>  	/*
>  	 * ACPI tables may not be DMA protected by tboot, so use DMAR copy
> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
> 
>  int __init dmar_table_init(void)
>  {
> -	static int dmar_table_initialized;
>  	int ret;
> 
> -	if (dmar_table_initialized == 0) {
> -		ret = parse_dmar_table();
> -		if (ret < 0) {
> -			if (ret != -ENODEV)
> -				pr_info("Parse DMAR table failure.\n");
> -		} else  if (list_empty(&dmar_drhd_units)) {
> -			pr_info("No DMAR devices found\n");
> -			ret = -ENODEV;
> -		}
> -
> -		if (ret < 0)
> -			dmar_table_initialized = ret;
> -		else
> -			dmar_table_initialized = 1;
> +	ret = parse_dmar_table();
> +	if (ret < 0) {
> +		if (ret != -ENODEV)
> +			pr_info("Parse DMAR table failure.\n");
> +	} else  if (list_empty(&dmar_drhd_units)) {
> +		pr_info("No DMAR devices found\n");
> +		ret = -ENODEV;
>  	}
> 
> -	return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
> +	return ret;
>  }
> 
>  static void warn_invalid_dmar(u64 addr, const char *message)
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 687f18f..90f74f4 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
>  	}
> 
>  	down_write(&dmar_global_lock);
> +
> +	intel_iommu_free_dmars();
>  	if (dmar_table_init()) {
>  		if (force_on)
>  			panic("tboot: Failed to initialize DMAR table\n");
> diff --git a/drivers/iommu/intel_irq_remapping.c
> b/drivers/iommu/intel_irq_remapping.c
> index a5b89f6..ccaacda 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
>  		pr_warn("Failed to enable irq remapping. You are vulnerable to
> irq-injection attacks.\n");
>  }
> 
> -static int __init intel_prepare_irq_remapping(void)
> +static int __init __intel_prepare_irq_remapping(void)
>  {
>  	struct dmar_drhd_unit *drhd;
>  	struct intel_iommu *iommu;
> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
>  	return -ENODEV;
>  }
> 
> +static int __init intel_prepare_irq_remapping(void)
> +{
> +	int ret;
> +
> +	ret = __intel_prepare_irq_remapping();
> +
> +	if (dmar_tbl_original) {
> +		acpi_put_table(dmar_tbl_original);
> +		dmar_tbl_original = NULL;
> +		dmar_tbl = NULL;
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * Set Posted-Interrupts capability.
>   */
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index e8ffba1..987b076 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
> 
>  #ifdef CONFIG_DMAR_TABLE
>  extern struct acpi_table_header *dmar_tbl;
> +extern struct acpi_table_header *dmar_tbl_original;
> +
>  struct dmar_drhd_unit {
>  	struct list_head list;		/* list of drhd units	*/
>  	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
> diff --git a/init/main.c b/init/main.c
> index 52dee20..052481f 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -655,12 +655,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();
> 
> Thanks,
> 	dou.
> > 
> > > 
> > > 
> > > Thanks,
> > > 
> > > 	dou.
> > > > 
> > > > Thanks
> > > > Lv
> > > > 
> > > > > From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> > > > > Sent: Friday, July 14, 2017 1:53 PM
> > > > > To: x86@kernel.org; linux-kernel@vger.kernel.org
> > > > > Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
> > > > > peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
> > > > > <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
> > > > > Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
> > > > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > > > > 
> > > > > Linux uses acpi_early_init() to put the ACPI table management into
> > > > > the late stage from the early stage where the mapped ACPI tables is
> > > > > temporary and should be unmapped.
> > > > > 
> > > > > But, now initializing interrupt delivery mode should map and parse the
> > > > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > > > the DMAR table after using in the early stage.
> > > > > 
> > > > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > > > be mapped and parsed in late stage like before.
> > > > > 
> > > > > Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> > > > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> > > > > Cc: linux-acpi@vger.kernel.org
> > > > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > > > Cc: Zheng, Lv <lv.zheng@intel.com>
> > > > > Cc: Julian Wollrath <jwollrath@web.de>
> > > > > ---
> > > > > 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").
> > > > > 
> > > > >  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();
> > > > > --
> > > > > 2.5.5
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> 
> 

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

[Dou Liyang]

Hi Baoquan,

At 07/27/2017 02:08 PM, bhe@redhat.com wrote:
> On 07/26/17 at 08:19pm, Dou Liyang wrote:
>> Hi Baoquan,
>>>> There are two places where we used DMAR table in Linux:
>>>>
>>>> 1) In detect_intel_iommu() in ACPI early stage:
>>>>
>>>> ...
>>>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>>>> ....
>>>> if (dmar_tbl) {
>>>> 	acpi_put_table(dmar_tbl);
>>>> 	dmar_tbl = NULL;
>>>> }
>>>>
>>>> 2) In dmar_table_init() in ACPI late stage:
>>>>
>>>> ...
>>>> status = acpi_get_table(ACPI_SIG_DMAR, 0, &dmar_tbl);
>>>> ...
>>>>
>>>> As we know, dmar_table_init() is called by intel_iommu_init() and
>>>> intel_prepare_irq_remapping().
>>>>
>>>> When I invoked acpi_put_table() in the intel_prepare_irq_remapping() in
>>>> early stage like 1) shows, kernel will panic.
>>>
>>> That's because acpi_put_table() will make the table pointer be NULL,
>>> while dmar_table_init() will skip parse_dmar_table() calling if
>>> dmar_table_initialized is set to 1 in intel_prepare_irq_remapping().
>>>
>>> Dmar hardware support interrupt remapping and io remapping separately. But
>>> intel_iommu_init() is called later than intel_prepare_irq_remapping().
>>> So what if make dmar_table_init() a reentrant function? You can just
>>> have a try, but maybe not a good idea, the dmar table will be parsed
>>> twice.
>>
>> The true reason why the kernel panic is that acpi_put_table() only
>> released DMAR table structure, but not released the remapping
>> structures in DMAR table, such as DRHD, RMRR. So the address of
>> RMRR parsed in early ACPI stage will be used in late ACPI stage in
>> intel_iommu_init(), which make the kernel panic.
>>
>> The solution is invoking the intel_iommu_free_dmars() before
>> dmar_table_init() in intel_iommu_init() to release the RMRR.
>> Demo code will show at the bottom.
>>
>> I prefer to invoke acpi_early_init() earlier. But it needs a regression
>> test[1].
>
> Good work, in fact not only intel iommu matters here, I gues you haven't
> tried amd system with a AMD-VI which does the amd iommu work. It's
> similar to intel iommu and the same principle but different acpi tables.

Yes, I missed it, I didn't try it in AMD platform. If I want to use
acpi_put_table(), I should consider both the Intel and AMD platform.

> So it's fine you want to put acpi_early_init earlier.

Got it.

>
>>
>> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
>> it in Thinkpad s430, It's OK.
>>
>> BTY, I am confused how does the ACPI subsystem affect PIT which
>> will be used to fast calibrate CPU frequency[2].
>
> I checked code, and have no any idea. Add Joey Lee to list, see if he
> can help answer this.

Thank you very much.

sincerely,

	dou.

>
>>
>> Do you have any idea?
>>
>> [1] https://lkml.org/lkml/2014/3/10/123
>> [2] https://lkml.org/lkml/2014/3/12/3
>>
>>
>>  drivers/iommu/dmar.c                | 27 +++++++++++----------------
>>  drivers/iommu/intel-iommu.c         |  2 ++
>>  drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
>>  include/linux/dmar.h                |  2 ++
>>  init/main.c                         |  2 +-
>>  5 files changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
>> index c8b0329..e6261b7 100644
>> --- a/drivers/iommu/dmar.c
>> +++ b/drivers/iommu/dmar.c
>> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
>>  LIST_HEAD(dmar_drhd_units);
>>
>>  struct acpi_table_header * __initdata dmar_tbl;
>> +struct acpi_table_header * __initdata dmar_tbl_original;
>> +
>>  static int dmar_dev_scope_status = 1;
>>  static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
>>
>> @@ -627,6 +629,7 @@ parse_dmar_table(void)
>>  	 * fixed map.
>>  	 */
>>  	dmar_table_detect();
>> +	dmar_tbl_original = dmar_tbl;
>>
>>  	/*
>>  	 * ACPI tables may not be DMA protected by tboot, so use DMAR copy
>> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
>>
>>  int __init dmar_table_init(void)
>>  {
>> -	static int dmar_table_initialized;
>>  	int ret;
>>
>> -	if (dmar_table_initialized == 0) {
>> -		ret = parse_dmar_table();
>> -		if (ret < 0) {
>> -			if (ret != -ENODEV)
>> -				pr_info("Parse DMAR table failure.\n");
>> -		} else  if (list_empty(&dmar_drhd_units)) {
>> -			pr_info("No DMAR devices found\n");
>> -			ret = -ENODEV;
>> -		}
>> -
>> -		if (ret < 0)
>> -			dmar_table_initialized = ret;
>> -		else
>> -			dmar_table_initialized = 1;
>> +	ret = parse_dmar_table();
>> +	if (ret < 0) {
>> +		if (ret != -ENODEV)
>> +			pr_info("Parse DMAR table failure.\n");
>> +	} else  if (list_empty(&dmar_drhd_units)) {
>> +		pr_info("No DMAR devices found\n");
>> +		ret = -ENODEV;
>>  	}
>>
>> -	return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
>> +	return ret;
>>  }
>>
>>  static void warn_invalid_dmar(u64 addr, const char *message)
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 687f18f..90f74f4 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
>>  	}
>>
>>  	down_write(&dmar_global_lock);
>> +
>> +	intel_iommu_free_dmars();
>>  	if (dmar_table_init()) {
>>  		if (force_on)
>>  			panic("tboot: Failed to initialize DMAR table\n");
>> diff --git a/drivers/iommu/intel_irq_remapping.c
>> b/drivers/iommu/intel_irq_remapping.c
>> index a5b89f6..ccaacda 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
>>  		pr_warn("Failed to enable irq remapping. You are vulnerable to
>> irq-injection attacks.\n");
>>  }
>>
>> -static int __init intel_prepare_irq_remapping(void)
>> +static int __init __intel_prepare_irq_remapping(void)
>>  {
>>  	struct dmar_drhd_unit *drhd;
>>  	struct intel_iommu *iommu;
>> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
>>  	return -ENODEV;
>>  }
>>
>> +static int __init intel_prepare_irq_remapping(void)
>> +{
>> +	int ret;
>> +
>> +	ret = __intel_prepare_irq_remapping();
>> +
>> +	if (dmar_tbl_original) {
>> +		acpi_put_table(dmar_tbl_original);
>> +		dmar_tbl_original = NULL;
>> +		dmar_tbl = NULL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  /*
>>   * Set Posted-Interrupts capability.
>>   */
>> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>> index e8ffba1..987b076 100644
>> --- a/include/linux/dmar.h
>> +++ b/include/linux/dmar.h
>> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
>>
>>  #ifdef CONFIG_DMAR_TABLE
>>  extern struct acpi_table_header *dmar_tbl;
>> +extern struct acpi_table_header *dmar_tbl_original;
>> +
>>  struct dmar_drhd_unit {
>>  	struct list_head list;		/* list of drhd units	*/
>>  	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
>> diff --git a/init/main.c b/init/main.c
>> index 52dee20..052481f 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -655,12 +655,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();
>>
>> Thanks,
>> 	dou.
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> 	dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>>>>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>>>>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>>>>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
>>>>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> Cc: linux-acpi@vger.kernel.org
>>>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>>>> ---
>>>>>> 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").
>>>>>>
>>>>>>  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();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>

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

[Dou Liyang]

Hi Baoquan,

At 07/27/2017 02:08 PM, bhe@redhat.com wrote:
> On 07/26/17 at 08:19pm, Dou Liyang wrote:
>> Hi Baoquan,
[...]
>>
>> I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
>> it in Thinkpad s430, It's OK.
>>
>> BTY, I am confused how does the ACPI subsystem affect PIT which
>> will be used to fast calibrate CPU frequency[2].

In combination with what Joey said[1] and the code, I guess,

The acpi_enable() called in acpi_enable_system() transfers the system
into ACPI mode and write data to an I/O port. PIT works using the I/O
port. In Thinkpad x121e with ACPI mode, the counter in PIT couldn't
count right and might got an error value.

[1] https://lkml.org/lkml/2014/3/12/3

Thanks,
	dou.
>
> I checked code, and have no any idea. Add Joey Lee to list, see if he
> can help answer this.
>
>>
>> Do you have any idea?
>>
>> [1] https://lkml.org/lkml/2014/3/10/123
>> [2] https://lkml.org/lkml/2014/3/12/3
>>
>>
>>  drivers/iommu/dmar.c                | 27 +++++++++++----------------
>>  drivers/iommu/intel-iommu.c         |  2 ++
>>  drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
>>  include/linux/dmar.h                |  2 ++
>>  init/main.c                         |  2 +-
>>  5 files changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
>> index c8b0329..e6261b7 100644
>> --- a/drivers/iommu/dmar.c
>> +++ b/drivers/iommu/dmar.c
>> @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
>>  LIST_HEAD(dmar_drhd_units);
>>
>>  struct acpi_table_header * __initdata dmar_tbl;
>> +struct acpi_table_header * __initdata dmar_tbl_original;
>> +
>>  static int dmar_dev_scope_status = 1;
>>  static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
>>
>> @@ -627,6 +629,7 @@ parse_dmar_table(void)
>>  	 * fixed map.
>>  	 */
>>  	dmar_table_detect();
>> +	dmar_tbl_original = dmar_tbl;
>>
>>  	/*
>>  	 * ACPI tables may not be DMA protected by tboot, so use DMAR copy
>> @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
>>
>>  int __init dmar_table_init(void)
>>  {
>> -	static int dmar_table_initialized;
>>  	int ret;
>>
>> -	if (dmar_table_initialized == 0) {
>> -		ret = parse_dmar_table();
>> -		if (ret < 0) {
>> -			if (ret != -ENODEV)
>> -				pr_info("Parse DMAR table failure.\n");
>> -		} else  if (list_empty(&dmar_drhd_units)) {
>> -			pr_info("No DMAR devices found\n");
>> -			ret = -ENODEV;
>> -		}
>> -
>> -		if (ret < 0)
>> -			dmar_table_initialized = ret;
>> -		else
>> -			dmar_table_initialized = 1;
>> +	ret = parse_dmar_table();
>> +	if (ret < 0) {
>> +		if (ret != -ENODEV)
>> +			pr_info("Parse DMAR table failure.\n");
>> +	} else  if (list_empty(&dmar_drhd_units)) {
>> +		pr_info("No DMAR devices found\n");
>> +		ret = -ENODEV;
>>  	}
>>
>> -	return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
>> +	return ret;
>>  }
>>
>>  static void warn_invalid_dmar(u64 addr, const char *message)
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 687f18f..90f74f4 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
>>  	}
>>
>>  	down_write(&dmar_global_lock);
>> +
>> +	intel_iommu_free_dmars();
>>  	if (dmar_table_init()) {
>>  		if (force_on)
>>  			panic("tboot: Failed to initialize DMAR table\n");
>> diff --git a/drivers/iommu/intel_irq_remapping.c
>> b/drivers/iommu/intel_irq_remapping.c
>> index a5b89f6..ccaacda 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
>>  		pr_warn("Failed to enable irq remapping. You are vulnerable to
>> irq-injection attacks.\n");
>>  }
>>
>> -static int __init intel_prepare_irq_remapping(void)
>> +static int __init __intel_prepare_irq_remapping(void)
>>  {
>>  	struct dmar_drhd_unit *drhd;
>>  	struct intel_iommu *iommu;
>> @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
>>  	return -ENODEV;
>>  }
>>
>> +static int __init intel_prepare_irq_remapping(void)
>> +{
>> +	int ret;
>> +
>> +	ret = __intel_prepare_irq_remapping();
>> +
>> +	if (dmar_tbl_original) {
>> +		acpi_put_table(dmar_tbl_original);
>> +		dmar_tbl_original = NULL;
>> +		dmar_tbl = NULL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  /*
>>   * Set Posted-Interrupts capability.
>>   */
>> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>> index e8ffba1..987b076 100644
>> --- a/include/linux/dmar.h
>> +++ b/include/linux/dmar.h
>> @@ -50,6 +50,8 @@ struct dmar_dev_scope {
>>
>>  #ifdef CONFIG_DMAR_TABLE
>>  extern struct acpi_table_header *dmar_tbl;
>> +extern struct acpi_table_header *dmar_tbl_original;
>> +
>>  struct dmar_drhd_unit {
>>  	struct list_head list;		/* list of drhd units	*/
>>  	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
>> diff --git a/init/main.c b/init/main.c
>> index 52dee20..052481f 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -655,12 +655,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();
>>
>> Thanks,
>> 	dou.
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> 	dou.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>> From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
>>>>>> Sent: Friday, July 14, 2017 1:53 PM
>>>>>> To: x86@kernel.org; linux-kernel@vger.kernel.org
>>>>>> Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
>>>>>> peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
>>>>>> <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
>>>>>> Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
>>>>>> Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
>>>>>>
>>>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>>>> the late stage from the early stage where the mapped ACPI tables is
>>>>>> temporary and should be unmapped.
>>>>>>
>>>>>> But, now initializing interrupt delivery mode should map and parse the
>>>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>>>> the DMAR table after using in the early stage.
>>>>>>
>>>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>>>> be mapped and parsed in late stage like before.
>>>>>>
>>>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> Cc: linux-acpi@vger.kernel.org
>>>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>>>> ---
>>>>>> 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").
>>>>>>
>>>>>>  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();
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>

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

[bhe]

On 07/31/17 at 07:20pm, Dou Liyang wrote:
> Hi Baoquan,
> 
> At 07/27/2017 02:08 PM, bhe@redhat.com wrote:
> > On 07/26/17 at 08:19pm, Dou Liyang wrote:
> > > Hi Baoquan,
> [...]
> > > 
> > > I am looking for Thinkpad x121e (AMD E-450 APU) to test. I have tested
> > > it in Thinkpad s430, It's OK.
> > > 
> > > BTY, I am confused how does the ACPI subsystem affect PIT which
> > > will be used to fast calibrate CPU frequency[2].
> 
> In combination with what Joey said[1] and the code, I guess,
> 
> The acpi_enable() called in acpi_enable_system() transfers the system
> into ACPI mode and write data to an I/O port. PIT works using the I/O
> port. In Thinkpad x121e with ACPI mode, the counter in PIT couldn't
> count right and might got an error value.
> 
> [1] https://lkml.org/lkml/2014/3/12/3

Awesome! I guess you are possibly correct. A MUST job for entering
into ACPI mode is to enable SCI and hook a handler. SCI will take over
all acpi interrupt events. I googled SCI and vaguely remember someone
said the I/O port r/w event also will be handled by SCI, but didn't find
a clear description in ACPI spec. If that cause failure, it might
happen inside pit_expect_msb() called by quick_pit_calibrate(), maybe
you can try to add debug message in this place, not very sure if it
be debugged.

Maybe tglx or Rafael can help have a look and confirm if it's correct
or not.

Thanks
Baoquan

> > 
> > I checked code, and have no any idea. Add Joey Lee to list, see if he
> > can help answer this.
> > 
> > > 
> > > Do you have any idea?
> > > 
> > > [1] https://lkml.org/lkml/2014/3/10/123
> > > [2] https://lkml.org/lkml/2014/3/12/3
> > > 
> > > 
> > >  drivers/iommu/dmar.c                | 27 +++++++++++----------------
> > >  drivers/iommu/intel-iommu.c         |  2 ++
> > >  drivers/iommu/intel_irq_remapping.c | 17 ++++++++++++++++-
> > >  include/linux/dmar.h                |  2 ++
> > >  init/main.c                         |  2 +-
> > >  5 files changed, 32 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> > > index c8b0329..e6261b7 100644
> > > --- a/drivers/iommu/dmar.c
> > > +++ b/drivers/iommu/dmar.c
> > > @@ -68,6 +68,8 @@ DECLARE_RWSEM(dmar_global_lock);
> > >  LIST_HEAD(dmar_drhd_units);
> > > 
> > >  struct acpi_table_header * __initdata dmar_tbl;
> > > +struct acpi_table_header * __initdata dmar_tbl_original;
> > > +
> > >  static int dmar_dev_scope_status = 1;
> > >  static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
> > > 
> > > @@ -627,6 +629,7 @@ parse_dmar_table(void)
> > >  	 * fixed map.
> > >  	 */
> > >  	dmar_table_detect();
> > > +	dmar_tbl_original = dmar_tbl;
> > > 
> > >  	/*
> > >  	 * ACPI tables may not be DMA protected by tboot, so use DMAR copy
> > > @@ -811,26 +814,18 @@ int __init dmar_dev_scope_init(void)
> > > 
> > >  int __init dmar_table_init(void)
> > >  {
> > > -	static int dmar_table_initialized;
> > >  	int ret;
> > > 
> > > -	if (dmar_table_initialized == 0) {
> > > -		ret = parse_dmar_table();
> > > -		if (ret < 0) {
> > > -			if (ret != -ENODEV)
> > > -				pr_info("Parse DMAR table failure.\n");
> > > -		} else  if (list_empty(&dmar_drhd_units)) {
> > > -			pr_info("No DMAR devices found\n");
> > > -			ret = -ENODEV;
> > > -		}
> > > -
> > > -		if (ret < 0)
> > > -			dmar_table_initialized = ret;
> > > -		else
> > > -			dmar_table_initialized = 1;
> > > +	ret = parse_dmar_table();
> > > +	if (ret < 0) {
> > > +		if (ret != -ENODEV)
> > > +			pr_info("Parse DMAR table failure.\n");
> > > +	} else  if (list_empty(&dmar_drhd_units)) {
> > > +		pr_info("No DMAR devices found\n");
> > > +		ret = -ENODEV;
> > >  	}
> > > 
> > > -	return dmar_table_initialized < 0 ? dmar_table_initialized : 0;
> > > +	return ret;
> > >  }
> > > 
> > >  static void warn_invalid_dmar(u64 addr, const char *message)
> > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > > index 687f18f..90f74f4 100644
> > > --- a/drivers/iommu/intel-iommu.c
> > > +++ b/drivers/iommu/intel-iommu.c
> > > @@ -4832,6 +4832,8 @@ int __init intel_iommu_init(void)
> > >  	}
> > > 
> > >  	down_write(&dmar_global_lock);
> > > +
> > > +	intel_iommu_free_dmars();
> > >  	if (dmar_table_init()) {
> > >  		if (force_on)
> > >  			panic("tboot: Failed to initialize DMAR table\n");
> > > diff --git a/drivers/iommu/intel_irq_remapping.c
> > > b/drivers/iommu/intel_irq_remapping.c
> > > index a5b89f6..ccaacda 100644
> > > --- a/drivers/iommu/intel_irq_remapping.c
> > > +++ b/drivers/iommu/intel_irq_remapping.c
> > > @@ -675,7 +675,7 @@ static void __init intel_cleanup_irq_remapping(void)
> > >  		pr_warn("Failed to enable irq remapping. You are vulnerable to
> > > irq-injection attacks.\n");
> > >  }
> > > 
> > > -static int __init intel_prepare_irq_remapping(void)
> > > +static int __init __intel_prepare_irq_remapping(void)
> > >  {
> > >  	struct dmar_drhd_unit *drhd;
> > >  	struct intel_iommu *iommu;
> > > @@ -743,6 +743,21 @@ static int __init intel_prepare_irq_remapping(void)
> > >  	return -ENODEV;
> > >  }
> > > 
> > > +static int __init intel_prepare_irq_remapping(void)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = __intel_prepare_irq_remapping();
> > > +
> > > +	if (dmar_tbl_original) {
> > > +		acpi_put_table(dmar_tbl_original);
> > > +		dmar_tbl_original = NULL;
> > > +		dmar_tbl = NULL;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  /*
> > >   * Set Posted-Interrupts capability.
> > >   */
> > > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > > index e8ffba1..987b076 100644
> > > --- a/include/linux/dmar.h
> > > +++ b/include/linux/dmar.h
> > > @@ -50,6 +50,8 @@ struct dmar_dev_scope {
> > > 
> > >  #ifdef CONFIG_DMAR_TABLE
> > >  extern struct acpi_table_header *dmar_tbl;
> > > +extern struct acpi_table_header *dmar_tbl_original;
> > > +
> > >  struct dmar_drhd_unit {
> > >  	struct list_head list;		/* list of drhd units	*/
> > >  	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
> > > diff --git a/init/main.c b/init/main.c
> > > index 52dee20..052481f 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -655,12 +655,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();
> > > 
> > > Thanks,
> > > 	dou.
> > > > 
> > > > > 
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > 	dou.
> > > > > > 
> > > > > > Thanks
> > > > > > Lv
> > > > > > 
> > > > > > > From: Dou Liyang [mailto:douly.fnst@cn.fujitsu.com]
> > > > > > > Sent: Friday, July 14, 2017 1:53 PM
> > > > > > > To: x86@kernel.org; linux-kernel@vger.kernel.org
> > > > > > > Cc: tglx@linutronix.de; mingo@kernel.org; hpa@zytor.com; ebiederm@xmission.com; bhe@redhat.com;
> > > > > > > peterz@infradead.org; izumi.taku@jp.fujitsu.com; tokunaga.keiich@jp.fujitsu.com; Dou Liyang
> > > > > > > <douly.fnst@cn.fujitsu.com>; linux-acpi@vger.kernel.org; Rafael J. Wysocki <rjw@rjwysocki.net>; Zheng,
> > > > > > > Lv <lv.zheng@intel.com>; Julian Wollrath <jwollrath@web.de>
> > > > > > > Subject: [PATCH v7 12/13] ACPI / init: Invoke early ACPI initialization earlier
> > > > > > > 
> > > > > > > Linux uses acpi_early_init() to put the ACPI table management into
> > > > > > > the late stage from the early stage where the mapped ACPI tables is
> > > > > > > temporary and should be unmapped.
> > > > > > > 
> > > > > > > But, now initializing interrupt delivery mode should map and parse the
> > > > > > > DMAR table earlier in the early stage. This causes an ACPI error when
> > > > > > > Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> > > > > > > the DMAR table after using in the early stage.
> > > > > > > 
> > > > > > > Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> > > > > > > be mapped and parsed in late stage like before.
> > > > > > > 
> > > > > > > Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> > > > > > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> > > > > > > Cc: linux-acpi@vger.kernel.org
> > > > > > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > > > > > Cc: Zheng, Lv <lv.zheng@intel.com>
> > > > > > > Cc: Julian Wollrath <jwollrath@web.de>
> > > > > > > ---
> > > > > > > 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").
> > > > > > > 
> > > > > > >  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();
> > > > > > > --
> > > > > > > 2.5.5
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> 
> 

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

[Dou Liyang]

Hi,

At 07/14/2017 01:52 PM, Dou Liyang wrote:
> Linux uses acpi_early_init() to put the ACPI table management into
> the late stage from the early stage where the mapped ACPI tables is
> temporary and should be unmapped.
>
> But, now initializing interrupt delivery mode should map and parse the
> DMAR table earlier in the early stage. This causes an ACPI error when
> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> the DMAR table after using in the early stage.
>
> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> be mapped and parsed in late stage like before.
>
> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Zheng, Lv <lv.zheng@intel.com>
> Cc: Julian Wollrath <jwollrath@web.de>
> ---
> 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
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.

[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();
>

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

[Dou Liyang]

Hi Rafael, Zheng,

At 07/31/2017 06:50 PM, Dou Liyang wrote:
> Hi,
>
> At 07/14/2017 01:52 PM, Dou Liyang wrote:
>> Linux uses acpi_early_init() to put the ACPI table management into
>> the late stage from the early stage where the mapped ACPI tables is
>> temporary and should be unmapped.
>>
>> But, now initializing interrupt delivery mode should map and parse the
>> DMAR table earlier in the early stage. This causes an ACPI error when
>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>> the DMAR table after using in the early stage.
>>
>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>> be mapped and parsed in late stage like before.
>>
>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>> Cc: linux-acpi@vger.kernel.org
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Zheng, Lv <lv.zheng@intel.com>
>> Cc: Julian Wollrath <jwollrath@web.de>
>> ---
>> 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
> 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();
>>

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

[Baoquan He]

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?

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();
> > > 
> 
> 

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

[Dou Liyang]

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();
>>>>
>>
>>
>
>
>

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

[Baoquan He]

On 08/24/17 at 05:28pm, Dou Liyang wrote:
> 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.

Thanks for telling!

It's truly weird that the TSC becomes unstable only if enabling ACPI
subsystem earlier than using PIT.

Let's see what other people say about this.

Btw, you will resend another round, right? Then I would like to test
your new post.

Thanks
Baoquan

> 
> 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();
> > > > > 
> > > 
> > > 
> > 
> > 
> > 
> 
> 

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

[Dou Liyang]

At 08/24/2017 06:21 PM, Baoquan He wrote:
> On 08/24/17 at 05:28pm, Dou Liyang wrote:
>> 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.
>
> Thanks for telling!
>
> It's truly weird that the TSC becomes unstable only if enabling ACPI
> subsystem earlier than using PIT.
>
> Let's see what other people say about this.
>
> Btw, you will resend another round, right? Then I would like to test
> your new post.

Yes, I am waiting ACPI maintainers advice and I prepare to re-base it
next week when rc7 is out.

Thanks,
	dou.

>
> Thanks
> Baoquan
>
>>
>> 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();
>>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>

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

[Rafael J. Wysocki]

On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
> Hi Rafael, Zheng,
> 
> At 07/31/2017 06:50 PM, Dou Liyang wrote:
> > Hi,
> >
> > At 07/14/2017 01:52 PM, Dou Liyang wrote:
> >> Linux uses acpi_early_init() to put the ACPI table management into
> >> the late stage from the early stage where the mapped ACPI tables is
> >> temporary and should be unmapped.
> >>
> >> But, now initializing interrupt delivery mode should map and parse the
> >> DMAR table earlier in the early stage. This causes an ACPI error when
> >> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >> the DMAR table after using in the early stage.
> >>
> >> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >> be mapped and parsed in late stage like before.
> >>
> >> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >> Cc: linux-acpi@vger.kernel.org
> >> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> >> Cc: Zheng, Lv <lv.zheng@intel.com>
> >> Cc: Julian Wollrath <jwollrath@web.de>
> >> ---
> >> 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
> > 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.

OK, but does it depend on anything?  Or does anything depend on it?

It is [12/13] in a series, so it looks like it doesn't depend on the
previous patches in it, but the next one may depend on it?  Which is the
case?

Thanks,
Rafael

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

[Dou Liyang]

Hi Rafael,

At 08/25/2017 12:38 AM, Rafael J. Wysocki wrote:
> On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
>> Hi Rafael, Zheng,
>>
>> At 07/31/2017 06:50 PM, Dou Liyang wrote:
>>> Hi,
>>>
>>> At 07/14/2017 01:52 PM, Dou Liyang wrote:
>>>> Linux uses acpi_early_init() to put the ACPI table management into
>>>> the late stage from the early stage where the mapped ACPI tables is
>>>> temporary and should be unmapped.
>>>>
>>>> But, now initializing interrupt delivery mode should map and parse the
>>>> DMAR table earlier in the early stage. This causes an ACPI error when
>>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
>>>> the DMAR table after using in the early stage.
>>>>
>>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
>>>> be mapped and parsed in late stage like before.
>>>>
>>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>> Cc: linux-acpi@vger.kernel.org
>>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>>>> Cc: Zheng, Lv <lv.zheng@intel.com>
>>>> Cc: Julian Wollrath <jwollrath@web.de>
>>>> ---
>>>> 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
>>> 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.
>
> OK, but does it depend on anything?  Or does anything depend on it?
>

It depends on nothing and can be considered independent.

[11/13] patch in this series depends on it. [11/13] patch caused an
ACPI error, we used this patch to fix it.

> It is [12/13] in a series, so it looks like it doesn't depend on the
> previous patches in it, but the next one may depend on it?  Which is the
> case?
>

The second case(the next one may depend on it) is what I want.

But, seems I made a mistake about the order of the patches. I should
put it before [11/13] to avoid the ACPI error.

I will adjust the order of the patches in the next version, and post
the whole series to you.

Thanks,
	dou.

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

[Rafael J. Wysocki]

On Friday, August 25, 2017 4:06:11 AM CEST Dou Liyang wrote:
> Hi Rafael,
> 
> At 08/25/2017 12:38 AM, Rafael J. Wysocki wrote:
> > On Thursday, August 24, 2017 5:54:28 AM CEST Dou Liyang wrote:
> >> Hi Rafael, Zheng,
> >>
> >> At 07/31/2017 06:50 PM, Dou Liyang wrote:
> >>> Hi,
> >>>
> >>> At 07/14/2017 01:52 PM, Dou Liyang wrote:
> >>>> Linux uses acpi_early_init() to put the ACPI table management into
> >>>> the late stage from the early stage where the mapped ACPI tables is
> >>>> temporary and should be unmapped.
> >>>>
> >>>> But, now initializing interrupt delivery mode should map and parse the
> >>>> DMAR table earlier in the early stage. This causes an ACPI error when
> >>>> Linux reallocates the ACPI root tables. Because Linux doesn't unmapped
> >>>> the DMAR table after using in the early stage.
> >>>>
> >>>> Invoke acpi_early_init() earlier before late_time_init(), Keep the DMAR
> >>>> be mapped and parsed in late stage like before.
> >>>>
> >>>> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >>>> Cc: linux-acpi@vger.kernel.org
> >>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> >>>> Cc: Zheng, Lv <lv.zheng@intel.com>
> >>>> Cc: Julian Wollrath <jwollrath@web.de>
> >>>> ---
> >>>> 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
> >>> 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.
> >
> > OK, but does it depend on anything?  Or does anything depend on it?
> >
> 
> It depends on nothing and can be considered independent.

OK

Please send it as an independent patch, then.

> [11/13] patch in this series depends on it. [11/13] patch caused an
> ACPI error, we used this patch to fix it.

So the ordering of patches in the series should be different, then.

It should be ordered so as to avoid triggering the warning at all,
so this patch should go before the [11/13].

> > It is [12/13] in a series, so it looks like it doesn't depend on the
> > previous patches in it, but the next one may depend on it?  Which is the
> > case?
> >
> 
> The second case(the next one may depend on it) is what I want.
> 
> But, seems I made a mistake about the order of the patches. I should
> put it before [11/13] to avoid the ACPI error.

Right.

> I will adjust the order of the patches in the next version, and post
> the whole series to you.

Please just CC it to linux-acpi.

Thanks,
Rafael

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

[Dou Liyang]

Hi Rafael,

At 08/25/2017 08:27 PM, Rafael J. Wysocki wrote:
> On Friday, August 25, 2017 4:06:11 AM CEST Dou Liyang wrote:
[...]
>>>>
>>>> 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.
>>>
>>> OK, but does it depend on anything?  Or does anything depend on it?
>>>
>>
>> It depends on nothing and can be considered independent.
>
> OK
>
> Please send it as an independent patch, then.
>
>> [11/13] patch in this series depends on it. [11/13] patch caused an
>> ACPI error, we used this patch to fix it.
>
> So the ordering of patches in the series should be different, then.
>
> It should be ordered so as to avoid triggering the warning at all,
> so this patch should go before the [11/13].

Yes, Indeed.

>
>>> It is [12/13] in a series, so it looks like it doesn't depend on the
>>> previous patches in it, but the next one may depend on it?  Which is the
>>> case?
>>>
>>
>> The second case(the next one may depend on it) is what I want.
>>
>> But, seems I made a mistake about the order of the patches. I should
>> put it before [11/13] to avoid the ACPI error.
>
> Right.
>
>> I will adjust the order of the patches in the next version, and post
>> the whole series to you.
>
> Please just CC it to linux-acpi.

Got it. Will just CC it to linux-acpi, and CC the whole series to you.

Thanks,
	dou.
>
> Thanks,
> Rafael
>
>
>
>