Re: [PATCH] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-05-30 Thread Ross Lagerwall
> From: Jan Beulich 
> Sent: Thursday, May 25, 2023 10:31 AM
> To: Ross Lagerwall 
> Cc: Thomas Gleixner ; Ingo Molnar ; 
> Borislav Petkov ; Dave Hansen ; 
> x...@kernel.org ; Juergen Gross ; Boris 
> Ostrovsky ; Peter Jones ; 
> Konrad Rzeszutek Wilk ; linux-ker...@vger.kernel.org 
> ; xen-devel@lists.xenproject.org 
> 
> Subject: Re: [PATCH] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 
>  
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the sender and know the content is safe.
> 
> On 24.05.2023 18:05, Ross Lagerwall wrote:
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -772,8 +772,14 @@ char * __init xen_memory_setup(void)
> > * UNUSABLE regions in domUs are not handled and will need
> > * a patch in the future.
> > */
> 
> I think this comment now wants to move ...
> 
> > - if (xen_initial_domain())
> > + if (xen_initial_domain()) {
> 
> ... here. And then likely you want a blank line ...
> 
> >    xen_ignore_unusable();
> 
> ... here.

OK

> 
> > + /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
> > + xen_e820_table.entries[xen_e820_table.nr_entries].addr = 
> > 0x8;
> > + xen_e820_table.entries[xen_e820_table.nr_entries].size = 
> > 0x8;
> > + xen_e820_table.entries[xen_e820_table.nr_entries].type = 
> > E820_TYPE_RESERVED;
> > + xen_e820_table.nr_entries++;
> 
> Surely this can be omitted when !CONFIG_ISCSI_IBFT_FIND?
> 

Yes, good point. I will fix that.

Thanks,
Ross


Re: [PATCH] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-05-25 Thread Jan Beulich
On 24.05.2023 18:05, Ross Lagerwall wrote:
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -772,8 +772,14 @@ char * __init xen_memory_setup(void)
>* UNUSABLE regions in domUs are not handled and will need
>* a patch in the future.
>*/

I think this comment now wants to move ...

> - if (xen_initial_domain())
> + if (xen_initial_domain()) {

... here. And then likely you want a blank line ...

>   xen_ignore_unusable();

... here.

> + /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
> + xen_e820_table.entries[xen_e820_table.nr_entries].addr = 
> 0x8;
> + xen_e820_table.entries[xen_e820_table.nr_entries].size = 
> 0x8;
> + xen_e820_table.entries[xen_e820_table.nr_entries].type = 
> E820_TYPE_RESERVED;
> + xen_e820_table.nr_entries++;

Surely this can be omitted when !CONFIG_ISCSI_IBFT_FIND?

Jan



[PATCH] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-05-24 Thread Ross Lagerwall
Since firmware doesn't indicate the iBFT in the E820, add a reserved
region so that it gets identity mapped when running as Dom 0 so that it
is possible to search for it. Move the call to reserve_ibft_region()
later so that it is called after the Xen identity mapping adjustments
are applied.

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

Signed-off-by: Ross Lagerwall 
---

It tested this to work under Xen and native Linux.

 arch/x86/kernel/setup.c|  2 +-
 arch/x86/xen/setup.c   |  8 +++-
 drivers/firmware/iscsi_ibft_find.c | 24 +---
 3 files changed, 25 insertions(+), 9 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..daab59df3b99 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -772,8 +772,14 @@ char * __init xen_memory_setup(void)
 * UNUSABLE regions in domUs are not handled and will need
 * a patch in the future.
 */
-   if (xen_initial_domain())
+   if (xen_initial_domain()) {
xen_ignore_unusable();
+   /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
+   xen_e820_table.entries[xen_e820_table.nr_entries].addr = 
0x8;
+   xen_e820_table.entries[xen_e820_table.nr_entries].size = 
0x8;
+   xen_e820_table.entries[xen_e820_table.nr_entries].type = 
E820_TYPE_RESERVED;
+   xen_e820_table.nr_entries++;
+   }
 
/* Make sure the Xen-supplied memory map is well-ordered. */
e820__update_table(_e820_table);
diff --git a/drivers/firmware/iscsi_ibft_find.c 
b/drivers/firmware/iscsi_ibft_find.c
index 94b49ccd23ac..e3c1449987dd 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -52,9 +52,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 +70,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 +91,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", 
_phys_addr);
-   return;
+   goto out;
}
}
}
}
+
+out:
+   early_memunmap(virt, PAGE_SIZE);
 }
-- 
2.31.1