Area at the 0x0FFFD9 physical location in the PC memory space is regular 
memory, traditionally ROM BIOS and more recently a copy of BIOS code and 
data in RAM, write-protected.

Use memremap() then to get access to it rather than ioremap(), avoiding 
issues in virtualization scenarios and complementing changes such as 
commit f7750a795687 ("x86, mpparse, x86/acpi, x86/PCI, x86/dmi, SFI: Use 
memremap() for RAM mappings") or commit 5997efb96756 ("x86/boot: Use 
memremap() to map the MPF and MPC data").

Reported-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Closes: https://lore.kernel.org/r/20240822095122.736522-1-kirill.shutemov@linux.intel.com
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
Hi,

 It's not clear to me if pieces added with commit 6a92b11169a6 ("x86/EISA: 
Don't probe EISA bus for Xen PV guests") are still needed with this change 
in place; it's not my area of experience and the submitter of said commit 
clearly didn't realise this is really an access to regular memory rather 
than MMIO.  If they are not needed, they can be discarded with a follow-up 
change.

 I have verified this change with my plain EISA i486 box, and just to be 
sure with a debug patch to report that `EISA_bus' (hardly used nowadays) 
has indeed been set.  Please apply.

  Maciej
---
 arch/x86/kernel/eisa.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

linux-x86-eisa-bus-probe-memremap.diff
Index: linux-macro/arch/x86/kernel/eisa.c
===================================================================
--- linux-macro.orig/arch/x86/kernel/eisa.c
+++ linux-macro/arch/x86/kernel/eisa.c
@@ -11,15 +11,15 @@
 
 static __init int eisa_bus_probe(void)
 {
-	void __iomem *p;
+	void *p;
 
 	if ((xen_pv_domain() && !xen_initial_domain()) || cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
 		return 0;
 
-	p = ioremap(0x0FFFD9, 4);
+	p = memremap(0x0FFFD9, 4, MEMREMAP_WB);
 	if (p && readl(p) == 'E' + ('I' << 8) + ('S' << 16) + ('A' << 24))
 		EISA_bus = 1;
-	iounmap(p);
+	memunmap(p);
 	return 0;
 }
 subsys_initcall(eisa_bus_probe);

On Sat, Aug 24, 2024 at 11:17:10PM +0100, Maciej W. Rozycki wrote:
> -	void __iomem *p;
> +	void *p;
>  
>  	if ((xen_pv_domain() && !xen_initial_domain()) || cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>  		return 0;
>  
> -	p = ioremap(0x0FFFD9, 4);
> +	p = memremap(0x0FFFD9, 4, MEMREMAP_WB);
>  	if (p && readl(p) == 'E' + ('I' << 8) + ('S' << 16) + ('A' << 24))
>  		EISA_bus = 1;

readl requires and __iomem pointer.  If this is just a memory region you
can and should directly dereference the address instead.

Note that sparse will complain about the above as well.

On Tue, Aug 27, 2024 at 11:14:45PM -0700, Christoph Hellwig wrote:
> On Sat, Aug 24, 2024 at 11:17:10PM +0100, Maciej W. Rozycki wrote:
> > -	void __iomem *p;
> > +	void *p;
> >  
> >  	if ((xen_pv_domain() && !xen_initial_domain()) || cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> >  		return 0;
> >  
> > -	p = ioremap(0x0FFFD9, 4);
> > +	p = memremap(0x0FFFD9, 4, MEMREMAP_WB);
> >  	if (p && readl(p) == 'E' + ('I' << 8) + ('S' << 16) + ('A' << 24))
> >  		EISA_bus = 1;
> 
> readl requires and __iomem pointer.  If this is just a memory region you
> can and should directly dereference the address instead.
> 
> Note that sparse will complain about the above as well.
 
See v2:

https://lore.kernel.org/all/alpine.DEB.2.21.2408261015270.30766@angie.orcam.me.uk


-- 
  Kiryl Shutsemau / Kirill A. Shutemov

The following commit has been merged into the x86/cleanups branch of tip:

Commit-ID:     80a4da05642c384bc6f5b602b865ebd7e3963902
Gitweb:        https://git.kernel.org/tip/80a4da05642c384bc6f5b602b865ebd7e3963902
Author:        Maciej W. Rozycki <macro@orcam.me.uk>
AuthorDate:    Sat, 24 Aug 2024 23:17:10 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sun, 25 Aug 2024 14:29:38 +02:00

x86/EISA: Use memremap() to probe for the EISA BIOS signature

The area at the 0x0FFFD9 physical location in the PC memory space is
regular memory, traditionally ROM BIOS and more recently a copy of BIOS
code and data in RAM, write-protected.

Therefore use memremap() to get access to it rather than ioremap(),
avoiding issues in virtualization scenarios and complementing changes such
as commit f7750a795687 ("x86, mpparse, x86/acpi, x86/PCI, x86/dmi, SFI: Use
memremap() for RAM mappings") or commit 5997efb96756 ("x86/boot: Use
memremap() to map the MPF and MPC data").

Reported-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/alpine.DEB.2.21.2408242025210.30766@angie.orcam.me.uk
Closes: https://lore.kernel.org/r/20240822095122.736522-1-kirill.shutemov@linux.intel.com
---
 arch/x86/kernel/eisa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/eisa.c b/arch/x86/kernel/eisa.c
index 53935b4..5a4f99a 100644
--- a/arch/x86/kernel/eisa.c
+++ b/arch/x86/kernel/eisa.c
@@ -11,15 +11,15 @@
 
 static __init int eisa_bus_probe(void)
 {
-	void __iomem *p;
+	void *p;
 
 	if ((xen_pv_domain() && !xen_initial_domain()) || cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
 		return 0;
 
-	p = ioremap(0x0FFFD9, 4);
+	p = memremap(0x0FFFD9, 4, MEMREMAP_WB);
 	if (p && readl(p) == 'E' + ('I' << 8) + ('S' << 16) + ('A' << 24))
 		EISA_bus = 1;
-	iounmap(p);
+	memunmap(p);
 	return 0;
 }
 subsys_initcall(eisa_bus_probe);