[PATCH v2 0/3] x86: make rsdp address accessible via boot params

[PATCH v2 0/3] x86: make rsdp address accessible via boot params

[Juergen Gross]

In the non-EFI boot path the ACPI RSDP table is currently found via
either EBDA or by searching through low memory for the RSDP magic.
This requires the RSDP to be located in the first 1MB of physical
memory. Xen PVH guests, however, get the RSDP address via the start of
day information block.

In order to support an arbitrary RSDP address this patch series adds
the physical address of the RSDP to the boot params structure filled
by the boot loader. A kernel booted directly in PVH mode can save the
RSDP address in the boot params, while a kernel booted in PVH mode via
grub can rely on the RSDP address being specified by grub2 (which in
turn got the address via the start of day information block from Xen).

Juergen Gross (3):
  x86/boot: add acpi rsdp address to setup_header
  x86/acpi: take rsdp address for boot params if available
  x86/xen: supply rsdp address in boot params for pvh guests

 Documentation/x86/boot.txt            | 19 +++++++++++++++++++
 arch/x86/boot/header.S                |  6 +++++-
 arch/x86/include/uapi/asm/bootparam.h |  1 +
 arch/x86/xen/enlighten_pvh.c          |  5 ++++-
 drivers/acpi/osl.c                    |  8 ++++++++
 5 files changed, 37 insertions(+), 2 deletions(-)

-- 
2.12.3

[PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

Xen PVH guests receive the address of the RSDP table from Xen. In order
to support booting a Xen PVH guest via grub2 using the standard x86
boot entry we need a way fro grub2 to pass the RSDP address to the
kernel.

For this purpose expand the struct setup_header to hold the physical
address of the RSDP address. Being zero means it isn't specified and
has to be located the legacy way (searching through low memory or
EBDA).

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
 Documentation/x86/boot.txt            | 19 +++++++++++++++++++
 arch/x86/boot/header.S                |  6 +++++-
 arch/x86/include/uapi/asm/bootparam.h |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 5e9b826b5f62..a33c224797e4 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
 	 	to struct boot_params for loading bzImage and ramdisk
 		above 4G in 64bit.
 
+Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
+		xloadflags to support booting a 64 bit kernel from 32 bit
+		EFI
+
+Protocol 2.14	(Kernel 4.16) Added acpi_rsdp_addr holding the physical
+		address of the ACPI RSDP table.
+
 **** MEMORY LAYOUT
 
 The traditional memory map for the kernel loader, used for Image or
@@ -197,6 +204,7 @@ Offset	Proto	Name		Meaning
 0258/8	2.10+	pref_address	Preferred loading address
 0260/4	2.10+	init_size	Linear memory required during initialization
 0264/4	2.11+	handover_offset	Offset of handover entry point
+0268/8	2.14+	acpi_rsdp_addr	Physical address of RSDP table
 
 (1) For backwards compatibility, if the setup_sects field contains 0, the
     real value is 4.
@@ -744,6 +752,17 @@ Offset/size:	0x264/4
 
   See EFI HANDOVER PROTOCOL below for more details.
 
+Field name:	acpi_rsdp_addr
+Type:		write
+Offset/size:	0x268/8
+Protocol:	2.14+
+
+  This field can be set by the boot loader to tell the kernel the
+  physical address of the ACPI RSDP table.
+
+  A value of 0 indicates the kernel should fall back to the standard
+  methods to locate the RSDP (search in EBDA/low memory).
+
 
 **** THE IMAGE CHECKSUM
 
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 850b8762e889..e7184127f309 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x020d		# header version number (>= 0x0105)
+		.word	0x020e		# header version number (>= 0x0105)
 					# or else old loadlin-1.5 will fail)
 		.globl realmode_swtch
 realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
@@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 init_size:		.long INIT_SIZE		# kernel initialization size
 handover_offset:	.long 0			# Filled in by build.c
 
+acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
+						# ACPI RSDP table, added with
+						# version 2.14
+
 # End of setup header #####################################################
 
 	.section ".entrytext", "ax"
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index afdd5ae0fcc4..5742e433e93e 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -85,6 +85,7 @@ struct setup_header {
 	__u64	pref_address;
 	__u32	init_size;
 	__u32	handover_offset;
+	__u64	acpi_rsdp_addr;
 } __attribute__((packed));
 
 struct sys_desc_table {
-- 
2.12.3

[PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

Xen PVH guests receive the address of the RSDP table from Xen. In order
to support booting a Xen PVH guest via grub2 using the standard x86
boot entry we need a way fro grub2 to pass the RSDP address to the
kernel.

For this purpose expand the struct setup_header to hold the physical
address of the RSDP address. Being zero means it isn't specified and
has to be located the legacy way (searching through low memory or
EBDA).

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
 Documentation/x86/boot.txt            | 19 +++++++++++++++++++
 arch/x86/boot/header.S                |  6 +++++-
 arch/x86/include/uapi/asm/bootparam.h |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 5e9b826b5f62..a33c224797e4 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
 	 	to struct boot_params for loading bzImage and ramdisk
 		above 4G in 64bit.
 
+Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
+		xloadflags to support booting a 64 bit kernel from 32 bit
+		EFI
+
+Protocol 2.14	(Kernel 4.16) Added acpi_rsdp_addr holding the physical
+		address of the ACPI RSDP table.
+
 **** MEMORY LAYOUT
 
 The traditional memory map for the kernel loader, used for Image or
@@ -197,6 +204,7 @@ Offset	Proto	Name		Meaning
 0258/8	2.10+	pref_address	Preferred loading address
 0260/4	2.10+	init_size	Linear memory required during initialization
 0264/4	2.11+	handover_offset	Offset of handover entry point
+0268/8	2.14+	acpi_rsdp_addr	Physical address of RSDP table
 
 (1) For backwards compatibility, if the setup_sects field contains 0, the
     real value is 4.
@@ -744,6 +752,17 @@ Offset/size:	0x264/4
 
   See EFI HANDOVER PROTOCOL below for more details.
 
+Field name:	acpi_rsdp_addr
+Type:		write
+Offset/size:	0x268/8
+Protocol:	2.14+
+
+  This field can be set by the boot loader to tell the kernel the
+  physical address of the ACPI RSDP table.
+
+  A value of 0 indicates the kernel should fall back to the standard
+  methods to locate the RSDP (search in EBDA/low memory).
+
 
 **** THE IMAGE CHECKSUM
 
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 850b8762e889..e7184127f309 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x020d		# header version number (>= 0x0105)
+		.word	0x020e		# header version number (>= 0x0105)
 					# or else old loadlin-1.5 will fail)
 		.globl realmode_swtch
 realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
@@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 init_size:		.long INIT_SIZE		# kernel initialization size
 handover_offset:	.long 0			# Filled in by build.c
 
+acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
+						# ACPI RSDP table, added with
+						# version 2.14
+
 # End of setup header #####################################################
 
 	.section ".entrytext", "ax"
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index afdd5ae0fcc4..5742e433e93e 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -85,6 +85,7 @@ struct setup_header {
 	__u64	pref_address;
 	__u32	init_size;
 	__u32	handover_offset;
+	__u64	acpi_rsdp_addr;
 } __attribute__((packed));
 
 struct sys_desc_table {
-- 
2.12.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

In case the rsdp address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/acpi/osl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..3b25e2ad7d75 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,10 @@
 #include <linux/uaccess.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 
+#ifdef CONFIG_X86
+#include <asm/setup.h>
+#endif
+
 #include "internal.h"
 
 #define _COMPONENT		ACPI_OS_SERVICES
@@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 	if (acpi_rsdp)
 		return acpi_rsdp;
 #endif
+#ifdef CONFIG_X86
+	if (boot_params.hdr.acpi_rsdp_addr)
+		return boot_params.hdr.acpi_rsdp_addr;
+#endif
 
 	if (efi_enabled(EFI_CONFIG_TABLES)) {
 		if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
-- 
2.12.3

[PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

In case the rsdp address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/acpi/osl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..3b25e2ad7d75 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,10 @@
 #include <linux/uaccess.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 
+#ifdef CONFIG_X86
+#include <asm/setup.h>
+#endif
+
 #include "internal.h"
 
 #define _COMPONENT		ACPI_OS_SERVICES
@@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 	if (acpi_rsdp)
 		return acpi_rsdp;
 #endif
+#ifdef CONFIG_X86
+	if (boot_params.hdr.acpi_rsdp_addr)
+		return boot_params.hdr.acpi_rsdp_addr;
+#endif
 
 	if (efi_enabled(EFI_CONFIG_TABLES)) {
 		if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
-- 
2.12.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Juergen Gross]

When booted via the special PVH entry save the RSDP address set in the
boot information block in struct boot_params. This will enable Xen to
locate the RSDP at an arbitrary address.

Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
which should have been 0x020c.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2: set bootloader version to 2.14 (Roger Pau Monné)
---
 arch/x86/xen/enlighten_pvh.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index 436c4f003e17..036e3a5f284a 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
 	 *
 	 * Version 2.12 supports Xen entry point but we will use default x86/PC
 	 * environment (i.e. hardware_subarch 0).
+	 * The RSDP address is available from version 2.14 on.
 	 */
-	pvh_bootparams.hdr.version = 0x212;
+	pvh_bootparams.hdr.version = 0x20e;
 	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
+
+	pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr;
 }
 
 /*
-- 
2.12.3

[PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Juergen Gross]

When booted via the special PVH entry save the RSDP address set in the
boot information block in struct boot_params. This will enable Xen to
locate the RSDP at an arbitrary address.

Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
which should have been 0x020c.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2: set bootloader version to 2.14 (Roger Pau Monné)
---
 arch/x86/xen/enlighten_pvh.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index 436c4f003e17..036e3a5f284a 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
 	 *
 	 * Version 2.12 supports Xen entry point but we will use default x86/PC
 	 * environment (i.e. hardware_subarch 0).
+	 * The RSDP address is available from version 2.14 on.
 	 */
-	pvh_bootparams.hdr.version = 0x212;
+	pvh_bootparams.hdr.version = 0x20e;
 	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
+
+	pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr;
 }
 
 /*
-- 
2.12.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> In case the rsdp address in struct boot_params is specified don't try
> to find the table by searching, but take the address directly as set
> by the boot loader.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  drivers/acpi/osl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb24a99..3b25e2ad7d75 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -45,6 +45,10 @@
>  #include <linux/uaccess.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/setup.h>
> +#endif
> +
>  #include "internal.h"
>  
>  #define _COMPONENT		ACPI_OS_SERVICES
> @@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>  	if (acpi_rsdp)
>  		return acpi_rsdp;
>  #endif
> +#ifdef CONFIG_X86
> +	if (boot_params.hdr.acpi_rsdp_addr)
> +		return boot_params.hdr.acpi_rsdp_addr;
> +#endif

Argh, that's typical short sighted hackery, layering violations and general 
eyesore combined into a single patch ...

Those #ifdefs are a disgrace, plus why should generic ACPI code include platform 
details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to 
non-x86 - so someone will have to redo this work for ARM64 as well in the future 
...

So how about doing it right:

1)

Add a __weak acpi_arch_get_root_pointer() __weak function to drivers/acpi/osl.c:


__weak acpi_physical_address acpi_arch_get_root_pointer(void)
{
	return 0;
}

2)

use it in acpi_os_get_root_pointer():

	...
	pa = acpi_arch_get_root_pointer();
	if (pa)
		return pa;
	...

3)

Override the default variant in x86's acpi.c via something like:

acpi_physical_address acpi_arch_get_root_pointer(void)
{
	return boot_params.hdr.acpi_rsdp_addr;
}

4)

Add this to arch/x86/include/asm/acpi.h:

extern acpi_physical_address acpi_arch_get_root_pointer(void);

5)

Add #include <asm/acpi.h> to drivers/acpi/osl.c.


That looks much cleaner, has no layering violations and is infinitely more 
extensible, right?

Thanks,

	Ingo

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> In case the rsdp address in struct boot_params is specified don't try
> to find the table by searching, but take the address directly as set
> by the boot loader.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  drivers/acpi/osl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb24a99..3b25e2ad7d75 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -45,6 +45,10 @@
>  #include <linux/uaccess.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/setup.h>
> +#endif
> +
>  #include "internal.h"
>  
>  #define _COMPONENT		ACPI_OS_SERVICES
> @@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>  	if (acpi_rsdp)
>  		return acpi_rsdp;
>  #endif
> +#ifdef CONFIG_X86
> +	if (boot_params.hdr.acpi_rsdp_addr)
> +		return boot_params.hdr.acpi_rsdp_addr;
> +#endif

Argh, that's typical short sighted hackery, layering violations and general 
eyesore combined into a single patch ...

Those #ifdefs are a disgrace, plus why should generic ACPI code include platform 
details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to 
non-x86 - so someone will have to redo this work for ARM64 as well in the future 
...

So how about doing it right:

1)

Add a __weak acpi_arch_get_root_pointer() __weak function to drivers/acpi/osl.c:


__weak acpi_physical_address acpi_arch_get_root_pointer(void)
{
	return 0;
}

2)

use it in acpi_os_get_root_pointer():

	...
	pa = acpi_arch_get_root_pointer();
	if (pa)
		return pa;
	...

3)

Override the default variant in x86's acpi.c via something like:

acpi_physical_address acpi_arch_get_root_pointer(void)
{
	return boot_params.hdr.acpi_rsdp_addr;
}

4)

Add this to arch/x86/include/asm/acpi.h:

extern acpi_physical_address acpi_arch_get_root_pointer(void);

5)

Add #include <asm/acpi.h> to drivers/acpi/osl.c.


That looks much cleaner, has no layering violations and is infinitely more 
extensible, right?

Thanks,

	Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Xen PVH guests receive the address of the RSDP table from Xen. In order
> to support booting a Xen PVH guest via grub2 using the standard x86
> boot entry we need a way fro grub2 to pass the RSDP address to the
> kernel.
> 
> For this purpose expand the struct setup_header to hold the physical
> address of the RSDP address. Being zero means it isn't specified and
> has to be located the legacy way (searching through low memory or
> EBDA).

s/fro
 /for

pedantry:

s/grub2
 /Grub2

> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  Documentation/x86/boot.txt            | 19 +++++++++++++++++++
>  arch/x86/boot/header.S                |  6 +++++-
>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
> index 5e9b826b5f62..a33c224797e4 100644
> --- a/Documentation/x86/boot.txt
> +++ b/Documentation/x86/boot.txt
> @@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
>  	 	to struct boot_params for loading bzImage and ramdisk
>  		above 4G in 64bit.
>  
> +Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
> +		xloadflags to support booting a 64 bit kernel from 32 bit
> +		EFI

The changelog should I think declare that we add documentation for the 2.13 
protocol iteration as well.

Also, please use a consistent spelling of '32-bit' and '64-bit' in the same 
sentence!

> +Field name:	acpi_rsdp_addr
> +Type:		write
> +Offset/size:	0x268/8
> +Protocol:	2.14+
> +
> +  This field can be set by the boot loader to tell the kernel the
> +  physical address of the ACPI RSDP table.
> +
> +  A value of 0 indicates the kernel should fall back to the standard
> +  methods to locate the RSDP (search in EBDA/low memory).

That's not the only method used: the ACPI RSDP address can also be discovered via 
efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.

>  **** THE IMAGE CHECKSUM
>  
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index 850b8762e889..e7184127f309 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -300,7 +300,7 @@ _start:
>  	# Part 2 of the header, from the old setup.S
>  
>  		.ascii	"HdrS"		# header signature
> -		.word	0x020d		# header version number (>= 0x0105)
> +		.word	0x020e		# header version number (>= 0x0105)
>  					# or else old loadlin-1.5 will fail)
>  		.globl realmode_swtch
>  realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
> @@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
>  init_size:		.long INIT_SIZE		# kernel initialization size
>  handover_offset:	.long 0			# Filled in by build.c
>  
> +acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
> +						# ACPI RSDP table, added with
> +						# version 2.14

s/pointer to ACPI RSDP table
 /pointer to the ACPI RSDP table

Also, a more fundamental question: why doesn't Xen use EFI to hand over hardware 
configuration details?

Thanks,

	Ingo

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Xen PVH guests receive the address of the RSDP table from Xen. In order
> to support booting a Xen PVH guest via grub2 using the standard x86
> boot entry we need a way fro grub2 to pass the RSDP address to the
> kernel.
> 
> For this purpose expand the struct setup_header to hold the physical
> address of the RSDP address. Being zero means it isn't specified and
> has to be located the legacy way (searching through low memory or
> EBDA).

s/fro
 /for

pedantry:

s/grub2
 /Grub2

> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  Documentation/x86/boot.txt            | 19 +++++++++++++++++++
>  arch/x86/boot/header.S                |  6 +++++-
>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
> index 5e9b826b5f62..a33c224797e4 100644
> --- a/Documentation/x86/boot.txt
> +++ b/Documentation/x86/boot.txt
> @@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
>  	 	to struct boot_params for loading bzImage and ramdisk
>  		above 4G in 64bit.
>  
> +Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
> +		xloadflags to support booting a 64 bit kernel from 32 bit
> +		EFI

The changelog should I think declare that we add documentation for the 2.13 
protocol iteration as well.

Also, please use a consistent spelling of '32-bit' and '64-bit' in the same 
sentence!

> +Field name:	acpi_rsdp_addr
> +Type:		write
> +Offset/size:	0x268/8
> +Protocol:	2.14+
> +
> +  This field can be set by the boot loader to tell the kernel the
> +  physical address of the ACPI RSDP table.
> +
> +  A value of 0 indicates the kernel should fall back to the standard
> +  methods to locate the RSDP (search in EBDA/low memory).

That's not the only method used: the ACPI RSDP address can also be discovered via 
efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.

>  **** THE IMAGE CHECKSUM
>  
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index 850b8762e889..e7184127f309 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -300,7 +300,7 @@ _start:
>  	# Part 2 of the header, from the old setup.S
>  
>  		.ascii	"HdrS"		# header signature
> -		.word	0x020d		# header version number (>= 0x0105)
> +		.word	0x020e		# header version number (>= 0x0105)
>  					# or else old loadlin-1.5 will fail)
>  		.globl realmode_swtch
>  realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
> @@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
>  init_size:		.long INIT_SIZE		# kernel initialization size
>  handover_offset:	.long 0			# Filled in by build.c
>  
> +acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
> +						# ACPI RSDP table, added with
> +						# version 2.14

s/pointer to ACPI RSDP table
 /pointer to the ACPI RSDP table

Also, a more fundamental question: why doesn't Xen use EFI to hand over hardware 
configuration details?

Thanks,

	Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> When booted via the special PVH entry save the RSDP address set in the
> boot information block in struct boot_params. This will enable Xen to
> locate the RSDP at an arbitrary address.
> 
> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
> which should have been 0x020c.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2: set bootloader version to 2.14 (Roger Pau Monné)
> ---
>  arch/x86/xen/enlighten_pvh.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
> index 436c4f003e17..036e3a5f284a 100644
> --- a/arch/x86/xen/enlighten_pvh.c
> +++ b/arch/x86/xen/enlighten_pvh.c
> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
>  	 *
>  	 * Version 2.12 supports Xen entry point but we will use default x86/PC
>  	 * environment (i.e. hardware_subarch 0).
> +	 * The RSDP address is available from version 2.14 on.
>  	 */
> -	pvh_bootparams.hdr.version = 0x212;
> +	pvh_bootparams.hdr.version = 0x20e;

While 0x212 was "obvious" to read but totally wrong, it would be less fragile and 
more readable if the version was generated as something like:

	pvh_bootparams.hdr.version = (2 << 8) | 14;

similar to how it's written in other cases:

>  	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */

Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug appears to 
have been introduced at around v4.12.

Thanks,

	Ingo

Re: [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> When booted via the special PVH entry save the RSDP address set in the
> boot information block in struct boot_params. This will enable Xen to
> locate the RSDP at an arbitrary address.
> 
> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
> which should have been 0x020c.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2: set bootloader version to 2.14 (Roger Pau Monné)
> ---
>  arch/x86/xen/enlighten_pvh.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
> index 436c4f003e17..036e3a5f284a 100644
> --- a/arch/x86/xen/enlighten_pvh.c
> +++ b/arch/x86/xen/enlighten_pvh.c
> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
>  	 *
>  	 * Version 2.12 supports Xen entry point but we will use default x86/PC
>  	 * environment (i.e. hardware_subarch 0).
> +	 * The RSDP address is available from version 2.14 on.
>  	 */
> -	pvh_bootparams.hdr.version = 0x212;
> +	pvh_bootparams.hdr.version = 0x20e;

While 0x212 was "obvious" to read but totally wrong, it would be less fragile and 
more readable if the version was generated as something like:

	pvh_bootparams.hdr.version = (2 << 8) | 14;

similar to how it's written in other cases:

>  	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */

Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug appears to 
have been introduced at around v4.12.

Thanks,

	Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 08:05, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> In case the rsdp address in struct boot_params is specified don't try
>> to find the table by searching, but take the address directly as set
>> by the boot loader.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  drivers/acpi/osl.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 3bb46cb24a99..3b25e2ad7d75 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -45,6 +45,10 @@
>>  #include <linux/uaccess.h>
>>  #include <linux/io-64-nonatomic-lo-hi.h>
>>  
>> +#ifdef CONFIG_X86
>> +#include <asm/setup.h>
>> +#endif
>> +
>>  #include "internal.h"
>>  
>>  #define _COMPONENT		ACPI_OS_SERVICES
>> @@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  	if (acpi_rsdp)
>>  		return acpi_rsdp;
>>  #endif
>> +#ifdef CONFIG_X86
>> +	if (boot_params.hdr.acpi_rsdp_addr)
>> +		return boot_params.hdr.acpi_rsdp_addr;
>> +#endif
> 
> Argh, that's typical short sighted hackery, layering violations and general 
> eyesore combined into a single patch ...
> 
> Those #ifdefs are a disgrace, plus why should generic ACPI code include platform 
> details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to 
> non-x86 - so someone will have to redo this work for ARM64 as well in the future 
> ...
> 
> So how about doing it right:
> 
> 1)
> 
> Add a __weak acpi_arch_get_root_pointer() __weak function to drivers/acpi/osl.c:
> 
> 
> __weak acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return 0;
> }
> 
> 2)
> 
> use it in acpi_os_get_root_pointer():
> 
> 	...
> 	pa = acpi_arch_get_root_pointer();
> 	if (pa)
> 		return pa;
> 	...
> 
> 3)
> 
> Override the default variant in x86's acpi.c via something like:
> 
> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return boot_params.hdr.acpi_rsdp_addr;
> }
> 
> 4)
> 
> Add this to arch/x86/include/asm/acpi.h:
> 
> extern acpi_physical_address acpi_arch_get_root_pointer(void);
> 
> 5)
> 
> Add #include <asm/acpi.h> to drivers/acpi/osl.c.
> 
> 
> That looks much cleaner, has no layering violations and is infinitely more 
> extensible, right?

Right.

Thanks for the very constructive comment.


Juergen

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 08:05, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> In case the rsdp address in struct boot_params is specified don't try
>> to find the table by searching, but take the address directly as set
>> by the boot loader.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  drivers/acpi/osl.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 3bb46cb24a99..3b25e2ad7d75 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -45,6 +45,10 @@
>>  #include <linux/uaccess.h>
>>  #include <linux/io-64-nonatomic-lo-hi.h>
>>  
>> +#ifdef CONFIG_X86
>> +#include <asm/setup.h>
>> +#endif
>> +
>>  #include "internal.h"
>>  
>>  #define _COMPONENT		ACPI_OS_SERVICES
>> @@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  	if (acpi_rsdp)
>>  		return acpi_rsdp;
>>  #endif
>> +#ifdef CONFIG_X86
>> +	if (boot_params.hdr.acpi_rsdp_addr)
>> +		return boot_params.hdr.acpi_rsdp_addr;
>> +#endif
> 
> Argh, that's typical short sighted hackery, layering violations and general 
> eyesore combined into a single patch ...
> 
> Those #ifdefs are a disgrace, plus why should generic ACPI code include platform 
> details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to 
> non-x86 - so someone will have to redo this work for ARM64 as well in the future 
> ...
> 
> So how about doing it right:
> 
> 1)
> 
> Add a __weak acpi_arch_get_root_pointer() __weak function to drivers/acpi/osl.c:
> 
> 
> __weak acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return 0;
> }
> 
> 2)
> 
> use it in acpi_os_get_root_pointer():
> 
> 	...
> 	pa = acpi_arch_get_root_pointer();
> 	if (pa)
> 		return pa;
> 	...
> 
> 3)
> 
> Override the default variant in x86's acpi.c via something like:
> 
> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return boot_params.hdr.acpi_rsdp_addr;
> }
> 
> 4)
> 
> Add this to arch/x86/include/asm/acpi.h:
> 
> extern acpi_physical_address acpi_arch_get_root_pointer(void);
> 
> 5)
> 
> Add #include <asm/acpi.h> to drivers/acpi/osl.c.
> 
> 
> That looks much cleaner, has no layering violations and is infinitely more 
> extensible, right?

Right.

Thanks for the very constructive comment.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

>>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> hardware configuration details?

Iirc the main purpose of the change here is to allow booting PVH
(guest or Dom0) with Grub2 in the middle. PVH, at least for the
time being, is something that gets away without any firmware
(and I'm pretty certain this is going to remain that way for Dom0).
ACPI tables are being built by the tool stack (guest) or hypervisor
(Dom0). Hence there simply isn't any EFI which could be used to
propagate such information.

Jan

Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Jan Beulich]

>>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> hardware configuration details?

Iirc the main purpose of the change here is to allow booting PVH
(guest or Dom0) with Grub2 in the middle. PVH, at least for the
time being, is something that gets away without any firmware
(and I'm pretty certain this is going to remain that way for Dom0).
ACPI tables are being built by the tool stack (guest) or hypervisor
(Dom0). Hence there simply isn't any EFI which could be used to
propagate such information.

Jan

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Jan Beulich]

>>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> hardware configuration details?

Iirc the main purpose of the change here is to allow booting PVH
(guest or Dom0) with Grub2 in the middle. PVH, at least for the
time being, is something that gets away without any firmware
(and I'm pretty certain this is going to remain that way for Dom0).
ACPI tables are being built by the tool stack (guest) or hypervisor
(Dom0). Hence there simply isn't any EFI which could be used to
propagate such information.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> > Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> > hardware configuration details?
> 
> Iirc the main purpose of the change here is to allow booting PVH
> (guest or Dom0) with Grub2 in the middle. PVH, at least for the
> time being, is something that gets away without any firmware
> (and I'm pretty certain this is going to remain that way for Dom0).
> ACPI tables are being built by the tool stack (guest) or hypervisor
> (Dom0). Hence there simply isn't any EFI which could be used to
> propagate such information.

Ok, that's fair enough. If hpa (or someone else) doesn't object to the boot 
protocol extension this approach looks good to me in principle.

Thanks,

	Ingo

Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> > Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> > hardware configuration details?
> 
> Iirc the main purpose of the change here is to allow booting PVH
> (guest or Dom0) with Grub2 in the middle. PVH, at least for the
> time being, is something that gets away without any firmware
> (and I'm pretty certain this is going to remain that way for Dom0).
> ACPI tables are being built by the tool stack (guest) or hypervisor
> (Dom0). Hence there simply isn't any EFI which could be used to
> propagate such information.

Ok, that's fair enough. If hpa (or someone else) doesn't object to the boot 
protocol extension this approach looks good to me in principle.

Thanks,

	Ingo

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 08.12.17 at 08:16, <mingo@kernel.org> wrote:
> > Also, a more fundamental question: why doesn't Xen use EFI to hand over 
> > hardware configuration details?
> 
> Iirc the main purpose of the change here is to allow booting PVH
> (guest or Dom0) with Grub2 in the middle. PVH, at least for the
> time being, is something that gets away without any firmware
> (and I'm pretty certain this is going to remain that way for Dom0).
> ACPI tables are being built by the tool stack (guest) or hypervisor
> (Dom0). Hence there simply isn't any EFI which could be used to
> propagate such information.

Ok, that's fair enough. If hpa (or someone else) doesn't object to the boot 
protocol extension this approach looks good to me in principle.

Thanks,

	Ingo


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

On 08/12/17 08:16, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> Xen PVH guests receive the address of the RSDP table from Xen. In order
>> to support booting a Xen PVH guest via grub2 using the standard x86
>> boot entry we need a way fro grub2 to pass the RSDP address to the
>> kernel.
>>
>> For this purpose expand the struct setup_header to hold the physical
>> address of the RSDP address. Being zero means it isn't specified and
>> has to be located the legacy way (searching through low memory or
>> EBDA).
> 
> s/fro
>  /for
> 
> pedantry:
> 
> s/grub2
>  /Grub2

Okay.

> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>>  Documentation/x86/boot.txt            | 19 +++++++++++++++++++
>>  arch/x86/boot/header.S                |  6 +++++-
>>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>>  3 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
>> index 5e9b826b5f62..a33c224797e4 100644
>> --- a/Documentation/x86/boot.txt
>> +++ b/Documentation/x86/boot.txt
>> @@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
>>  	 	to struct boot_params for loading bzImage and ramdisk
>>  		above 4G in 64bit.
>>  
>> +Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
>> +		xloadflags to support booting a 64 bit kernel from 32 bit
>> +		EFI
> 
> The changelog should I think declare that we add documentation for the 2.13 
> protocol iteration as well.
> 
> Also, please use a consistent spelling of '32-bit' and '64-bit' in the same 
> sentence!

Okay.

> 
>> +Field name:	acpi_rsdp_addr
>> +Type:		write
>> +Offset/size:	0x268/8
>> +Protocol:	2.14+
>> +
>> +  This field can be set by the boot loader to tell the kernel the
>> +  physical address of the ACPI RSDP table.
>> +
>> +  A value of 0 indicates the kernel should fall back to the standard
>> +  methods to locate the RSDP (search in EBDA/low memory).
> 
> That's not the only method used: the ACPI RSDP address can also be discovered via 
> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.

Sure, but this is valid for booting via EFI only.

> 
>>  **** THE IMAGE CHECKSUM
>>  
>> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>> index 850b8762e889..e7184127f309 100644
>> --- a/arch/x86/boot/header.S
>> +++ b/arch/x86/boot/header.S
>> @@ -300,7 +300,7 @@ _start:
>>  	# Part 2 of the header, from the old setup.S
>>  
>>  		.ascii	"HdrS"		# header signature
>> -		.word	0x020d		# header version number (>= 0x0105)
>> +		.word	0x020e		# header version number (>= 0x0105)
>>  					# or else old loadlin-1.5 will fail)
>>  		.globl realmode_swtch
>>  realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
>> @@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
>>  init_size:		.long INIT_SIZE		# kernel initialization size
>>  handover_offset:	.long 0			# Filled in by build.c
>>  
>> +acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
>> +						# ACPI RSDP table, added with
>> +						# version 2.14
> 
> s/pointer to ACPI RSDP table
>  /pointer to the ACPI RSDP table

Okay.

> 
> Also, a more fundamental question: why doesn't Xen use EFI to hand over hardware 
> configuration details?

I think Jan has answered this question quite well.


Juergen

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

On 08/12/17 08:16, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> Xen PVH guests receive the address of the RSDP table from Xen. In order
>> to support booting a Xen PVH guest via grub2 using the standard x86
>> boot entry we need a way fro grub2 to pass the RSDP address to the
>> kernel.
>>
>> For this purpose expand the struct setup_header to hold the physical
>> address of the RSDP address. Being zero means it isn't specified and
>> has to be located the legacy way (searching through low memory or
>> EBDA).
> 
> s/fro
>  /for
> 
> pedantry:
> 
> s/grub2
>  /Grub2

Okay.

> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>>  Documentation/x86/boot.txt            | 19 +++++++++++++++++++
>>  arch/x86/boot/header.S                |  6 +++++-
>>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>>  3 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
>> index 5e9b826b5f62..a33c224797e4 100644
>> --- a/Documentation/x86/boot.txt
>> +++ b/Documentation/x86/boot.txt
>> @@ -61,6 +61,13 @@ Protocol 2.12:	(Kernel 3.8) Added the xloadflags field and extension fields
>>  	 	to struct boot_params for loading bzImage and ramdisk
>>  		above 4G in 64bit.
>>  
>> +Protocol 2.13:	(Kernel 3.14) Support 32- and 64-bit flags being set in
>> +		xloadflags to support booting a 64 bit kernel from 32 bit
>> +		EFI
> 
> The changelog should I think declare that we add documentation for the 2.13 
> protocol iteration as well.
> 
> Also, please use a consistent spelling of '32-bit' and '64-bit' in the same 
> sentence!

Okay.

> 
>> +Field name:	acpi_rsdp_addr
>> +Type:		write
>> +Offset/size:	0x268/8
>> +Protocol:	2.14+
>> +
>> +  This field can be set by the boot loader to tell the kernel the
>> +  physical address of the ACPI RSDP table.
>> +
>> +  A value of 0 indicates the kernel should fall back to the standard
>> +  methods to locate the RSDP (search in EBDA/low memory).
> 
> That's not the only method used: the ACPI RSDP address can also be discovered via 
> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.

Sure, but this is valid for booting via EFI only.

> 
>>  **** THE IMAGE CHECKSUM
>>  
>> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>> index 850b8762e889..e7184127f309 100644
>> --- a/arch/x86/boot/header.S
>> +++ b/arch/x86/boot/header.S
>> @@ -300,7 +300,7 @@ _start:
>>  	# Part 2 of the header, from the old setup.S
>>  
>>  		.ascii	"HdrS"		# header signature
>> -		.word	0x020d		# header version number (>= 0x0105)
>> +		.word	0x020e		# header version number (>= 0x0105)
>>  					# or else old loadlin-1.5 will fail)
>>  		.globl realmode_swtch
>>  realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG
>> @@ -558,6 +558,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
>>  init_size:		.long INIT_SIZE		# kernel initialization size
>>  handover_offset:	.long 0			# Filled in by build.c
>>  
>> +acpi_rsdp_addr:		.quad 0			# 64-bit physical pointer to
>> +						# ACPI RSDP table, added with
>> +						# version 2.14
> 
> s/pointer to ACPI RSDP table
>  /pointer to the ACPI RSDP table

Okay.

> 
> Also, a more fundamental question: why doesn't Xen use EFI to hand over hardware 
> configuration details?

I think Jan has answered this question quite well.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Juergen Gross]

On 08/12/17 08:22, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> When booted via the special PVH entry save the RSDP address set in the
>> boot information block in struct boot_params. This will enable Xen to
>> locate the RSDP at an arbitrary address.
>>
>> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
>> which should have been 0x020c.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2: set bootloader version to 2.14 (Roger Pau Monné)
>> ---
>>  arch/x86/xen/enlighten_pvh.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
>> index 436c4f003e17..036e3a5f284a 100644
>> --- a/arch/x86/xen/enlighten_pvh.c
>> +++ b/arch/x86/xen/enlighten_pvh.c
>> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
>>  	 *
>>  	 * Version 2.12 supports Xen entry point but we will use default x86/PC
>>  	 * environment (i.e. hardware_subarch 0).
>> +	 * The RSDP address is available from version 2.14 on.
>>  	 */
>> -	pvh_bootparams.hdr.version = 0x212;
>> +	pvh_bootparams.hdr.version = 0x20e;
> 
> While 0x212 was "obvious" to read but totally wrong, it would be less fragile and 
> more readable if the version was generated as something like:
> 
> 	pvh_bootparams.hdr.version = (2 << 8) | 14;

Sure, I can make that change.

> 
> similar to how it's written in other cases:
> 
>>  	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
> 
> Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug appears to 
> have been introduced at around v4.12.

While not really being very important, this seems to be cleaner, yes.
After all this value is visible in sysfs, so it should be correct.


Juergen

Re: [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests

[Juergen Gross]

On 08/12/17 08:22, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> When booted via the special PVH entry save the RSDP address set in the
>> boot information block in struct boot_params. This will enable Xen to
>> locate the RSDP at an arbitrary address.
>>
>> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212
>> which should have been 0x020c.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2: set bootloader version to 2.14 (Roger Pau Monné)
>> ---
>>  arch/x86/xen/enlighten_pvh.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
>> index 436c4f003e17..036e3a5f284a 100644
>> --- a/arch/x86/xen/enlighten_pvh.c
>> +++ b/arch/x86/xen/enlighten_pvh.c
>> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void)
>>  	 *
>>  	 * Version 2.12 supports Xen entry point but we will use default x86/PC
>>  	 * environment (i.e. hardware_subarch 0).
>> +	 * The RSDP address is available from version 2.14 on.
>>  	 */
>> -	pvh_bootparams.hdr.version = 0x212;
>> +	pvh_bootparams.hdr.version = 0x20e;
> 
> While 0x212 was "obvious" to read but totally wrong, it would be less fragile and 
> more readable if the version was generated as something like:
> 
> 	pvh_bootparams.hdr.version = (2 << 8) | 14;

Sure, I can make that change.

> 
> similar to how it's written in other cases:
> 
>>  	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
> 
> Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug appears to 
> have been introduced at around v4.12.

While not really being very important, this seems to be cleaner, yes.
After all this value is visible in sysfs, so it should be correct.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> >> +Offset/size:	0x268/8
> >> +Protocol:	2.14+
> >> +
> >> +  This field can be set by the boot loader to tell the kernel the
> >> +  physical address of the ACPI RSDP table.
> >> +
> >> +  A value of 0 indicates the kernel should fall back to the standard
> >> +  methods to locate the RSDP (search in EBDA/low memory).
> > 
> > That's not the only method used: the ACPI RSDP address can also be discovered via 
> > efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.
> 
> Sure, but this is valid for booting via EFI only.

Yeah, so what I tried to say is that the description as written is not fully 
correct and triggered my pedantry:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP (search in EBDA/low memory).

To make it correct we need to either write less:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP.

or write more and make it open ended so it doesn't have to be extended with every 
method of getting the RSDP that might be added in the future:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP (search in EBDA/low memory, get it from
 +  EFI if present, etc.).

... or so?

Thanks,

	Ingo

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> >> +Offset/size:	0x268/8
> >> +Protocol:	2.14+
> >> +
> >> +  This field can be set by the boot loader to tell the kernel the
> >> +  physical address of the ACPI RSDP table.
> >> +
> >> +  A value of 0 indicates the kernel should fall back to the standard
> >> +  methods to locate the RSDP (search in EBDA/low memory).
> > 
> > That's not the only method used: the ACPI RSDP address can also be discovered via 
> > efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.
> 
> Sure, but this is valid for booting via EFI only.

Yeah, so what I tried to say is that the description as written is not fully 
correct and triggered my pedantry:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP (search in EBDA/low memory).

To make it correct we need to either write less:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP.

or write more and make it open ended so it doesn't have to be extended with every 
method of getting the RSDP that might be added in the future:

 +  A value of 0 indicates the kernel should fall back to the standard
 +  methods to locate the RSDP (search in EBDA/low memory, get it from
 +  EFI if present, etc.).

... or so?

Thanks,

	Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

On 08/12/17 09:48, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>>>> +Offset/size:	0x268/8
>>>> +Protocol:	2.14+
>>>> +
>>>> +  This field can be set by the boot loader to tell the kernel the
>>>> +  physical address of the ACPI RSDP table.
>>>> +
>>>> +  A value of 0 indicates the kernel should fall back to the standard
>>>> +  methods to locate the RSDP (search in EBDA/low memory).
>>>
>>> That's not the only method used: the ACPI RSDP address can also be discovered via 
>>> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.
>>
>> Sure, but this is valid for booting via EFI only.
> 
> Yeah, so what I tried to say is that the description as written is not fully 
> correct and triggered my pedantry:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP (search in EBDA/low memory).
> 
> To make it correct we need to either write less:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP.
> 
> or write more and make it open ended so it doesn't have to be extended with every 
> method of getting the RSDP that might be added in the future:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP (search in EBDA/low memory, get it from
>  +  EFI if present, etc.).
> 
> ... or so?

Aah, okay. I got your remark wrong then.

I think I'll go with the shorter variant.


Juergen

Re: [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header

[Juergen Gross]

On 08/12/17 09:48, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>>>> +Offset/size:	0x268/8
>>>> +Protocol:	2.14+
>>>> +
>>>> +  This field can be set by the boot loader to tell the kernel the
>>>> +  physical address of the ACPI RSDP table.
>>>> +
>>>> +  A value of 0 indicates the kernel should fall back to the standard
>>>> +  methods to locate the RSDP (search in EBDA/low memory).
>>>
>>> That's not the only method used: the ACPI RSDP address can also be discovered via 
>>> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values.
>>
>> Sure, but this is valid for booting via EFI only.
> 
> Yeah, so what I tried to say is that the description as written is not fully 
> correct and triggered my pedantry:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP (search in EBDA/low memory).
> 
> To make it correct we need to either write less:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP.
> 
> or write more and make it open ended so it doesn't have to be extended with every 
> method of getting the RSDP that might be added in the future:
> 
>  +  A value of 0 indicates the kernel should fall back to the standard
>  +  methods to locate the RSDP (search in EBDA/low memory, get it from
>  +  EFI if present, etc.).
> 
> ... or so?

Aah, okay. I got your remark wrong then.

I think I'll go with the shorter variant.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 08:05, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:

...

> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return boot_params.hdr.acpi_rsdp_addr;
> }
> 
> 4)
> 
> Add this to arch/x86/include/asm/acpi.h:
> 
> extern acpi_physical_address acpi_arch_get_root_pointer(void);

Uuh, this leads to problems for files including <asm/acpi.h> directly:
acpi_physical_address won't be defined, and including <acpi/actypes.h>
from arch/x86/include/asm/acpi.h will lead to:

#error unknown ACPI_MACHINE_WIDTH

This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
which seems to be the wrong layering.

So I could:

a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
   instead
b) don't use acpi_physical_address but either u64 or unsigned long.
c) ?

What would be your preference?


Juergen

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 08:05, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:

...

> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> 	return boot_params.hdr.acpi_rsdp_addr;
> }
> 
> 4)
> 
> Add this to arch/x86/include/asm/acpi.h:
> 
> extern acpi_physical_address acpi_arch_get_root_pointer(void);

Uuh, this leads to problems for files including <asm/acpi.h> directly:
acpi_physical_address won't be defined, and including <acpi/actypes.h>
from arch/x86/include/asm/acpi.h will lead to:

#error unknown ACPI_MACHINE_WIDTH

This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
which seems to be the wrong layering.

So I could:

a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
   instead
b) don't use acpi_physical_address but either u64 or unsigned long.
c) ?

What would be your preference?


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> On 08/12/17 08:05, Ingo Molnar wrote:
> > 
> > * Juergen Gross <jgross@suse.com> wrote:
> 
> ...
> 
> > acpi_physical_address acpi_arch_get_root_pointer(void)
> > {
> > 	return boot_params.hdr.acpi_rsdp_addr;
> > }
> > 
> > 4)
> > 
> > Add this to arch/x86/include/asm/acpi.h:
> > 
> > extern acpi_physical_address acpi_arch_get_root_pointer(void);
> 
> Uuh, this leads to problems for files including <asm/acpi.h> directly:
> acpi_physical_address won't be defined, and including <acpi/actypes.h>
> from arch/x86/include/asm/acpi.h will lead to:
> 
> #error unknown ACPI_MACHINE_WIDTH
> 
> This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
> which seems to be the wrong layering.
> 
> So I could:
> 
> a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
>    instead
> b) don't use acpi_physical_address but either u64 or unsigned long.
> c) ?
> 
> What would be your preference?

Would it help if you put the prototype into linux/acpi.h perhaps? It's a generic 
facility in principle, even if only used by x86 at the moment.

Thanks,

	Ingo

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> On 08/12/17 08:05, Ingo Molnar wrote:
> > 
> > * Juergen Gross <jgross@suse.com> wrote:
> 
> ...
> 
> > acpi_physical_address acpi_arch_get_root_pointer(void)
> > {
> > 	return boot_params.hdr.acpi_rsdp_addr;
> > }
> > 
> > 4)
> > 
> > Add this to arch/x86/include/asm/acpi.h:
> > 
> > extern acpi_physical_address acpi_arch_get_root_pointer(void);
> 
> Uuh, this leads to problems for files including <asm/acpi.h> directly:
> acpi_physical_address won't be defined, and including <acpi/actypes.h>
> from arch/x86/include/asm/acpi.h will lead to:
> 
> #error unknown ACPI_MACHINE_WIDTH
> 
> This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
> which seems to be the wrong layering.
> 
> So I could:
> 
> a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
>    instead
> b) don't use acpi_physical_address but either u64 or unsigned long.
> c) ?
> 
> What would be your preference?

Would it help if you put the prototype into linux/acpi.h perhaps? It's a generic 
facility in principle, even if only used by x86 at the moment.

Thanks,

	Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 12:26, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> On 08/12/17 08:05, Ingo Molnar wrote:
>>>
>>> * Juergen Gross <jgross@suse.com> wrote:
>>
>> ...
>>
>>> acpi_physical_address acpi_arch_get_root_pointer(void)
>>> {
>>> 	return boot_params.hdr.acpi_rsdp_addr;
>>> }
>>>
>>> 4)
>>>
>>> Add this to arch/x86/include/asm/acpi.h:
>>>
>>> extern acpi_physical_address acpi_arch_get_root_pointer(void);
>>
>> Uuh, this leads to problems for files including <asm/acpi.h> directly:
>> acpi_physical_address won't be defined, and including <acpi/actypes.h>
>> from arch/x86/include/asm/acpi.h will lead to:
>>
>> #error unknown ACPI_MACHINE_WIDTH
>>
>> This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
>> which seems to be the wrong layering.
>>
>> So I could:
>>
>> a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
>>    instead
>> b) don't use acpi_physical_address but either u64 or unsigned long.
>> c) ?
>>
>> What would be your preference?
> 
> Would it help if you put the prototype into linux/acpi.h perhaps? It's a generic 
> facility in principle, even if only used by x86 at the moment.

Yes, that seems to work.


Thanks,

Juergen

Re: [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available

[Juergen Gross]

On 08/12/17 12:26, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> On 08/12/17 08:05, Ingo Molnar wrote:
>>>
>>> * Juergen Gross <jgross@suse.com> wrote:
>>
>> ...
>>
>>> acpi_physical_address acpi_arch_get_root_pointer(void)
>>> {
>>> 	return boot_params.hdr.acpi_rsdp_addr;
>>> }
>>>
>>> 4)
>>>
>>> Add this to arch/x86/include/asm/acpi.h:
>>>
>>> extern acpi_physical_address acpi_arch_get_root_pointer(void);
>>
>> Uuh, this leads to problems for files including <asm/acpi.h> directly:
>> acpi_physical_address won't be defined, and including <acpi/actypes.h>
>> from arch/x86/include/asm/acpi.h will lead to:
>>
>> #error unknown ACPI_MACHINE_WIDTH
>>
>> This can only be avoided by including <linux/acpi.h> from <asm/acpi.h>
>> which seems to be the wrong layering.
>>
>> So I could:
>>
>> a) modify the sources including <asm/acpi.h> to use <linux/acpi.h>
>>    instead
>> b) don't use acpi_physical_address but either u64 or unsigned long.
>> c) ?
>>
>> What would be your preference?
> 
> Would it help if you put the prototype into linux/acpi.h perhaps? It's a generic 
> facility in principle, even if only used by x86 at the moment.

Yes, that seems to work.


Thanks,

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel