xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
@ 2023-06-05 10:28 Ross Lagerwall
  2023-06-08 16:38 ` Konrad Rzeszutek Wilk
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ross Lagerwall @ 2023-06-05 10:28 UTC (permalink / raw)
  To: linux-kernel, xen-devel
  Cc: Jan Beulich, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Juergen Gross, Boris Ostrovsky, Peter Jones,
	Konrad Rzeszutek Wilk, Ross Lagerwall

To facilitate diskless iSCSI boot, the firmware can place a table of
configuration details in memory called the iBFT. The presence of this
table is not specified, nor is the precise location (and it's not in the
E820) so the kernel has to search for a magic marker to find it.

When running under Xen, Dom 0 does not have access to the entire host's
memory, only certain regions which are identity-mapped which means that
the pseudo-physical address in Dom0 == real host physical address.
Add the iBFT search bounds as a reserved region which causes it to be
identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
access to the specific physical memory to correctly search for the iBFT
magic marker (and later access the full table).

This necessitates moving the call to reserve_ibft_region() somewhat
later so that it is called after e820__memory_setup() which is when the
Xen identity mapping adjustments are applied. The precise location of
the call is not too important so I've put it alongside dmi_setup() which
does similar scanning of memory for configuration tables.

Finally in the iBFT find code, instead of using isa_bus_to_virt() which
doesn't do the right thing under Xen, use early_memremap() like the
dmi_setup() code does.

The result of these changes is that it is possible to boot a diskless
Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
find the iBFT and consequently, the iSCSI root disk.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---

In v3:
* Expanded commit message.
* Used IBFT_ constants.

 arch/x86/kernel/setup.c            |  2 +-
 arch/x86/xen/setup.c               | 28 +++++++++++++++++++---------
 drivers/firmware/iscsi_ibft_find.c | 26 +++++++++++++++++---------
 include/linux/iscsi_ibft.h         | 10 +++++++++-
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 16babff771bd..616b80507abd 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -796,7 +796,6 @@ static void __init early_reserve_memory(void)
 
 	memblock_x86_reserve_range_setup_data();
 
-	reserve_ibft_region();
 	reserve_bios_regions();
 	trim_snb_memory();
 }
@@ -1032,6 +1031,7 @@ void __init setup_arch(char **cmdline_p)
 	if (efi_enabled(EFI_BOOT))
 		efi_init();
 
+	reserve_ibft_region();
 	dmi_setup();
 
 	/*
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index c2be3efb2ba0..8b5cf7bb1f47 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/iscsi_ibft.h>
 #include <linux/sched.h>
 #include <linux/kstrtox.h>
 #include <linux/mm.h>
@@ -764,17 +765,26 @@ char * __init xen_memory_setup(void)
 	BUG_ON(memmap.nr_entries == 0);
 	xen_e820_table.nr_entries = memmap.nr_entries;
 
-	/*
-	 * Xen won't allow a 1:1 mapping to be created to UNUSABLE
-	 * regions, so if we're using the machine memory map leave the
-	 * region as RAM as it is in the pseudo-physical map.
-	 *
-	 * UNUSABLE regions in domUs are not handled and will need
-	 * a patch in the future.
-	 */
-	if (xen_initial_domain())
+	if (xen_initial_domain()) {
+		/*
+		 * Xen won't allow a 1:1 mapping to be created to UNUSABLE
+		 * regions, so if we're using the machine memory map leave the
+		 * region as RAM as it is in the pseudo-physical map.
+		 *
+		 * UNUSABLE regions in domUs are not handled and will need
+		 * a patch in the future.
+		 */
 		xen_ignore_unusable();
 
+#ifdef CONFIG_ISCSI_IBFT_FIND
+		/* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
+		xen_e820_table.entries[xen_e820_table.nr_entries].addr = IBFT_START;
+		xen_e820_table.entries[xen_e820_table.nr_entries].size = IBFT_END - IBFT_START;
+		xen_e820_table.entries[xen_e820_table.nr_entries].type = E820_TYPE_RESERVED;
+		xen_e820_table.nr_entries++;
+#endif
+	}
+
 	/* Make sure the Xen-supplied memory map is well-ordered. */
 	e820__update_table(&xen_e820_table);
 
diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index 94b49ccd23ac..71f51303c2ba 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -42,8 +42,6 @@ static const struct {
 };
 
 #define IBFT_SIGN_LEN 4
-#define IBFT_START 0x80000 /* 512kB */
-#define IBFT_END 0x100000 /* 1MB */
 #define VGA_MEM 0xA0000 /* VGA buffer */
 #define VGA_SIZE 0x20000 /* 128kB */
 
@@ -52,9 +50,9 @@ static const struct {
  */
 void __init reserve_ibft_region(void)
 {
-	unsigned long pos;
+	unsigned long pos, virt_pos = 0;
 	unsigned int len = 0;
-	void *virt;
+	void *virt = NULL;
 	int i;
 
 	ibft_phys_addr = 0;
@@ -70,13 +68,20 @@ void __init reserve_ibft_region(void)
 		 * so skip that area */
 		if (pos == VGA_MEM)
 			pos += VGA_SIZE;
-		virt = isa_bus_to_virt(pos);
+
+		/* Map page by page */
+		if (offset_in_page(pos) == 0) {
+			if (virt)
+				early_memunmap(virt, PAGE_SIZE);
+			virt = early_memremap_ro(pos, PAGE_SIZE);
+			virt_pos = pos;
+		}
 
 		for (i = 0; i < ARRAY_SIZE(ibft_signs); i++) {
-			if (memcmp(virt, ibft_signs[i].sign, IBFT_SIGN_LEN) ==
-			    0) {
+			if (memcmp(virt + (pos - virt_pos), ibft_signs[i].sign,
+				   IBFT_SIGN_LEN) == 0) {
 				unsigned long *addr =
-				    (unsigned long *)isa_bus_to_virt(pos + 4);
+				    (unsigned long *)(virt + pos - virt_pos + 4);
 				len = *addr;
 				/* if the length of the table extends past 1M,
 				 * the table cannot be valid. */
@@ -84,9 +89,12 @@ void __init reserve_ibft_region(void)
 					ibft_phys_addr = pos;
 					memblock_reserve(ibft_phys_addr, PAGE_ALIGN(len));
 					pr_info("iBFT found at %pa.\n", &ibft_phys_addr);
-					return;
+					goto out;
 				}
 			}
 		}
 	}
+
+out:
+	early_memunmap(virt, PAGE_SIZE);
 }
diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
index 790e7fcfc1a6..e2742748104d 100644
--- a/include/linux/iscsi_ibft.h
+++ b/include/linux/iscsi_ibft.h
@@ -21,12 +21,20 @@
  */
 extern phys_addr_t ibft_phys_addr;
 
+#ifdef CONFIG_ISCSI_IBFT_FIND
+
 /*
  * Routine used to find and reserve the iSCSI Boot Format Table. The
  * physical address is set in the ibft_phys_addr variable.
  */
-#ifdef CONFIG_ISCSI_IBFT_FIND
 void reserve_ibft_region(void);
+
+/*
+ * Physical bounds to search for the iSCSI Boot Format Table.
+ */
+#define IBFT_START 0x80000 /* 512kB */
+#define IBFT_END 0x100000 /* 1MB */
+
 #else
 static inline void reserve_ibft_region(void) {}
 #endif
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-05 10:28 [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 Ross Lagerwall
@ 2023-06-08 16:38 ` Konrad Rzeszutek Wilk
  2023-06-09 14:25   ` Ross Lagerwall
  2023-06-09 15:08 ` Juergen Gross
  2023-06-09 15:16 ` Dave Hansen
  2 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2023-06-08 16:38 UTC (permalink / raw)
  To: Ross Lagerwall
  Cc: linux-kernel, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Juergen Gross,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk

It looks fine to me, but could I ask you to triple check that on
non-Xen it still detects the iBFT?

Thx!

On Mon, Jun 5, 2023 at 6:28 AM Ross Lagerwall <ross.lagerwall@citrix.com> wrote:
>
> To facilitate diskless iSCSI boot, the firmware can place a table of
> configuration details in memory called the iBFT. The presence of this
> table is not specified, nor is the precise location (and it's not in the
> E820) so the kernel has to search for a magic marker to find it.
>
> When running under Xen, Dom 0 does not have access to the entire host's
> memory, only certain regions which are identity-mapped which means that
> the pseudo-physical address in Dom0 == real host physical address.
> Add the iBFT search bounds as a reserved region which causes it to be
> identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> access to the specific physical memory to correctly search for the iBFT
> magic marker (and later access the full table).
>
> This necessitates moving the call to reserve_ibft_region() somewhat
> later so that it is called after e820__memory_setup() which is when the
> Xen identity mapping adjustments are applied. The precise location of
> the call is not too important so I've put it alongside dmi_setup() which
> does similar scanning of memory for configuration tables.
>
> Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> doesn't do the right thing under Xen, use early_memremap() like the
> dmi_setup() code does.
>
> The result of these changes is that it is possible to boot a diskless
> Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> find the iBFT and consequently, the iSCSI root disk.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>
> In v3:
> * Expanded commit message.
> * Used IBFT_ constants.
>
>  arch/x86/kernel/setup.c            |  2 +-
>  arch/x86/xen/setup.c               | 28 +++++++++++++++++++---------
>  drivers/firmware/iscsi_ibft_find.c | 26 +++++++++++++++++---------
>  include/linux/iscsi_ibft.h         | 10 +++++++++-
>  4 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 16babff771bd..616b80507abd 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -796,7 +796,6 @@ static void __init early_reserve_memory(void)
>
>         memblock_x86_reserve_range_setup_data();
>
> -       reserve_ibft_region();
>         reserve_bios_regions();
>         trim_snb_memory();
>  }
> @@ -1032,6 +1031,7 @@ void __init setup_arch(char **cmdline_p)
>         if (efi_enabled(EFI_BOOT))
>                 efi_init();
>
> +       reserve_ibft_region();
>         dmi_setup();
>
>         /*
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index c2be3efb2ba0..8b5cf7bb1f47 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -6,6 +6,7 @@
>   */
>
>  #include <linux/init.h>
> +#include <linux/iscsi_ibft.h>
>  #include <linux/sched.h>
>  #include <linux/kstrtox.h>
>  #include <linux/mm.h>
> @@ -764,17 +765,26 @@ char * __init xen_memory_setup(void)
>         BUG_ON(memmap.nr_entries == 0);
>         xen_e820_table.nr_entries = memmap.nr_entries;
>
> -       /*
> -        * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> -        * regions, so if we're using the machine memory map leave the
> -        * region as RAM as it is in the pseudo-physical map.
> -        *
> -        * UNUSABLE regions in domUs are not handled and will need
> -        * a patch in the future.
> -        */
> -       if (xen_initial_domain())
> +       if (xen_initial_domain()) {
> +               /*
> +                * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> +                * regions, so if we're using the machine memory map leave the
> +                * region as RAM as it is in the pseudo-physical map.
> +                *
> +                * UNUSABLE regions in domUs are not handled and will need
> +                * a patch in the future.
> +                */
>                 xen_ignore_unusable();
>
> +#ifdef CONFIG_ISCSI_IBFT_FIND
> +               /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
> +               xen_e820_table.entries[xen_e820_table.nr_entries].addr = IBFT_START;
> +               xen_e820_table.entries[xen_e820_table.nr_entries].size = IBFT_END - IBFT_START;
> +               xen_e820_table.entries[xen_e820_table.nr_entries].type = E820_TYPE_RESERVED;
> +               xen_e820_table.nr_entries++;
> +#endif
> +       }
> +
>         /* Make sure the Xen-supplied memory map is well-ordered. */
>         e820__update_table(&xen_e820_table);
>
> diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
> index 94b49ccd23ac..71f51303c2ba 100644
> --- a/drivers/firmware/iscsi_ibft_find.c
> +++ b/drivers/firmware/iscsi_ibft_find.c
> @@ -42,8 +42,6 @@ static const struct {
>  };
>
>  #define IBFT_SIGN_LEN 4
> -#define IBFT_START 0x80000 /* 512kB */
> -#define IBFT_END 0x100000 /* 1MB */
>  #define VGA_MEM 0xA0000 /* VGA buffer */
>  #define VGA_SIZE 0x20000 /* 128kB */
>
> @@ -52,9 +50,9 @@ static const struct {
>   */
>  void __init reserve_ibft_region(void)
>  {
> -       unsigned long pos;
> +       unsigned long pos, virt_pos = 0;
>         unsigned int len = 0;
> -       void *virt;
> +       void *virt = NULL;
>         int i;
>
>         ibft_phys_addr = 0;
> @@ -70,13 +68,20 @@ void __init reserve_ibft_region(void)
>                  * so skip that area */
>                 if (pos == VGA_MEM)
>                         pos += VGA_SIZE;
> -               virt = isa_bus_to_virt(pos);
> +
> +               /* Map page by page */
> +               if (offset_in_page(pos) == 0) {
> +                       if (virt)
> +                               early_memunmap(virt, PAGE_SIZE);
> +                       virt = early_memremap_ro(pos, PAGE_SIZE);
> +                       virt_pos = pos;
> +               }
>
>                 for (i = 0; i < ARRAY_SIZE(ibft_signs); i++) {
> -                       if (memcmp(virt, ibft_signs[i].sign, IBFT_SIGN_LEN) ==
> -                           0) {
> +                       if (memcmp(virt + (pos - virt_pos), ibft_signs[i].sign,
> +                                  IBFT_SIGN_LEN) == 0) {
>                                 unsigned long *addr =
> -                                   (unsigned long *)isa_bus_to_virt(pos + 4);
> +                                   (unsigned long *)(virt + pos - virt_pos + 4);
>                                 len = *addr;
>                                 /* if the length of the table extends past 1M,
>                                  * the table cannot be valid. */
> @@ -84,9 +89,12 @@ void __init reserve_ibft_region(void)
>                                         ibft_phys_addr = pos;
>                                         memblock_reserve(ibft_phys_addr, PAGE_ALIGN(len));
>                                         pr_info("iBFT found at %pa.\n", &ibft_phys_addr);
> -                                       return;
> +                                       goto out;
>                                 }
>                         }
>                 }
>         }
> +
> +out:
> +       early_memunmap(virt, PAGE_SIZE);
>  }
> diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
> index 790e7fcfc1a6..e2742748104d 100644
> --- a/include/linux/iscsi_ibft.h
> +++ b/include/linux/iscsi_ibft.h
> @@ -21,12 +21,20 @@
>   */
>  extern phys_addr_t ibft_phys_addr;
>
> +#ifdef CONFIG_ISCSI_IBFT_FIND
> +
>  /*
>   * Routine used to find and reserve the iSCSI Boot Format Table. The
>   * physical address is set in the ibft_phys_addr variable.
>   */
> -#ifdef CONFIG_ISCSI_IBFT_FIND
>  void reserve_ibft_region(void);
> +
> +/*
> + * Physical bounds to search for the iSCSI Boot Format Table.
> + */
> +#define IBFT_START 0x80000 /* 512kB */
> +#define IBFT_END 0x100000 /* 1MB */
> +
>  #else
>  static inline void reserve_ibft_region(void) {}
>  #endif
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-08 16:38 ` Konrad Rzeszutek Wilk
@ 2023-06-09 14:25   ` Ross Lagerwall
  0 siblings, 0 replies; 9+ messages in thread
From: Ross Lagerwall @ 2023-06-09 14:25 UTC (permalink / raw)
  To: konrad
  Cc: linux-kernel, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Juergen Gross,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk

> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> on behalf of Konrad Rzeszutek Wilk <konrad@darnok.org>
> Sent: Thursday, June 8, 2023 5:38 PM
> To: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>; Jan Beulich <jbeulich@suse.com>; Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; Dave Hansen <dave.hansen@linux.intel.com>; x86@kernel.org <x86@kernel.org>; Juergen Gross <jgross@suse.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>; Peter Jones <pjones@redhat.com>; Konrad Rzeszutek Wilk <konrad@kernel.org>
> Subject: Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 
>  
> It looks fine to me, but could I ask you to triple check that on
> non-Xen it still detects the iBFT?
> 
> Thx!

I have checked again, and yes, it still detects the iBFT in the non-Xen
case and the information in /sys/firmware/ibft looks correct.

Ross

> 
> On Mon, Jun 5, 2023 at 6:28 AM Ross Lagerwall <ross.lagerwall@citrix.com> wrote:
> >
> > To facilitate diskless iSCSI boot, the firmware can place a table of
> > configuration details in memory called the iBFT. The presence of this
> > table is not specified, nor is the precise location (and it's not in the
> > E820) so the kernel has to search for a magic marker to find it.
> >
> > When running under Xen, Dom 0 does not have access to the entire host's
> > memory, only certain regions which are identity-mapped which means that
> > the pseudo-physical address in Dom0 == real host physical address.
> > Add the iBFT search bounds as a reserved region which causes it to be
> > identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> > access to the specific physical memory to correctly search for the iBFT
> > magic marker (and later access the full table).
> >
> > This necessitates moving the call to reserve_ibft_region() somewhat
> > later so that it is called after e820__memory_setup() which is when the
> > Xen identity mapping adjustments are applied. The precise location of
> > the call is not too important so I've put it alongside dmi_setup() which
> > does similar scanning of memory for configuration tables.
> >
> > Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> > doesn't do the right thing under Xen, use early_memremap() like the
> > dmi_setup() code does.
> >
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > ---
> >
> > In v3:
> > * Expanded commit message.
> > * Used IBFT_ constants.
> >
> >  arch/x86/kernel/setup.c            |  2 +-
> >  arch/x86/xen/setup.c               | 28 +++++++++++++++++++---------
> >  drivers/firmware/iscsi_ibft_find.c | 26 +++++++++++++++++---------
> >  include/linux/iscsi_ibft.h         | 10 +++++++++-
> >  4 files changed, 46 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 16babff771bd..616b80507abd 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -796,7 +796,6 @@ static void __init early_reserve_memory(void)
> >
> >         memblock_x86_reserve_range_setup_data();
> >
> > -       reserve_ibft_region();
> >         reserve_bios_regions();
> >         trim_snb_memory();
> >  }
> > @@ -1032,6 +1031,7 @@ void __init setup_arch(char **cmdline_p)
> >         if (efi_enabled(EFI_BOOT))
> >                 efi_init();
> >
> > +       reserve_ibft_region();
> >         dmi_setup();
> >
> >         /*
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index c2be3efb2ba0..8b5cf7bb1f47 100644
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -6,6 +6,7 @@
> >   */
> >
> >  #include <linux/init.h>
> > +#include <linux/iscsi_ibft.h>
> >  #include <linux/sched.h>
> >  #include <linux/kstrtox.h>
> >  #include <linux/mm.h>
> > @@ -764,17 +765,26 @@ char * __init xen_memory_setup(void)
> >         BUG_ON(memmap.nr_entries == 0);
> >         xen_e820_table.nr_entries = memmap.nr_entries;
> >
> > -       /*
> > -        * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> > -        * regions, so if we're using the machine memory map leave the
> > -        * region as RAM as it is in the pseudo-physical map.
> > -        *
> > -        * UNUSABLE regions in domUs are not handled and will need
> > -        * a patch in the future.
> > -        */
> > -       if (xen_initial_domain())
> > +       if (xen_initial_domain()) {
> > +               /*
> > +                * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> > +                * regions, so if we're using the machine memory map leave the
> > +                * region as RAM as it is in the pseudo-physical map.
> > +                *
> > +                * UNUSABLE regions in domUs are not handled and will need
> > +                * a patch in the future.
> > +                */
> >                 xen_ignore_unusable();
> >
> > +#ifdef CONFIG_ISCSI_IBFT_FIND
> > +               /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
> > +               xen_e820_table.entries[xen_e820_table.nr_entries].addr = IBFT_START;
> > +               xen_e820_table.entries[xen_e820_table.nr_entries].size = IBFT_END - IBFT_START;
> > +               xen_e820_table.entries[xen_e820_table.nr_entries].type = E820_TYPE_RESERVED;
> > +               xen_e820_table.nr_entries++;
> > +#endif
> > +       }
> > +
> >         /* Make sure the Xen-supplied memory map is well-ordered. */
> >         e820__update_table(&xen_e820_table);
> >
> > diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
> > index 94b49ccd23ac..71f51303c2ba 100644
> > --- a/drivers/firmware/iscsi_ibft_find.c
> > +++ b/drivers/firmware/iscsi_ibft_find.c
> > @@ -42,8 +42,6 @@ static const struct {
> >  };
> >
> >  #define IBFT_SIGN_LEN 4
> > -#define IBFT_START 0x80000 /* 512kB */
> > -#define IBFT_END 0x100000 /* 1MB */
> >  #define VGA_MEM 0xA0000 /* VGA buffer */
> >  #define VGA_SIZE 0x20000 /* 128kB */
> >
> > @@ -52,9 +50,9 @@ static const struct {
> >   */
> >  void __init reserve_ibft_region(void)
> >  {
> > -       unsigned long pos;
> > +       unsigned long pos, virt_pos = 0;
> >         unsigned int len = 0;
> > -       void *virt;
> > +       void *virt = NULL;
> >         int i;
> >
> >         ibft_phys_addr = 0;
> > @@ -70,13 +68,20 @@ void __init reserve_ibft_region(void)
> >                  * so skip that area */
> >                 if (pos == VGA_MEM)
> >                         pos += VGA_SIZE;
> > -               virt = isa_bus_to_virt(pos);
> > +
> > +               /* Map page by page */
> > +               if (offset_in_page(pos) == 0) {
> > +                       if (virt)
> > +                               early_memunmap(virt, PAGE_SIZE);
> > +                       virt = early_memremap_ro(pos, PAGE_SIZE);
> > +                       virt_pos = pos;
> > +               }
> >
> >                 for (i = 0; i < ARRAY_SIZE(ibft_signs); i++) {
> > -                       if (memcmp(virt, ibft_signs[i].sign, IBFT_SIGN_LEN) ==
> > -                           0) {
> > +                       if (memcmp(virt + (pos - virt_pos), ibft_signs[i].sign,
> > +                                  IBFT_SIGN_LEN) == 0) {
> >                                 unsigned long *addr =
> > -                                   (unsigned long *)isa_bus_to_virt(pos + 4);
> > +                                   (unsigned long *)(virt + pos - virt_pos + 4);
> >                                 len = *addr;
> >                                 /* if the length of the table extends past 1M,
> >                                  * the table cannot be valid. */
> > @@ -84,9 +89,12 @@ void __init reserve_ibft_region(void)
> >                                         ibft_phys_addr = pos;
> >                                         memblock_reserve(ibft_phys_addr, PAGE_ALIGN(len));
> >                                         pr_info("iBFT found at %pa.\n", &ibft_phys_addr);
> > -                                       return;
> > +                                       goto out;
> >                                 }
> >                         }
> >                 }
> >         }
> > +
> > +out:
> > +       early_memunmap(virt, PAGE_SIZE);
> >  }
> > diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
> > index 790e7fcfc1a6..e2742748104d 100644
> > --- a/include/linux/iscsi_ibft.h
> > +++ b/include/linux/iscsi_ibft.h
> > @@ -21,12 +21,20 @@
> >   */
> >  extern phys_addr_t ibft_phys_addr;
> >
> > +#ifdef CONFIG_ISCSI_IBFT_FIND
> > +
> >  /*
> >   * Routine used to find and reserve the iSCSI Boot Format Table. The
> >   * physical address is set in the ibft_phys_addr variable.
> >   */
> > -#ifdef CONFIG_ISCSI_IBFT_FIND
> >  void reserve_ibft_region(void);
> > +
> > +/*
> > + * Physical bounds to search for the iSCSI Boot Format Table.
> > + */
> > +#define IBFT_START 0x80000 /* 512kB */
> > +#define IBFT_END 0x100000 /* 1MB */
> > +
> >  #else
> >  static inline void reserve_ibft_region(void) {}
> >  #endif
> > --
> > 2.31.1
> >
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-05 10:28 [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 Ross Lagerwall
  2023-06-08 16:38 ` Konrad Rzeszutek Wilk
@ 2023-06-09 15:08 ` Juergen Gross
  2023-06-09 15:22   ` Konrad Rzeszutek Wilk
  2023-06-09 15:16 ` Dave Hansen
  2 siblings, 1 reply; 9+ messages in thread
From: Juergen Gross @ 2023-06-09 15:08 UTC (permalink / raw)
  To: Ross Lagerwall, linux-kernel, xen-devel
  Cc: Jan Beulich, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Boris Ostrovsky, Peter Jones,
	Konrad Rzeszutek Wilk


[-- Attachment #1.1.1: Type: text/plain, Size: 1702 bytes --]

On 05.06.23 12:28, Ross Lagerwall wrote:
> To facilitate diskless iSCSI boot, the firmware can place a table of
> configuration details in memory called the iBFT. The presence of this
> table is not specified, nor is the precise location (and it's not in the
> E820) so the kernel has to search for a magic marker to find it.
> 
> When running under Xen, Dom 0 does not have access to the entire host's
> memory, only certain regions which are identity-mapped which means that
> the pseudo-physical address in Dom0 == real host physical address.
> Add the iBFT search bounds as a reserved region which causes it to be
> identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> access to the specific physical memory to correctly search for the iBFT
> magic marker (and later access the full table).
> 
> This necessitates moving the call to reserve_ibft_region() somewhat
> later so that it is called after e820__memory_setup() which is when the
> Xen identity mapping adjustments are applied. The precise location of
> the call is not too important so I've put it alongside dmi_setup() which
> does similar scanning of memory for configuration tables.
> 
> Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> doesn't do the right thing under Xen, use early_memremap() like the
> dmi_setup() code does.
> 
> The result of these changes is that it is possible to boot a diskless
> Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> find the iBFT and consequently, the iSCSI root disk.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-05 10:28 [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 Ross Lagerwall
  2023-06-08 16:38 ` Konrad Rzeszutek Wilk
  2023-06-09 15:08 ` Juergen Gross
@ 2023-06-09 15:16 ` Dave Hansen
  2023-06-09 15:27   ` Konrad Rzeszutek Wilk
  2 siblings, 1 reply; 9+ messages in thread
From: Dave Hansen @ 2023-06-09 15:16 UTC (permalink / raw)
  To: Ross Lagerwall, linux-kernel, xen-devel
  Cc: Jan Beulich, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Juergen Gross, Boris Ostrovsky, Peter Jones,
	Konrad Rzeszutek Wilk

On 6/5/23 03:28, Ross Lagerwall wrote:
> The result of these changes is that it is possible to boot a diskless
> Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> find the iBFT and consequently, the iSCSI root disk.

Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # for x86

The work in this patch seems pretty evenly split between x86 and iSCSI.
Any preferences on who picks it up?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-09 15:08 ` Juergen Gross
@ 2023-06-09 15:22   ` Konrad Rzeszutek Wilk
  2023-06-09 15:32     ` Dave Hansen
  0 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2023-06-09 15:22 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Ross Lagerwall, LKML, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, X86 ML,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

Feel free to add my Acked-by.

Dave, are you OK with the change in where the reserve call is made?

Thx

On Fri, Jun 9, 2023, 11:08 AM Juergen Gross <jgross@suse.com> wrote:

> On 05.06.23 12:28, Ross Lagerwall wrote:
> > To facilitate diskless iSCSI boot, the firmware can place a table of
> > configuration details in memory called the iBFT. The presence of this
> > table is not specified, nor is the precise location (and it's not in the
> > E820) so the kernel has to search for a magic marker to find it.
> >
> > When running under Xen, Dom 0 does not have access to the entire host's
> > memory, only certain regions which are identity-mapped which means that
> > the pseudo-physical address in Dom0 == real host physical address.
> > Add the iBFT search bounds as a reserved region which causes it to be
> > identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> > access to the specific physical memory to correctly search for the iBFT
> > magic marker (and later access the full table).
> >
> > This necessitates moving the call to reserve_ibft_region() somewhat
> > later so that it is called after e820__memory_setup() which is when the
> > Xen identity mapping adjustments are applied. The precise location of
> > the call is not too important so I've put it alongside dmi_setup() which
> > does similar scanning of memory for configuration tables.
> >
> > Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> > doesn't do the right thing under Xen, use early_memremap() like the
> > dmi_setup() code does.
> >
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
>
>
> Juergen
>
>

[-- Attachment #2: Type: text/html, Size: 2628 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-09 15:16 ` Dave Hansen
@ 2023-06-09 15:27   ` Konrad Rzeszutek Wilk
  2023-06-09 15:30     ` Juergen Gross
  0 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2023-06-09 15:27 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Ross Lagerwall, LKML, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, X86 ML, Juergen Gross,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

Usually I put it in my tree (ibft) but since it is so simple and the user
is Xen it would make more sense to do it via the Xen tree (Juergen).

Thx

On Fri, Jun 9, 2023, 11:16 AM Dave Hansen <dave.hansen@intel.com> wrote:

> On 6/5/23 03:28, Ross Lagerwall wrote:
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # for x86
>
> The work in this patch seems pretty evenly split between x86 and iSCSI.
> Any preferences on who picks it up?
>

[-- Attachment #2: Type: text/html, Size: 1080 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-09 15:27   ` Konrad Rzeszutek Wilk
@ 2023-06-09 15:30     ` Juergen Gross
  0 siblings, 0 replies; 9+ messages in thread
From: Juergen Gross @ 2023-06-09 15:30 UTC (permalink / raw)
  To: konrad, Dave Hansen
  Cc: Ross Lagerwall, LKML, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, X86 ML,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk


[-- Attachment #1.1.1: Type: text/plain, Size: 882 bytes --]

On 09.06.23 17:27, Konrad Rzeszutek Wilk wrote:
> Usually I put it in my tree (ibft) but since it is so simple and the user is Xen 
> it would make more sense to do it via the Xen tree (Juergen).

Works for me.


Juergen

> 
> Thx
> 
> On Fri, Jun 9, 2023, 11:16 AM Dave Hansen <dave.hansen@intel.com 
> <mailto:dave.hansen@intel.com>> wrote:
> 
>     On 6/5/23 03:28, Ross Lagerwall wrote:
>      > The result of these changes is that it is possible to boot a diskless
>      > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
>      > find the iBFT and consequently, the iSCSI root disk.
> 
>     Acked-by: Dave Hansen <dave.hansen@linux.intel.com
>     <mailto:dave.hansen@linux.intel.com>> # for x86
> 
>     The work in this patch seems pretty evenly split between x86 and iSCSI.
>     Any preferences on who picks it up?
> 


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
  2023-06-09 15:22   ` Konrad Rzeszutek Wilk
@ 2023-06-09 15:32     ` Dave Hansen
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Hansen @ 2023-06-09 15:32 UTC (permalink / raw)
  To: konrad, Juergen Gross
  Cc: Ross Lagerwall, LKML, xen-devel, Jan Beulich, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, X86 ML,
	Boris Ostrovsky, Peter Jones, Konrad Rzeszutek Wilk

On 6/9/23 08:22, Konrad Rzeszutek Wilk wrote:
> Dave, are you OK with the change in where the reserve call is made?

The move makes logical sense.  I'm not 100% confident it won't regress
anything, but the blast radius should be limited to iSCSI.  But, yeah,
I'm OK with it.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-09 15:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-05 10:28 [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 Ross Lagerwall
2023-06-08 16:38 ` Konrad Rzeszutek Wilk
2023-06-09 14:25   ` Ross Lagerwall
2023-06-09 15:08 ` Juergen Gross
2023-06-09 15:22   ` Konrad Rzeszutek Wilk
2023-06-09 15:32     ` Dave Hansen
2023-06-09 15:16 ` Dave Hansen
2023-06-09 15:27   ` Konrad Rzeszutek Wilk
2023-06-09 15:30     ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).