[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM6PR03MB5372A47C99EDAAC251CF4660F051A@DM6PR03MB5372.namprd03.prod.outlook.com>
Date: Fri, 9 Jun 2023 14:25:46 +0000
From: Ross Lagerwall <ross.lagerwall@...rix.com>
To: "konrad@...nok.org" <konrad@...nok.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
Jan Beulich <jbeulich@...e.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, Juergen Gross <jgross@...e.com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Peter Jones <pjones@...hat.com>,
Konrad Rzeszutek Wilk <konrad@...nel.org>
Subject: Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0
> From: Xen-devel <xen-devel-bounces@...ts.xenproject.org> on behalf of Konrad Rzeszutek Wilk <konrad@...nok.org>
> Sent: Thursday, June 8, 2023 5:38 PM
> To: Ross Lagerwall <ross.lagerwall@...rix.com>
> Cc: linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>; xen-devel@...ts.xenproject.org <xen-devel@...ts.xenproject.org>; Jan Beulich <jbeulich@...e.com>; Thomas Gleixner <tglx@...utronix.de>; Ingo Molnar <mingo@...hat.com>; Borislav Petkov <bp@...en8.de>; Dave Hansen <dave.hansen@...ux.intel.com>; x86@...nel.org <x86@...nel.org>; Juergen Gross <jgross@...e.com>; Boris Ostrovsky <boris.ostrovsky@...cle.com>; Peter Jones <pjones@...hat.com>; Konrad Rzeszutek Wilk <konrad@...nel.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@...rix.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@...rix.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
> >
>
Powered by blists - more mailing lists