[PATCH v2 0/2] xen: re-enable booting as Xen PVH guest

[PATCH v2 0/2] xen: re-enable booting as Xen PVH guest

[Juergen Gross]

The Xen PVH boot protocol passes vital information to the kernel via
a start_info block. One of the data transferred is the physical address
of the RSDP table.

Unfortunately PVH support in the kernel didn't use that passed address
for RSDP, but relied on the legacy mechanism searching for the RSDP in
low memory. After a recent change in Xen putting the RSDP to a higher
address booting as PVH guest is now failing.

This small series repairs that by passing the RSDP address from the
start_info block to ACPI handling.

Juergen Gross (2):
  x86/acpi: add retrieval function for rsdp address
  xen: add acpi_arch_get_root_pointer() for pvh guests

 arch/x86/xen/enlighten_pvh.c | 14 +++++++++++---
 drivers/acpi/osl.c           | 10 +++++++++-
 include/linux/acpi.h         |  2 ++
 3 files changed, 22 insertions(+), 4 deletions(-)

-- 
2.13.6

[PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Juergen Gross]

Add a function to get the address of the RSDP table. Per default use a
__weak annotated function being a nop.

Cc: <stable@vger.kernel.org> # 4.11
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/acpi/osl.c   | 10 +++++++++-
 include/linux/acpi.h |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..2b77db914752 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -178,6 +178,11 @@ void acpi_os_vprintf(const char *fmt, va_list args)
 #endif
 }
 
+__weak acpi_physical_address acpi_arch_get_root_pointer(void)
+{
+	return 0;
+}
+
 #ifdef CONFIG_KEXEC
 static unsigned long acpi_rsdp;
 static int __init setup_acpi_rsdp(char *arg)
@@ -189,12 +194,15 @@ early_param("acpi_rsdp", setup_acpi_rsdp);
 
 acpi_physical_address __init acpi_os_get_root_pointer(void)
 {
-	acpi_physical_address pa = 0;
+	acpi_physical_address pa;
 
 #ifdef CONFIG_KEXEC
 	if (acpi_rsdp)
 		return acpi_rsdp;
 #endif
+	pa = acpi_arch_get_root_pointer();
+	if (pa)
+		return pa;
 
 	if (efi_enabled(EFI_CONFIG_TABLES)) {
 		if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..aa603cc5ad30 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1266,4 +1266,6 @@ static inline int lpit_read_residency_count_address(u64 *address)
 }
 #endif
 
+acpi_physical_address acpi_arch_get_root_pointer(void);
+
 #endif	/*_LINUX_ACPI_H*/
-- 
2.13.6

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Andy Shevchenko]

On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
> Add a function to get the address of the RSDP table. Per default use a
> __weak annotated function being a nop.

The problem with weak functions that we can't have more than one
implementation per kernel while we would like to built several code
paths.

I have stumbled on the similar stuff and realize that.

Perhaps, one of the solution is to have an additional struct under
x86_init to alternate ACPI related stuff.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Juergen Gross]

On 26/01/18 19:08, Andy Shevchenko wrote:
> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
>> Add a function to get the address of the RSDP table. Per default use a
>> __weak annotated function being a nop.
> 
> The problem with weak functions that we can't have more than one
> implementation per kernel while we would like to built several code
> paths.
> 
> I have stumbled on the similar stuff and realize that.
> 
> Perhaps, one of the solution is to have an additional struct under
> x86_init to alternate ACPI related stuff.

I think we can go that route when another user of that interface is
appearing.


Juergen

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Andy Shevchenko]

On Fri, Jan 26, 2018 at 8:21 PM, Juergen Gross <jgross@suse.com> wrote:
> On 26/01/18 19:08, Andy Shevchenko wrote:
>> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
>>> Add a function to get the address of the RSDP table. Per default use a
>>> __weak annotated function being a nop.
>>
>> The problem with weak functions that we can't have more than one
>> implementation per kernel while we would like to built several code
>> paths.
>>
>> I have stumbled on the similar stuff and realize that.
>>
>> Perhaps, one of the solution is to have an additional struct under
>> x86_init to alternate ACPI related stuff.
>
> I think we can go that route when another user of that interface is
> appearing.

Why not to establish the struct? At least this route I would like to
go with [1].

[1]: https://lkml.org/lkml/2018/1/17/834

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Rafael J. Wysocki]

On Sun, Jan 28, 2018 at 4:04 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Jan 26, 2018 at 8:21 PM, Juergen Gross <jgross@suse.com> wrote:
>> On 26/01/18 19:08, Andy Shevchenko wrote:
>>> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
>>>> Add a function to get the address of the RSDP table. Per default use a
>>>> __weak annotated function being a nop.
>>>
>>> The problem with weak functions that we can't have more than one
>>> implementation per kernel while we would like to built several code
>>> paths.
>>>
>>> I have stumbled on the similar stuff and realize that.
>>>
>>> Perhaps, one of the solution is to have an additional struct under
>>> x86_init to alternate ACPI related stuff.
>>
>> I think we can go that route when another user of that interface is
>> appearing.
>
> Why not to establish the struct? At least this route I would like to
> go with [1].
>
> [1]: https://lkml.org/lkml/2018/1/17/834

Maybe I'm a bit slow today, but care to explain what exactly you mean?

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Andy Shevchenko]

On Mon, Jan 29, 2018 at 5:02 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Sun, Jan 28, 2018 at 4:04 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Fri, Jan 26, 2018 at 8:21 PM, Juergen Gross <jgross@suse.com> wrote:
>>> On 26/01/18 19:08, Andy Shevchenko wrote:
>>>> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:

>>>> The problem with weak functions that we can't have more than one
>>>> implementation per kernel while we would like to built several code
>>>> paths.
>>>>
>>>> I have stumbled on the similar stuff and realize that.
>>>>
>>>> Perhaps, one of the solution is to have an additional struct under
>>>> x86_init to alternate ACPI related stuff.
>>>
>>> I think we can go that route when another user of that interface is
>>> appearing.
>>
>> Why not to establish the struct? At least this route I would like to
>> go with [1].
>>
>> [1]: https://lkml.org/lkml/2018/1/17/834
>
> Maybe I'm a bit slow today, but care to explain what exactly you mean?

Instead of declaring function as __weak, establish a new struct for
ACPI related stubs and incorporate it into x86_init.

That is my proposal. I think I would go this way in my case where I
need to treat differently ACPI HW reduced initialization of legacy
devices.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Rafael J. Wysocki]

On Wed, Jan 31, 2018 at 4:43 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Jan 29, 2018 at 5:02 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Sun, Jan 28, 2018 at 4:04 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Fri, Jan 26, 2018 at 8:21 PM, Juergen Gross <jgross@suse.com> wrote:
>>>> On 26/01/18 19:08, Andy Shevchenko wrote:
>>>>> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
>
>>>>> The problem with weak functions that we can't have more than one
>>>>> implementation per kernel while we would like to built several code
>>>>> paths.
>>>>>
>>>>> I have stumbled on the similar stuff and realize that.
>>>>>
>>>>> Perhaps, one of the solution is to have an additional struct under
>>>>> x86_init to alternate ACPI related stuff.
>>>>
>>>> I think we can go that route when another user of that interface is
>>>> appearing.
>>>
>>> Why not to establish the struct? At least this route I would like to
>>> go with [1].
>>>
>>> [1]: https://lkml.org/lkml/2018/1/17/834
>>
>> Maybe I'm a bit slow today, but care to explain what exactly you mean?
>
> Instead of declaring function as __weak, establish a new struct for
> ACPI related stubs and incorporate it into x86_init.
>
> That is my proposal. I think I would go this way in my case where I
> need to treat differently ACPI HW reduced initialization of legacy
> devices.

IOW you'd like to have a set of ACPI init callbacks that could be
defined by an arch, right?

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Andy Shevchenko]

On Thu, Feb 1, 2018 at 9:57 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Jan 31, 2018 at 4:43 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Jan 29, 2018 at 5:02 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>> On Sun, Jan 28, 2018 at 4:04 PM, Andy Shevchenko
>>> <andy.shevchenko@gmail.com> wrote:

>> Instead of declaring function as __weak, establish a new struct for
>> ACPI related stubs and incorporate it into x86_init.
>>
>> That is my proposal. I think I would go this way in my case where I
>> need to treat differently ACPI HW reduced initialization of legacy
>> devices.
>
> IOW you'd like to have a set of ACPI init callbacks that could be
> defined by an arch, right?

Correct!

And since there is another potential user (Xen) for this approach I
consider it a good chance to be chosen.
Though I have no idea if Xen can do things differently.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Rafael J. Wysocki]

On Thu, Feb 1, 2018 at 4:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Feb 1, 2018 at 9:57 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Wed, Jan 31, 2018 at 4:43 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Mon, Jan 29, 2018 at 5:02 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>> On Sun, Jan 28, 2018 at 4:04 PM, Andy Shevchenko
>>>> <andy.shevchenko@gmail.com> wrote:
>
>>> Instead of declaring function as __weak, establish a new struct for
>>> ACPI related stubs and incorporate it into x86_init.
>>>
>>> That is my proposal. I think I would go this way in my case where I
>>> need to treat differently ACPI HW reduced initialization of legacy
>>> devices.
>>
>> IOW you'd like to have a set of ACPI init callbacks that could be
>> defined by an arch, right?
>
> Correct!

OK

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Rafael J. Wysocki]

On Fri, Jan 26, 2018 at 7:08 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jan 25, 2018 at 4:36 PM, Juergen Gross <jgross@suse.com> wrote:
>> Add a function to get the address of the RSDP table. Per default use a
>> __weak annotated function being a nop.
>
> The problem with weak functions that we can't have more than one
> implementation per kernel while we would like to built several code
> paths.
>
> I have stumbled on the similar stuff and realize that.
>
> Perhaps, one of the solution is to have an additional struct under
> x86_init to alternate ACPI related stuff.

I'm not sure if that really is a problem in this particular case.

Why would you want to use different RSDP retrieval functions for one arch?

Thanks,
Rafael

Re: [PATCH v2 1/2] x86/acpi: add retrieval function for rsdp address

[Andy Shevchenko]

On Mon, Jan 29, 2018 at 5:01 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Jan 26, 2018 at 7:08 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

>> I have stumbled on the similar stuff and realize that.
>>
>> Perhaps, one of the solution is to have an additional struct under
>> x86_init to alternate ACPI related stuff.
>
> I'm not sure if that really is a problem in this particular case.
>
> Why would you want to use different RSDP retrieval functions for one arch?

I was not clear. I'm talking about approach struct x86_init vs. __weak function.
In my case it has nothing to do with RDSP, but with ACPI stubs.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] xen: add acpi_arch_get_root_pointer() for pvh guests

[Juergen Gross]

Add acpi_arch_get_root_pointer() for Xen PVH guests to communicate
the address of the RSDP table given to the kernel via Xen start info.

This makes the kernel boot again in PVH mode after on recent Xen the
RSDP was moved to higher addresses. So up to that change it was pure
luck that the legacy method to locate the RSDP was working when
running as PVH mode.

Cc: <stable@vger.kernel.org> # 4.11
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/enlighten_pvh.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index 436c4f003e17..f08fd43f2aa2 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -16,15 +16,23 @@
 /*
  * PVH variables.
  *
- * xen_pvh and pvh_bootparams need to live in data segment since they
- * are used after startup_{32|64}, which clear .bss, are invoked.
+ * xen_pvh, pvh_bootparams and pvh_start_info need to live in data segment
+ * since they are used after startup_{32|64}, which clear .bss, are invoked.
  */
 bool xen_pvh __attribute__((section(".data"))) = 0;
 struct boot_params pvh_bootparams __attribute__((section(".data")));
+struct hvm_start_info pvh_start_info __attribute__((section(".data")));
 
-struct hvm_start_info pvh_start_info;
 unsigned int pvh_start_info_sz = sizeof(pvh_start_info);
 
+acpi_physical_address acpi_arch_get_root_pointer(void)
+{
+	if (xen_pvh)
+		return pvh_start_info.rsdp_paddr;
+
+	return 0;
+}
+
 static void __init init_pvh_bootparams(void)
 {
 	struct xen_memory_map memmap;
-- 
2.13.6

Re: [PATCH v2 2/2] xen: add acpi_arch_get_root_pointer() for pvh guests

[Boris Ostrovsky]

On 01/25/2018 09:36 AM, Juergen Gross wrote:
> Add acpi_arch_get_root_pointer() for Xen PVH guests to communicate
> the address of the RSDP table given to the kernel via Xen start info.
> 
> This makes the kernel boot again in PVH mode after on recent Xen the
> RSDP was moved to higher addresses. So up to that change it was pure
> luck that the legacy method to locate the RSDP was working when
> running as PVH mode.
> 
> Cc: <stable@vger.kernel.org> # 4.11
> Signed-off-by: Juergen Gross <jgross@suse.com>


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>