[PATCH 0/4] Introduce and use absolute_pointer macro

Kernel test builds currently fail for several architectures with error
messages such as the following.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
./arch/m68k/include/asm/string.h:72:25: error:
	'__builtin_memcpy' reading 6 bytes from a region of size 0
		[-Werror=stringop-overread]

Such warnings may be reported by gcc 11.x for string and memory operations
on fixed addresses.

This patch series introduces absolute_pointer() to fix the problem.
absolute_pointer() disassociates a pointer from its originating symbol
type and context, and thus prevents gcc from making assumptions about
pointers passed to memory operations.

----------------------------------------------------------------
Guenter Roeck (4):
      compiler.h: Introduce absolute_pointer macro
      net: i825xx: Use absolute_pointer for memcpy on fixed memory location
      parisc: Use absolute_pointer for memcmp on fixed memory location
      alpha: Use absolute_pointer for strcmp on fixed memory location

 arch/alpha/kernel/setup.c           | 2 +-
 arch/parisc/kernel/setup.c          | 2 +-
 drivers/net/ethernet/i825xx/82596.c | 2 +-
 include/linux/compiler.h            | 2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)

absolute_pointer() disassociates a pointer from its originating symbol
type and context. Use it to prevent compiler warnings/errors such as

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
./arch/m68k/include/asm/string.h:72:25: error:
	'__builtin_memcpy' reading 6 bytes from a region of size 0
		[-Werror=stringop-overread]

Such warnings may be reported by gcc 11.x for string and memory operations
on fixed addresses.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/compiler.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b67261a1e3e9..3d5af56337bd 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h

@@ -188,6 +188,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
     (typeof(ptr)) (__ptr + (off)); })
 #endif
 
+#define absolute_pointer(val)	RELOC_HIDE((void *)(val), 0)
+
 #ifndef OPTIMIZER_HIDE_VAR
 /* Make the optimizer believe the variable can be manipulated arbitrarily. */
 #define OPTIMIZER_HIDE_VAR(var)						\

-- 
2.33.0

gcc 11.x reports the following compiler warning/error.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
    ./arch/m68k/include/asm/string.h:72:25: error:
            '__builtin_memcpy' reading 6 bytes from a region of size 0
                    [-Werror=stringop-overread]

Use absolute_address() to work around the problem.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/net/ethernet/i825xx/82596.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/i825xx/82596.c b/drivers/net/ethernet/i825xx/82596.c
index b8a40146b895..b482f6f633bd 100644
--- a/drivers/net/ethernet/i825xx/82596.c
+++ b/drivers/net/ethernet/i825xx/82596.c

@@ -1144,7 +1144,7 @@ static struct net_device * __init i82596_probe(void)
 			err = -ENODEV;
 			goto out;
 		}
-		memcpy(eth_addr, (void *) 0xfffc1f2c, ETH_ALEN);	/* YUCK! Get addr from NOVRAM */
+		memcpy(eth_addr, absolute_pointer(0xfffc1f2c), ETH_ALEN); /* YUCK! Get addr from NOVRAM */
 		dev->base_addr = MVME_I596_BASE;
 		dev->irq = (unsigned) MVME16x_IRQ_I596;
 		goto found;

-- 
2.33.0

On Sun, 12 Sep 2021 09:01:47 -0700
Guenter Roeck [off-list ref] wrote:

gcc 11.x reports the following compiler warning/error.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
    ./arch/m68k/include/asm/string.h:72:25: error:
            '__builtin_memcpy' reading 6 bytes from a region of size 0
                    [-Werror=stringop-overread]

Use absolute_address() to work around the problem.

=> absolute_pointer()

Regards,
      jer

On 9/12/21 9:11 AM, Jeroen Roovers wrote:

On Sun, 12 Sep 2021 09:01:47 -0700
Guenter Roeck [off-list ref] wrote:

quoted

gcc 11.x reports the following compiler warning/error.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
     ./arch/m68k/include/asm/string.h:72:25: error:
             '__builtin_memcpy' reading 6 bytes from a region of size 0
                     [-Werror=stringop-overread]

Use absolute_address() to work around the problem.

=> absolute_pointer()

Oopsie. Thanks!

Guenter

quoted

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
  drivers/net/ethernet/i825xx/82596.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/i825xx/82596.c

b/drivers/net/ethernet/i825xx/82596.c index
b8a40146b895..b482f6f633bd 100644 ---
a/drivers/net/ethernet/i825xx/82596.c +++
b/drivers/net/ethernet/i825xx/82596.c @@ -1144,7 +1144,7 @@ static
struct net_device * __init i82596_probe(void) err = -ENODEV;
  			goto out;
  		}
-		memcpy(eth_addr, (void *) 0xfffc1f2c,
ETH_ALEN);	/* YUCK! Get addr from NOVRAM */
+		memcpy(eth_addr, absolute_pointer(0xfffc1f2c),
ETH_ALEN); /* YUCK! Get addr from NOVRAM */ dev->base_addr =
MVME_I596_BASE; dev->irq = (unsigned) MVME16x_IRQ_I596;
  		goto found;

Regards,
       jer

parisc:allmodconfig fails to build with the following error
when using gcc 11.x.

arch/parisc/kernel/setup.c: In function 'start_parisc':
arch/parisc/kernel/setup.c:389:28: error:
	'__builtin_memcmp_eq' specified bound 8 exceeds source size 0

Avoid the problem by using absolute_pointer() when providing a memory
address to memcmp().

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/parisc/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c
index cceb09855e03..4e13345b6581 100644
--- a/arch/parisc/kernel/setup.c
+++ b/arch/parisc/kernel/setup.c

@@ -384,7 +384,7 @@ void __init start_parisc(void)
 	struct pdc_coproc_cfg coproc_cfg;
 
 	/* check QEMU/SeaBIOS marker in PAGE0 */
-	running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);
+	running_on_qemu = (memcmp(absolute_pointer(&PAGE0->pad0), "SeaBIOS", 8) == 0);
 
 	cpunum = smp_processor_id();
 

-- 
2.33.0

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

-       running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);
+       running_on_qemu = (memcmp(absolute_pointer(&PAGE0->pad0), "SeaBIOS", 8) == 0);

This seems entirely the wrong thing to do, and makes no sense. That
"&PAGE0->pad0" is a perfectly valid pointer, and that's not where the
problem is.

The problem is "PAGE0" itself:

    #define PAGE0   ((struct zeropage *)__PAGE_OFFSET)

which takes that absolute offset and creates a pointer out of it.

IOW, _that_ is what should have the "absolute_pointer()" thing, and in
that context the name of that macro and its use actually makes sense.

No?

An alternative - and possibly cleaner - approach that doesn't need
absolute_pointer() at all might be to just do

        extern struct zeropage PAGE0;

and then make that PAGE0 be defined to __PAGE_OFFSET in the parisc
vmlinux.lds.S file.

Then doing things like

        running_on_qemu = !memcmp(&PAGE0.pad0, "SeaBIOS", 8);

would JustWork(tm).

Hmm?

             Linus

* Linus Torvalds [off-list ref]:

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

quoted

-       running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);
+       running_on_qemu = (memcmp(absolute_pointer(&PAGE0->pad0), "SeaBIOS", 8) == 0);

This seems entirely the wrong thing to do, and makes no sense. That
"&PAGE0->pad0" is a perfectly valid pointer, and that's not where the
problem is.

The problem is "PAGE0" itself:

    #define PAGE0   ((struct zeropage *)__PAGE_OFFSET)

which takes that absolute offset and creates a pointer out of it.

IOW, _that_ is what should have the "absolute_pointer()" thing, and in
that context the name of that macro and its use actually makes sense.

No?

An alternative - and possibly cleaner - approach that doesn't need
absolute_pointer() at all might be to just do

        extern struct zeropage PAGE0;

and then make that PAGE0 be defined to __PAGE_OFFSET in the parisc
vmlinux.lds.S file.

Then doing things like

        running_on_qemu = !memcmp(&PAGE0.pad0, "SeaBIOS", 8);

would JustWork(tm).

Yes, this second approach seems to work nicely, although the patch
then gets slightly bigger.
Below is a tested patch.
I'll check it some further and apply it to the parisc tree then.

Thanks!
Helge

--------

[PATCH] Define and export PAGE0 in vmlinux.lds.S linker script

Signed-off-by: Helge Deller <deller@gmx.de>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>

---

diff --git a/arch/parisc/boot/compressed/misc.c b/arch/parisc/boot/compressed/misc.c
index 7ee49f5881d1..1b957c6bbe5c 100644
--- a/arch/parisc/boot/compressed/misc.c
+++ b/arch/parisc/boot/compressed/misc.c

@@ -319,7 +319,7 @@ unsigned long decompress_kernel(unsigned int started_wide,

 	/* Limit memory for bootoader to 1GB */
 	#define ARTIFICIAL_LIMIT (1*1024*1024*1024)
-	free_mem_end_ptr = PAGE0->imm_max_mem;
+	free_mem_end_ptr = PAGE0.imm_max_mem;
 	if (free_mem_end_ptr > ARTIFICIAL_LIMIT)
 		free_mem_end_ptr = ARTIFICIAL_LIMIT;

diff --git a/arch/parisc/boot/compressed/vmlinux.lds.S b/arch/parisc/boot/compressed/vmlinux.lds.S
index ab7b43990857..83a1f8f67aba 100644
--- a/arch/parisc/boot/compressed/vmlinux.lds.S
+++ b/arch/parisc/boot/compressed/vmlinux.lds.S

@@ -14,6 +14,9 @@ ENTRY(startup)

 SECTIONS
 {
+        . = __PAGE_OFFSET;
+        PAGE0 = .;
+
 	/* palo loads at 0x60000 */
 	/* loaded kernel will move to 0x10000 */
 	. = 0xe0000;    /* should not overwrite palo code */

diff --git a/arch/parisc/include/asm/page.h b/arch/parisc/include/asm/page.h
index d00313d1274e..7b64c05abd0c 100644
--- a/arch/parisc/include/asm/page.h
+++ b/arch/parisc/include/asm/page.h

@@ -100,6 +100,9 @@ typedef struct __physmem_range {
 extern physmem_range_t pmem_ranges[];
 extern int npmem_ranges;

+/* Address of PAGE0 is defined in vmlinux.lds.S */
+extern struct zeropage PAGE0;
+
 #endif /* !__ASSEMBLY__ */

 /* WARNING: The definitions below must match exactly to sizeof(pte_t)

@@ -184,8 +187,6 @@ extern int npmem_ranges;
 #include <asm-generic/getorder.h>
 #include <asm/pdc.h>

-#define PAGE0   ((struct zeropage *)__PAGE_OFFSET)
-
 /* DEFINITION OF THE ZERO-PAGE (PAG0) */
 /* based on work by Jason Eckhardt (jason@equator.com) */

diff --git a/arch/parisc/kernel/drivers.c b/arch/parisc/kernel/drivers.c
index 776d624a7207..2e8b6e530c09 100644
--- a/arch/parisc/kernel/drivers.c
+++ b/arch/parisc/kernel/drivers.c

@@ -944,9 +944,9 @@ static __init void qemu_header(void)

 	pr_info("#define PARISC_PDC_ENTRY_ORG 0x%04lx\n\n",
 #ifdef CONFIG_64BIT
-		(unsigned long)(PAGE0->mem_pdc_hi) << 32 |
+		(unsigned long)(PAGE0.mem_pdc_hi) << 32 |
 #endif
-		(unsigned long)PAGE0->mem_pdc);
+		(unsigned long)PAGE0.mem_pdc);

 	pr_info("#define PARISC_PDC_CACHE_INFO");
 	p = (unsigned long *) &cache_info;

diff --git a/arch/parisc/kernel/firmware.c b/arch/parisc/kernel/firmware.c
index 7034227dbdf3..17516de2f191 100644
--- a/arch/parisc/kernel/firmware.c
+++ b/arch/parisc/kernel/firmware.c

@@ -103,10 +103,10 @@ long real64_call(unsigned long function, ...);
 long real32_call(unsigned long function, ...);

 #ifdef CONFIG_64BIT
-#   define MEM_PDC (unsigned long)(PAGE0->mem_pdc_hi) << 32 | PAGE0->mem_pdc
+#   define MEM_PDC (unsigned long)(PAGE0.mem_pdc_hi) << 32 | PAGE0.mem_pdc
 #   define mem_pdc_call(args...) unlikely(parisc_narrow_firmware) ? real32_call(MEM_PDC, args) : real64_call(MEM_PDC, args)
 #else
-#   define MEM_PDC (unsigned long)PAGE0->mem_pdc
+#   define MEM_PDC (unsigned long)PAGE0.mem_pdc
 #   define mem_pdc_call(args...) real32_call(MEM_PDC, args)
 #endif

@@ -1249,9 +1249,9 @@ int pdc_iodc_print(const unsigned char *str, unsigned count)

 print:
         spin_lock_irqsave(&pdc_lock, flags);
-        real32_call(PAGE0->mem_cons.iodc_io,
-                    (unsigned long)PAGE0->mem_cons.hpa, ENTRY_IO_COUT,
-                    PAGE0->mem_cons.spa, __pa(PAGE0->mem_cons.dp.layers),
+        real32_call(PAGE0.mem_cons.iodc_io,
+                    (unsigned long)PAGE0.mem_cons.hpa, ENTRY_IO_COUT,
+                    PAGE0.mem_cons.spa, __pa(PAGE0.mem_cons.dp.layers),
                     __pa(iodc_retbuf), 0, __pa(iodc_dbuf), i, 0);
         spin_unlock_irqrestore(&pdc_lock, flags);

@@ -1272,14 +1272,14 @@ int pdc_iodc_getc(void)
 	unsigned long flags;

 	/* Bail if no console input device. */
-	if (!PAGE0->mem_kbd.iodc_io)
+	if (!PAGE0.mem_kbd.iodc_io)
 		return 0;

 	/* wait for a keyboard (rs232)-input */
 	spin_lock_irqsave(&pdc_lock, flags);
-	real32_call(PAGE0->mem_kbd.iodc_io,
-		    (unsigned long)PAGE0->mem_kbd.hpa, ENTRY_IO_CIN,
-		    PAGE0->mem_kbd.spa, __pa(PAGE0->mem_kbd.dp.layers),
+	real32_call(PAGE0.mem_kbd.iodc_io,
+		    (unsigned long)PAGE0.mem_kbd.hpa, ENTRY_IO_CIN,
+		    PAGE0.mem_kbd.spa, __pa(PAGE0.mem_kbd.dp.layers),
 		    __pa(iodc_retbuf), 0, __pa(iodc_dbuf), 1, 0);

 	ch = *iodc_dbuf;

diff --git a/arch/parisc/kernel/inventory.c b/arch/parisc/kernel/inventory.c
index 7ab2f2a54400..13234c663e4b 100644
--- a/arch/parisc/kernel/inventory.c
+++ b/arch/parisc/kernel/inventory.c

@@ -164,7 +164,7 @@ static void __init pagezero_memconfig(void)
 	 * should be done.
 	 */

-	npages = (PAGE_ALIGN(PAGE0->imm_max_mem) >> PAGE_SHIFT);
+	npages = (PAGE_ALIGN(PAGE0.imm_max_mem) >> PAGE_SHIFT);
 	set_pmem_entry(pmem_ranges,0UL,npages);
 	npmem_ranges = 1;
 }

@@ -648,8 +648,8 @@ void __init do_device_inventory(void)
 		struct resource res[3] = {0,};
 		unsigned int base;

-		base = ((unsigned long long) PAGE0->pad0[2] << 32)
-			| PAGE0->pad0[3]; /* SeaBIOS stored it here */
+		base = ((unsigned long long) PAGE0.pad0[2] << 32)
+			| PAGE0.pad0[3]; /* SeaBIOS stored it here */

 		res[0].name = "fw_cfg";
 		res[0].start = base;

diff --git a/arch/parisc/kernel/kexec.c b/arch/parisc/kernel/kexec.c
index 5eb7f30edc1f..6b39b81a96da 100644
--- a/arch/parisc/kernel/kexec.c
+++ b/arch/parisc/kernel/kexec.c

@@ -96,7 +96,7 @@ void machine_kexec(struct kimage *image)
 	*(unsigned long *)(virt + kexec_cmdline_offset) = arch->cmdline;
 	*(unsigned long *)(virt + kexec_initrd_start_offset) = arch->initrd_start;
 	*(unsigned long *)(virt + kexec_initrd_end_offset) = arch->initrd_end;
-	*(unsigned long *)(virt + kexec_free_mem_offset) = PAGE0->mem_free;
+	*(unsigned long *)(virt + kexec_free_mem_offset) = PAGE0.mem_free;

 	flush_cache_all();
 	flush_tlb_all();

diff --git a/arch/parisc/kernel/kexec_file.c b/arch/parisc/kernel/kexec_file.c
index 8c534204f0fd..619aeebc5800 100644
--- a/arch/parisc/kernel/kexec_file.c
+++ b/arch/parisc/kernel/kexec_file.c

@@ -61,7 +61,7 @@ static void *elf_load(struct kimage *image, char *kernel_buf,
 		kbuf.bufsz = kbuf.memsz = ALIGN(cmdline_len, 8);
 		kbuf.buf_align = PAGE_SIZE;
 		kbuf.top_down = false;
-		kbuf.buf_min = PAGE0->mem_free + PAGE_SIZE;
+		kbuf.buf_min = PAGE0.mem_free + PAGE_SIZE;
 		kbuf.buf_max = kernel_load_addr;
 		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 		ret = kexec_add_buffer(&kbuf);

diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c
index 2661cdd256ae..b635294184b9 100644
--- a/arch/parisc/kernel/pdc_cons.c
+++ b/arch/parisc/kernel/pdc_cons.c

@@ -218,8 +218,8 @@ static void pdc_console_init_force(void)
 	++pdc_console_initialized;

 	/* If the console is duplex then copy the COUT parameters to CIN. */
-	if (PAGE0->mem_cons.cl_class == CL_DUPLEX)
-		memcpy(&PAGE0->mem_kbd, &PAGE0->mem_cons, sizeof(PAGE0->mem_cons));
+	if (PAGE0.mem_cons.cl_class == CL_DUPLEX)
+		memcpy(&PAGE0.mem_kbd, &PAGE0.mem_cons, sizeof(PAGE0.mem_cons));

 	/* register the pdc console */
 	register_console(&pdc_cons);

diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
index 1b6129e7d776..7872322336a5 100644
--- a/arch/parisc/kernel/processor.c
+++ b/arch/parisc/kernel/processor.c

@@ -234,7 +234,7 @@ void __init collect_boot_cpu_data(void)
 	cr16_seed = get_cycles();
 	add_device_randomness(&cr16_seed, sizeof(cr16_seed));

-	boot_cpu_data.cpu_hz = 100 * PAGE0->mem_10msec; /* Hz of this PARISC */
+	boot_cpu_data.cpu_hz = 100 * PAGE0.mem_10msec; /* Hz of this PARISC */

 	/* get CPU-Model Information... */
 #define p ((unsigned long *)&boot_cpu_data.pdc.model)

diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c
index cceb09855e03..0168f7f83fdb 100644
--- a/arch/parisc/kernel/setup.c
+++ b/arch/parisc/kernel/setup.c

@@ -384,7 +384,7 @@ void __init start_parisc(void)
 	struct pdc_coproc_cfg coproc_cfg;

 	/* check QEMU/SeaBIOS marker in PAGE0 */
-	running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);
+	running_on_qemu = (memcmp(&PAGE0.pad0, "SeaBIOS", 8) == 0);

 	cpunum = smp_processor_id();

diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 9fb1e794831b..00283e41fc7f 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c

@@ -237,10 +237,10 @@ void __init time_init(void)
 {
 	unsigned long cr16_hz;

-	clocktick = (100 * PAGE0->mem_10msec) / HZ;
+	clocktick = (100 * PAGE0.mem_10msec) / HZ;
 	start_cpu_itimer();	/* get CPU 0 started */

-	cr16_hz = 100 * PAGE0->mem_10msec;  /* Hz */
+	cr16_hz = 100 * PAGE0.mem_10msec;  /* Hz */

 	/* register as sched_clock source */
 	sched_clock_register(read_cr16_sched_clock, BITS_PER_LONG, cr16_hz);

@@ -277,7 +277,7 @@ static int __init init_cr16_clocksource(void)

 	/* register at clocksource framework */
 	clocksource_register_hz(&clocksource_cr16,
-		100 * PAGE0->mem_10msec);
+		100 * PAGE0.mem_10msec);

 	return 0;
 }

diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S
index 2769eb991f58..87abee841182 100644
--- a/arch/parisc/kernel/vmlinux.lds.S
+++ b/arch/parisc/kernel/vmlinux.lds.S

@@ -55,6 +55,9 @@ jiffies = jiffies_64;
 #endif
 SECTIONS
 {
+        . = __PAGE_OFFSET;
+        PAGE0 = .;
+
 	. = KERNEL_BINARY_TEXT_START;

 	__init_begin = .;

diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index 3f7d6d5b56ac..733961eb25a1 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c

@@ -283,7 +283,7 @@ static void __init setup_bootmem(void)

 #define PDC_CONSOLE_IO_IODC_SIZE 32768

-	memblock_reserve(0UL, (unsigned long)(PAGE0->mem_free +
+	memblock_reserve(0UL, (unsigned long)(PAGE0.mem_free +
 				PDC_CONSOLE_IO_IODC_SIZE));
 	memblock_reserve(__pa(KERNEL_BINARY_TEXT_START),
 			(unsigned long)(_end - KERNEL_BINARY_TEXT_START));

diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index e60690d38d67..9d4ccb8c75bd 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c

@@ -1542,7 +1542,7 @@ static void sba_hw_init(struct sba_device *sba_dev)
 		**	o reprogram serial port
 		**	o unblock console output
 		*/
-		if (PAGE0->mem_kbd.cl_class == CL_KEYBD) {
+		if (PAGE0.mem_kbd.cl_class == CL_KEYBD) {
 			pdc_io_reset_devices();
 		}

@@ -1550,8 +1550,8 @@ static void sba_hw_init(struct sba_device *sba_dev)


 #if 0
-printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n", PAGE0->mem_boot.hpa,
-	PAGE0->mem_boot.spa, PAGE0->mem_boot.pad, PAGE0->mem_boot.cl_class);
+printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n", PAGE0.mem_boot.hpa,
+	PAGE0.mem_boot.spa, PAGE0.mem_boot.pad, PAGE0.mem_boot.cl_class);

 	/*
 	** Need to deal with DMA from LAN.

@@ -1562,8 +1562,8 @@ printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n", PAGE0->mem_boot.hpa,
 	** 	mem_boot hpa 0xf4008000 sba 0x0 pad 0x0 cl_class 0x1002
 	** ARGH! invalid class.
 	*/
-	if ((PAGE0->mem_boot.cl_class != CL_RANDOM)
-		&& (PAGE0->mem_boot.cl_class != CL_SEQU)) {
+	if ((PAGE0.mem_boot.cl_class != CL_RANDOM)
+		&& (PAGE0.mem_boot.cl_class != CL_SEQU)) {
 			pdc_io_reset();
 	}
 #endif

diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 1b451165311c..d5737d9fa8cf 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c

@@ -395,7 +395,7 @@ static int __init sticonsole_init(void)
     pr_info("sticon: Initializing STI text console.\n");
     console_lock();
     err = do_take_over_console(&sti_con, 0, MAX_NR_CONSOLES - 1,
-		PAGE0->mem_cons.cl_class != CL_DUPLEX);
+		PAGE0.mem_cons.cl_class != CL_DUPLEX);
     console_unlock();

     return err;

diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c
index f869b723494f..a6a6b5503ca1 100644
--- a/drivers/video/console/sticore.c
+++ b/drivers/video/console/sticore.c

@@ -966,7 +966,7 @@ static int __init sticore_pa_init(struct parisc_device *dev)
 	if (!sti)
 		sti = sti_try_rom_generic(hpa, hpa, NULL);
 	if (!sti)
-		sti = sti_try_rom_generic(PAGE0->proc_sti, hpa, NULL);
+		sti = sti_try_rom_generic(PAGE0.proc_sti, hpa, NULL);
 	if (!sti)
 		return 1;

On 9/12/21 12:58 PM, Helge Deller wrote:

* Linus Torvalds [off-list ref]:

quoted

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

quoted

-       running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);
+       running_on_qemu = (memcmp(absolute_pointer(&PAGE0->pad0), "SeaBIOS", 8) == 0);

This seems entirely the wrong thing to do, and makes no sense. That
"&PAGE0->pad0" is a perfectly valid pointer, and that's not where the
problem is.

The problem is "PAGE0" itself:

     #define PAGE0   ((struct zeropage *)__PAGE_OFFSET)

which takes that absolute offset and creates a pointer out of it.

IOW, _that_ is what should have the "absolute_pointer()" thing, and in
that context the name of that macro and its use actually makes sense.

No?

An alternative - and possibly cleaner - approach that doesn't need
absolute_pointer() at all might be to just do

         extern struct zeropage PAGE0;

and then make that PAGE0 be defined to __PAGE_OFFSET in the parisc
vmlinux.lds.S file.

Then doing things like

         running_on_qemu = !memcmp(&PAGE0.pad0, "SeaBIOS", 8);

would JustWork(tm).

Yes, this second approach seems to work nicely, although the patch
then gets slightly bigger.
Below is a tested patch.
I'll check it some further and apply it to the parisc tree then.

There are several PAGE0-> references left in the code after applying your patch.

$ git grep "PAGE0->"
arch/parisc/kernel/firmware.c:  if (!PAGE0->mem_kbd.iodc_io)
arch/parisc/kernel/firmware.c:  real32_call(PAGE0->mem_kbd.iodc_io,
arch/parisc/kernel/firmware.c:              (unsigned long)PAGE0->mem_kbd.hpa, ENTRY_IO_CIN,
arch/parisc/kernel/firmware.c:              PAGE0->mem_kbd.spa, __pa(PAGE0->mem_kbd.dp.layers),
arch/parisc/kernel/smp.c:       WARN_ON(((unsigned long)(PAGE0->mem_pdc_hi) << 32
arch/parisc/kernel/smp.c:                       | PAGE0->mem_pdc) != pdce_proc);

After fixing those, I can build a parisc image and boot it in qemu (32 bit).

Guenter

alpha:allmodconfig fails to build with the following error
when using gcc 11.x.

arch/alpha/kernel/setup.c: In function 'setup_arch':
arch/alpha/kernel/setup.c:493:13: error:
	'strcmp' reading 1 or more bytes from a region of size 0

Avoid the problem by using absolute_pointer() when providing a memory
address to strcmp().

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/alpha/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index b4fbbba30aa2..aab477a76c30 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c

@@ -490,7 +490,7 @@ setup_arch(char **cmdline_p)
 	/* Hack for Jensen... since we're restricted to 8 or 16 chars for
 	   boot flags depending on the boot mode, we need some shorthand.
 	   This should do for installation.  */
-	if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
+	if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {
 		strlcpy(command_line, "root=/dev/fd0 load_ramdisk=1", sizeof command_line);
 	} else {
 		strlcpy(command_line, COMMAND_LINE, sizeof command_line);

-- 
2.33.0

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

-       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
+       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {

Again, this feels very much like treating the symptoms, not actually
fixing the real issue.

It's COMMAND_LINE itself that should have been fixed up, not that one use of it.

Because the only reason you didn't get a warning from later uses is
that 'strcmp()' is doing compiler-specific magic. You're just delaying
the inevitable warnings about the other uses of that thing.

            Linus

On 9/12/21 12:13 PM, Linus Torvalds wrote:

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

quoted

-       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
+       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {

Again, this feels very much like treating the symptoms, not actually
fixing the real issue.

It's COMMAND_LINE itself that should have been fixed up, not that one use of it.

Because the only reason you didn't get a warning from later uses is
that 'strcmp()' is doing compiler-specific magic. You're just delaying
the inevitable warnings about the other uses of that thing.

Makes sense. I'll change the patch accordingly.

Thanks,
Guenter

On 9/12/21 12:13 PM, Linus Torvalds wrote:

On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck [off-list ref] wrote:

quoted

-       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
+       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {

Again, this feels very much like treating the symptoms, not actually
fixing the real issue.

It's COMMAND_LINE itself that should have been fixed up, not that one use of it.

Because the only reason you didn't get a warning from later uses is
that 'strcmp()' is doing compiler-specific magic. You're just delaying
the inevitable warnings about the other uses of that thing.

COMMAND_LINE is, for whatever reason, defined in
arch/alpha/include/uapi/asm/setup.h, ie in the uapi.

Can I even touch that ?

Guenter

On Sun, Sep 12, 2021 at 1:37 PM Guenter Roeck [off-list ref] wrote:

COMMAND_LINE is, for whatever reason, defined in
arch/alpha/include/uapi/asm/setup.h, ie in the uapi.

Can I even touch that ?

I think that's entirely a historical accident.

Note how most of those #define's don't even work for user space
because they use PAGE_OFFSET, which is defined in <asm/page.h>. And
others depend on the kernel config system.

There's a couple that do seem to be potentially for user space (MILO
bootloader), and who knows just what hacks that code might have with
internal knowledge of this header file. But anything I can find on the
net seems to predate our move to 'uapi' headers, so I wouldn't really
worry about it.

So I'd suggest just moving that whole file back to <asm/setup.h>,
changing it as necessary, and then seeing if anybody notices.

Because I suspect the answer to that is just crickets chirping..

              Linus