From: Konrad Rzeszutek Wilk <konrad@darnok.org>
To: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: linux-kernel@vger.kernel.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, 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
Date: Thu, 8 Jun 2023 12:38:11 -0400 [thread overview]
Message-ID: <CAPbh3rsnBgeZJCvzg3Bh_VRxiYEOe0vgQkpyeyDeCBirBLhLiQ@mail.gmail.com> (raw)
In-Reply-To: <20230605102840.1521549-1-ross.lagerwall@citrix.com>
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
>
next prev parent reply other threads:[~2023-06-08 16:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPbh3rsnBgeZJCvzg3Bh_VRxiYEOe0vgQkpyeyDeCBirBLhLiQ@mail.gmail.com \
--to=konrad@darnok.org \
--cc=boris.ostrovsky@oracle.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=jbeulich@suse.com \
--cc=jgross@suse.com \
--cc=konrad@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pjones@redhat.com \
--cc=ross.lagerwall@citrix.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).