[RFC 0/2] mac68k: NuBus fixes

[RFC 0/2] mac68k: NuBus fixes

[Finn Thain]

Geert has agreed to merge these two patches if they receive a favorable
review. Is anyone willing to do a review?


David Huggins-Daines (1):
  nubus: Remove slot zero probe

Finn Thain (1):
  nubus: Fix pointer validation

 drivers/nubus/nubus.c | 125 +++++---------------------------------------------
 1 file changed, 11 insertions(+), 114 deletions(-)

-- 
2.10.2

[RFC 1/2] nubus: Remove slot zero probe

[David Huggins-Daines]

Some long forgotten changes from the linux-mac68k CVS:

Remove the slot 0 (ROM) probing.
Remove the pointless White Screen Of Death crap.

The original commit is here:

http://linux-mac68k.cvs.sourceforge.net/viewvc/linux-mac68k/linux-mac68k/drivers/nubus/nubus.c?r1=1.22.2.1&r2=1.22.2.2&pathrev=linux-2_2

This is mostly dead code removal. It is also a reversion to the
pre-v2.3.17 version. The old code was thoroughly tested in Debian Sarge.

The present code for probing fake slot resources in Apple's on-board
(slot zero) ROMs could be useful if replaced the macintosh_config struct.
But it can't do so and we don't want two mechanisms for identifying
on-board hardware.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/nubus/nubus.c | 111 +++-----------------------------------------------
 1 file changed, 5 insertions(+), 106 deletions(-)

diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index 1742ccc..bb628de 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -13,7 +13,6 @@
 #include <linux/nubus.h>
 #include <linux/errno.h>
 #include <linux/init.h>
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <asm/setup.h>
@@ -34,14 +33,6 @@ extern void oss_nubus_init(void);
 
 #define NUBUS_TEST_PATTERN 0x5A932BC7
 
-/* Define this if you like to live dangerously - it is known not to
-   work on pretty much every machine except the Quadra 630 and the LC
-   III. */
-#undef I_WANT_TO_PROBE_SLOT_ZERO
-
-/* This sometimes helps combat failure to boot */
-#undef TRY_TO_DODGE_WSOD
-
 /* Globals */
 
 struct nubus_dev *nubus_devices;
@@ -454,10 +445,6 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
 	pr_info("  Function 0x%02x:\n", parent->type);
 	nubus_get_subdir(parent, &dir);
 
-	/* Apple seems to have botched the ROM on the IIx */
-	if (slot == 0 && (unsigned long)dir.base % 2)
-		dir.base += 1;
-
 	pr_debug("%s: parent is 0x%p, dir is 0x%p\n",
 	         __func__, parent->base, dir.base);
 
@@ -691,83 +678,6 @@ static int __init nubus_get_board_resource(struct nubus_board *board, int slot,
 	return 0;
 }
 
-/* Attempt to bypass the somewhat non-obvious arrangement of
-   sResources in the motherboard ROM */
-static void __init nubus_find_rom_dir(struct nubus_board* board)
-{
-	unsigned char *rp;
-	unsigned char *romdir;
-	struct nubus_dir dir;
-	struct nubus_dirent ent;
-
-	/* Check for the extra directory just under the format block */
-	rp = board->fblock;
-	nubus_rewind(&rp, 4, board->lanes);
-	if (nubus_get_rom(&rp, 4, board->lanes) != NUBUS_TEST_PATTERN) {
-		/* OK, the ROM was telling the truth */
-		board->directory = board->fblock;
-		nubus_move(&board->directory,
-			   nubus_expand32(board->doffset),
-			   board->lanes);
-		return;
-	}
-
-	/* On "slot zero", you have to walk down a few more
-	   directories to get to the equivalent of a real card's root
-	   directory.  We don't know what they were smoking when they
-	   came up with this. */
-	romdir = nubus_rom_addr(board->slot);
-	nubus_rewind(&romdir, ROM_DIR_OFFSET, board->lanes);
-	dir.base = dir.ptr = romdir;
-	dir.done = 0;
-	dir.mask = board->lanes;
-
-	/* This one points to an "Unknown Macintosh" directory */
-	if (nubus_readdir(&dir, &ent) == -1)
-		goto badrom;
-
-	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
-		printk(KERN_INFO "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
-	/* This one takes us to where we want to go. */
-	if (nubus_readdir(&dir, &ent) == -1)
-		goto badrom;
-	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
-		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
-	nubus_get_subdir(&ent, &dir);
-
-	/* Resource ID 01, also an "Unknown Macintosh" */
-	if (nubus_readdir(&dir, &ent) == -1)
-		goto badrom;
-	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
-		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
-
-	/* FIXME: the first one is *not* always the right one.  We
-	   suspect this has something to do with the ROM revision.
-	   "The HORROR ROM" (LC-series) uses 0x7e, while "The HORROR
-	   Continues" (Q630) uses 0x7b.  The DAFB Macs evidently use
-	   something else.  Please run "Slots" on your Mac (see
-	   include/linux/nubus.h for where to get this program) and
-	   tell us where the 'SiDirPtr' for Slot 0 is.  If you feel
-	   brave, you should also use MacsBug to walk down the ROM
-	   directories like this function does and try to find the
-	   path to that address... */
-	if (nubus_readdir(&dir, &ent) == -1)
-		goto badrom;
-	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
-		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
-
-	/* Bwahahahaha... */
-	nubus_get_subdir(&ent, &dir);
-	board->directory = dir.base;
-	return;
-
-	/* Even more evil laughter... */
- badrom:
-	board->directory = board->fblock;
-	nubus_move(&board->directory, nubus_expand32(board->doffset), board->lanes);
-	printk(KERN_ERR "nubus_get_rom_dir: ROM weirdness!  Notify the developers...\n");
-}
-
 /* Add a board (might be many devices) to the list */
 static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 {
@@ -828,8 +738,11 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 	 * since the initial Macintosh ROM releases skipped the check.
 	 */
 
-	/* Attempt to work around slot zero weirdness */
-	nubus_find_rom_dir(board);
+	/* Set up the directory pointer */
+	board->directory = board->fblock;
+	nubus_move(&board->directory, nubus_expand32(board->doffset),
+	           board->lanes);
+
 	nubus_get_root_dir(board, &dir);
 
 	/* We're ready to rock */
@@ -849,9 +762,6 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 		nubus_get_board_resource(board, slot, &ent);
 	}
 
-	/* Aaaarrrrgghh!  The LC III motherboard has *two* board
-	   resources.  I have no idea WTF to do about this. */
-
 	while (nubus_readdir(&dir, &ent) != -1) {
 		struct nubus_dev *dev;
 		struct nubus_dev **devp;
@@ -922,10 +832,6 @@ void __init nubus_scan_bus(void)
 {
 	int slot;
 
-	/* This might not work on your machine */
-#ifdef I_WANT_TO_PROBE_SLOT_ZERO
-	nubus_probe_slot(0);
-#endif
 	for (slot = 9; slot < 15; slot++) {
 		nubus_probe_slot(slot);
 	}
@@ -943,13 +849,6 @@ static int __init nubus_init(void)
 		via_nubus_init();
 	}
 
-#ifdef TRY_TO_DODGE_WSOD
-	/* Rogue Ethernet interrupts can kill the machine if we don't
-	   do this.  Obviously this is bogus.  Hopefully the local VIA
-	   gurus can fix the real cause of the problem. */
-	mdelay(1000);
-#endif
-
 	/* And probe */
 	pr_info("NuBus: Scanning NuBus slots.\n");
 	nubus_devices = NULL;
-- 
2.10.2

Re: [RFC 1/2] nubus: Remove slot zero probe

[Finn Thain]

Oops, I accidentally sent that with the wrong "From" headers. The email 
message was from me though the patch was from David. Sorry for any 
confusion...

On Sat, 22 Apr 2017, David Huggins-Daines wrote:

> Some long forgotten changes from the linux-mac68k CVS:
> 
> Remove the slot 0 (ROM) probing.
> Remove the pointless White Screen Of Death crap.
> 
> The original commit is here:
> 
> http://linux-mac68k.cvs.sourceforge.net/viewvc/linux-mac68k/linux-mac68k/drivers/nubus/nubus.c?r1=1.22.2.1&r2=1.22.2.2&pathrev=linux-2_2
> 
> This is mostly dead code removal. It is also a reversion to the
> pre-v2.3.17 version. The old code was thoroughly tested in Debian Sarge.
> 
> The present code for probing fake slot resources in Apple's on-board
> (slot zero) ROMs could be useful if replaced the macintosh_config struct.
> But it can't do so and we don't want two mechanisms for identifying
> on-board hardware.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>  drivers/nubus/nubus.c | 111 +++-----------------------------------------------
>  1 file changed, 5 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> index 1742ccc..bb628de 100644
> --- a/drivers/nubus/nubus.c
> +++ b/drivers/nubus/nubus.c
> @@ -13,7 +13,6 @@
>  #include <linux/nubus.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> -#include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <asm/setup.h>
> @@ -34,14 +33,6 @@ extern void oss_nubus_init(void);
>  
>  #define NUBUS_TEST_PATTERN 0x5A932BC7
>  
> -/* Define this if you like to live dangerously - it is known not to
> -   work on pretty much every machine except the Quadra 630 and the LC
> -   III. */
> -#undef I_WANT_TO_PROBE_SLOT_ZERO
> -
> -/* This sometimes helps combat failure to boot */
> -#undef TRY_TO_DODGE_WSOD
> -
>  /* Globals */
>  
>  struct nubus_dev *nubus_devices;
> @@ -454,10 +445,6 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
>  	pr_info("  Function 0x%02x:\n", parent->type);
>  	nubus_get_subdir(parent, &dir);
>  
> -	/* Apple seems to have botched the ROM on the IIx */
> -	if (slot == 0 && (unsigned long)dir.base % 2)
> -		dir.base += 1;
> -
>  	pr_debug("%s: parent is 0x%p, dir is 0x%p\n",
>  	         __func__, parent->base, dir.base);
>  
> @@ -691,83 +678,6 @@ static int __init nubus_get_board_resource(struct nubus_board *board, int slot,
>  	return 0;
>  }
>  
> -/* Attempt to bypass the somewhat non-obvious arrangement of
> -   sResources in the motherboard ROM */
> -static void __init nubus_find_rom_dir(struct nubus_board* board)
> -{
> -	unsigned char *rp;
> -	unsigned char *romdir;
> -	struct nubus_dir dir;
> -	struct nubus_dirent ent;
> -
> -	/* Check for the extra directory just under the format block */
> -	rp = board->fblock;
> -	nubus_rewind(&rp, 4, board->lanes);
> -	if (nubus_get_rom(&rp, 4, board->lanes) != NUBUS_TEST_PATTERN) {
> -		/* OK, the ROM was telling the truth */
> -		board->directory = board->fblock;
> -		nubus_move(&board->directory,
> -			   nubus_expand32(board->doffset),
> -			   board->lanes);
> -		return;
> -	}
> -
> -	/* On "slot zero", you have to walk down a few more
> -	   directories to get to the equivalent of a real card's root
> -	   directory.  We don't know what they were smoking when they
> -	   came up with this. */
> -	romdir = nubus_rom_addr(board->slot);
> -	nubus_rewind(&romdir, ROM_DIR_OFFSET, board->lanes);
> -	dir.base = dir.ptr = romdir;
> -	dir.done = 0;
> -	dir.mask = board->lanes;
> -
> -	/* This one points to an "Unknown Macintosh" directory */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_INFO "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -	/* This one takes us to where we want to go. */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -	nubus_get_subdir(&ent, &dir);
> -
> -	/* Resource ID 01, also an "Unknown Macintosh" */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -
> -	/* FIXME: the first one is *not* always the right one.  We
> -	   suspect this has something to do with the ROM revision.
> -	   "The HORROR ROM" (LC-series) uses 0x7e, while "The HORROR
> -	   Continues" (Q630) uses 0x7b.  The DAFB Macs evidently use
> -	   something else.  Please run "Slots" on your Mac (see
> -	   include/linux/nubus.h for where to get this program) and
> -	   tell us where the 'SiDirPtr' for Slot 0 is.  If you feel
> -	   brave, you should also use MacsBug to walk down the ROM
> -	   directories like this function does and try to find the
> -	   path to that address... */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -
> -	/* Bwahahahaha... */
> -	nubus_get_subdir(&ent, &dir);
> -	board->directory = dir.base;
> -	return;
> -
> -	/* Even more evil laughter... */
> - badrom:
> -	board->directory = board->fblock;
> -	nubus_move(&board->directory, nubus_expand32(board->doffset), board->lanes);
> -	printk(KERN_ERR "nubus_get_rom_dir: ROM weirdness!  Notify the developers...\n");
> -}
> -
>  /* Add a board (might be many devices) to the list */
>  static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  {
> @@ -828,8 +738,11 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  	 * since the initial Macintosh ROM releases skipped the check.
>  	 */
>  
> -	/* Attempt to work around slot zero weirdness */
> -	nubus_find_rom_dir(board);
> +	/* Set up the directory pointer */
> +	board->directory = board->fblock;
> +	nubus_move(&board->directory, nubus_expand32(board->doffset),
> +	           board->lanes);
> +
>  	nubus_get_root_dir(board, &dir);
>  
>  	/* We're ready to rock */
> @@ -849,9 +762,6 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  		nubus_get_board_resource(board, slot, &ent);
>  	}
>  
> -	/* Aaaarrrrgghh!  The LC III motherboard has *two* board
> -	   resources.  I have no idea WTF to do about this. */
> -
>  	while (nubus_readdir(&dir, &ent) != -1) {
>  		struct nubus_dev *dev;
>  		struct nubus_dev **devp;
> @@ -922,10 +832,6 @@ void __init nubus_scan_bus(void)
>  {
>  	int slot;
>  
> -	/* This might not work on your machine */
> -#ifdef I_WANT_TO_PROBE_SLOT_ZERO
> -	nubus_probe_slot(0);
> -#endif
>  	for (slot = 9; slot < 15; slot++) {
>  		nubus_probe_slot(slot);
>  	}
> @@ -943,13 +849,6 @@ static int __init nubus_init(void)
>  		via_nubus_init();
>  	}
>  
> -#ifdef TRY_TO_DODGE_WSOD
> -	/* Rogue Ethernet interrupts can kill the machine if we don't
> -	   do this.  Obviously this is bogus.  Hopefully the local VIA
> -	   gurus can fix the real cause of the problem. */
> -	mdelay(1000);
> -#endif
> -
>  	/* And probe */
>  	pr_info("NuBus: Scanning NuBus slots.\n");
>  	nubus_devices = NULL;
> 

Re: [RFC 1/2] nubus: Remove slot zero probe

[Michael Schmitz]

Hi Finn,

what would the onboard ROM resources be potentially used for? Something
that's not in the config struct or can't be added there because no one
remembers how to build the Penguin booter?

Seeing as the original code (that this patch reverts to) was used while
the Mac port was more widely in use and tested argues strongly in favour
of accepting this patch. Whoever wants to ever revive this code will
find it in the git history...

Cheers,

	Michael


Am 23.04.2017 um 13:24 schrieb David Huggins-Daines:
> Some long forgotten changes from the linux-mac68k CVS:
> 
> Remove the slot 0 (ROM) probing.
> Remove the pointless White Screen Of Death crap.
> 
> The original commit is here:
> 
> http://linux-mac68k.cvs.sourceforge.net/viewvc/linux-mac68k/linux-mac68k/drivers/nubus/nubus.c?r1=1.22.2.1&r2=1.22.2.2&pathrev=linux-2_2
> 
> This is mostly dead code removal. It is also a reversion to the
> pre-v2.3.17 version. The old code was thoroughly tested in Debian Sarge.
> 
> The present code for probing fake slot resources in Apple's on-board
> (slot zero) ROMs could be useful if replaced the macintosh_config struct.
> But it can't do so and we don't want two mechanisms for identifying
> on-board hardware.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>  drivers/nubus/nubus.c | 111 +++-----------------------------------------------
>  1 file changed, 5 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> index 1742ccc..bb628de 100644
> --- a/drivers/nubus/nubus.c
> +++ b/drivers/nubus/nubus.c
> @@ -13,7 +13,6 @@
>  #include <linux/nubus.h>
>  #include <linux/errno.h>
>  #include <linux/init.h>
> -#include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <asm/setup.h>
> @@ -34,14 +33,6 @@ extern void oss_nubus_init(void);
>  
>  #define NUBUS_TEST_PATTERN 0x5A932BC7
>  
> -/* Define this if you like to live dangerously - it is known not to
> -   work on pretty much every machine except the Quadra 630 and the LC
> -   III. */
> -#undef I_WANT_TO_PROBE_SLOT_ZERO
> -
> -/* This sometimes helps combat failure to boot */
> -#undef TRY_TO_DODGE_WSOD
> -
>  /* Globals */
>  
>  struct nubus_dev *nubus_devices;
> @@ -454,10 +445,6 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
>  	pr_info("  Function 0x%02x:\n", parent->type);
>  	nubus_get_subdir(parent, &dir);
>  
> -	/* Apple seems to have botched the ROM on the IIx */
> -	if (slot == 0 && (unsigned long)dir.base % 2)
> -		dir.base += 1;
> -
>  	pr_debug("%s: parent is 0x%p, dir is 0x%p\n",
>  	         __func__, parent->base, dir.base);
>  
> @@ -691,83 +678,6 @@ static int __init nubus_get_board_resource(struct nubus_board *board, int slot,
>  	return 0;
>  }
>  
> -/* Attempt to bypass the somewhat non-obvious arrangement of
> -   sResources in the motherboard ROM */
> -static void __init nubus_find_rom_dir(struct nubus_board* board)
> -{
> -	unsigned char *rp;
> -	unsigned char *romdir;
> -	struct nubus_dir dir;
> -	struct nubus_dirent ent;
> -
> -	/* Check for the extra directory just under the format block */
> -	rp = board->fblock;
> -	nubus_rewind(&rp, 4, board->lanes);
> -	if (nubus_get_rom(&rp, 4, board->lanes) != NUBUS_TEST_PATTERN) {
> -		/* OK, the ROM was telling the truth */
> -		board->directory = board->fblock;
> -		nubus_move(&board->directory,
> -			   nubus_expand32(board->doffset),
> -			   board->lanes);
> -		return;
> -	}
> -
> -	/* On "slot zero", you have to walk down a few more
> -	   directories to get to the equivalent of a real card's root
> -	   directory.  We don't know what they were smoking when they
> -	   came up with this. */
> -	romdir = nubus_rom_addr(board->slot);
> -	nubus_rewind(&romdir, ROM_DIR_OFFSET, board->lanes);
> -	dir.base = dir.ptr = romdir;
> -	dir.done = 0;
> -	dir.mask = board->lanes;
> -
> -	/* This one points to an "Unknown Macintosh" directory */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_INFO "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -	/* This one takes us to where we want to go. */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -	nubus_get_subdir(&ent, &dir);
> -
> -	/* Resource ID 01, also an "Unknown Macintosh" */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -
> -	/* FIXME: the first one is *not* always the right one.  We
> -	   suspect this has something to do with the ROM revision.
> -	   "The HORROR ROM" (LC-series) uses 0x7e, while "The HORROR
> -	   Continues" (Q630) uses 0x7b.  The DAFB Macs evidently use
> -	   something else.  Please run "Slots" on your Mac (see
> -	   include/linux/nubus.h for where to get this program) and
> -	   tell us where the 'SiDirPtr' for Slot 0 is.  If you feel
> -	   brave, you should also use MacsBug to walk down the ROM
> -	   directories like this function does and try to find the
> -	   path to that address... */
> -	if (nubus_readdir(&dir, &ent) == -1)
> -		goto badrom;
> -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> -
> -	/* Bwahahahaha... */
> -	nubus_get_subdir(&ent, &dir);
> -	board->directory = dir.base;
> -	return;
> -
> -	/* Even more evil laughter... */
> - badrom:
> -	board->directory = board->fblock;
> -	nubus_move(&board->directory, nubus_expand32(board->doffset), board->lanes);
> -	printk(KERN_ERR "nubus_get_rom_dir: ROM weirdness!  Notify the developers...\n");
> -}
> -
>  /* Add a board (might be many devices) to the list */
>  static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  {
> @@ -828,8 +738,11 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  	 * since the initial Macintosh ROM releases skipped the check.
>  	 */
>  
> -	/* Attempt to work around slot zero weirdness */
> -	nubus_find_rom_dir(board);
> +	/* Set up the directory pointer */
> +	board->directory = board->fblock;
> +	nubus_move(&board->directory, nubus_expand32(board->doffset),
> +	           board->lanes);
> +
>  	nubus_get_root_dir(board, &dir);
>  
>  	/* We're ready to rock */
> @@ -849,9 +762,6 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
>  		nubus_get_board_resource(board, slot, &ent);
>  	}
>  
> -	/* Aaaarrrrgghh!  The LC III motherboard has *two* board
> -	   resources.  I have no idea WTF to do about this. */
> -
>  	while (nubus_readdir(&dir, &ent) != -1) {
>  		struct nubus_dev *dev;
>  		struct nubus_dev **devp;
> @@ -922,10 +832,6 @@ void __init nubus_scan_bus(void)
>  {
>  	int slot;
>  
> -	/* This might not work on your machine */
> -#ifdef I_WANT_TO_PROBE_SLOT_ZERO
> -	nubus_probe_slot(0);
> -#endif
>  	for (slot = 9; slot < 15; slot++) {
>  		nubus_probe_slot(slot);
>  	}
> @@ -943,13 +849,6 @@ static int __init nubus_init(void)
>  		via_nubus_init();
>  	}
>  
> -#ifdef TRY_TO_DODGE_WSOD
> -	/* Rogue Ethernet interrupts can kill the machine if we don't
> -	   do this.  Obviously this is bogus.  Hopefully the local VIA
> -	   gurus can fix the real cause of the problem. */
> -	mdelay(1000);
> -#endif
> -
>  	/* And probe */
>  	pr_info("NuBus: Scanning NuBus slots.\n");
>  	nubus_devices = NULL;
> 

Re: [RFC 1/2] nubus: Remove slot zero probe

[Finn Thain]

On Tue, 25 Apr 2017, Michael Schmitz wrote:

> Hi Finn,
> 
> what would the onboard ROM resources be potentially used for? Something 
> that's not in the config struct or can't be added there because no one 
> remembers how to build the Penguin booter?
> 

The hardware specs in the Mac bootinfo data are basically ignored by the 
kernel. So I think the issue is stuff that's not in the config struct and 
can't be inferred from the gestalt ID at all. (The macintosh_config struct 
hinges on the gestalt ID.)

The bootinfo data should offer a normal gestalt ID... assuming the 
bootloader can obtain one. Theoretically EMILE could be used with a Mac 
ROM so old that the _Gestalt trap gives the wrong answer, or is simply 
not available...
https://www.fenestrated.net/mac/mirrors/Apple Technotes (As of 2002)/ov/ov_16.html

I strongly suspect that such machines have no slot zero ROM anyway. And 
this doesn't apply to Penguin at all, since it can assume System 7.1 and 
therefore it can get a correct gestalt ID.

The real problem is, even if the slot zero ROM tells you (say) the VRAM 
size on one model, a different model might not put that information there. 
And if we want to handle both cases, why bother with the slot zero ROMs at 
all?

> Seeing as the original code (that this patch reverts to) was used while 
> the Mac port was more widely in use and tested argues strongly in favour 
> of accepting this patch.

Right.

Another strong argument stems from all of the hacks (see below) around the 
various flaws in the various ROMs. If a large number of Mac models need 
special workarounds for flaws in their ROMs, you might as well just 
describe the various exceptions in an array of structs, indexed by gestalt 
ID number ... which is to reinvent the macintosh_config struct.

> Whoever wants to ever revive this code will find it in the git 
> history...
> 

Or the CVS history, since the decision to remove this code was made there 
too (many years ago).

-- 

> Cheers,
> 
> 	Michael
> 
> 
> Am 23.04.2017 um 13:24 schrieb David Huggins-Daines:
> > Some long forgotten changes from the linux-mac68k CVS:
> > 
> > Remove the slot 0 (ROM) probing.
> > Remove the pointless White Screen Of Death crap.
> > 
> > The original commit is here:
> > 
> > http://linux-mac68k.cvs.sourceforge.net/viewvc/linux-mac68k/linux-mac68k/drivers/nubus/nubus.c?r1=1.22.2.1&r2=1.22.2.2&pathrev=linux-2_2
> > 
> > This is mostly dead code removal. It is also a reversion to the
> > pre-v2.3.17 version. The old code was thoroughly tested in Debian Sarge.
> > 
> > The present code for probing fake slot resources in Apple's on-board
> > (slot zero) ROMs could be useful if replaced the macintosh_config struct.
> > But it can't do so and we don't want two mechanisms for identifying
> > on-board hardware.
> > 
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > ---
> >  drivers/nubus/nubus.c | 111 +++-----------------------------------------------
> >  1 file changed, 5 insertions(+), 106 deletions(-)
> > 
> > diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> > index 1742ccc..bb628de 100644
> > --- a/drivers/nubus/nubus.c
> > +++ b/drivers/nubus/nubus.c
> > @@ -13,7 +13,6 @@
> >  #include <linux/nubus.h>
> >  #include <linux/errno.h>
> >  #include <linux/init.h>
> > -#include <linux/delay.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> >  #include <asm/setup.h>
> > @@ -34,14 +33,6 @@ extern void oss_nubus_init(void);
> >  
> >  #define NUBUS_TEST_PATTERN 0x5A932BC7
> >  
> > -/* Define this if you like to live dangerously - it is known not to
> > -   work on pretty much every machine except the Quadra 630 and the LC
> > -   III. */
> > -#undef I_WANT_TO_PROBE_SLOT_ZERO
> > -
> > -/* This sometimes helps combat failure to boot */
> > -#undef TRY_TO_DODGE_WSOD
> > -
> >  /* Globals */
> >  
> >  struct nubus_dev *nubus_devices;
> > @@ -454,10 +445,6 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
> >  	pr_info("  Function 0x%02x:\n", parent->type);
> >  	nubus_get_subdir(parent, &dir);
> >  
> > -	/* Apple seems to have botched the ROM on the IIx */
> > -	if (slot == 0 && (unsigned long)dir.base % 2)
> > -		dir.base += 1;
> > -
> >  	pr_debug("%s: parent is 0x%p, dir is 0x%p\n",
> >  	         __func__, parent->base, dir.base);
> >  
> > @@ -691,83 +678,6 @@ static int __init nubus_get_board_resource(struct nubus_board *board, int slot,
> >  	return 0;
> >  }
> >  
> > -/* Attempt to bypass the somewhat non-obvious arrangement of
> > -   sResources in the motherboard ROM */
> > -static void __init nubus_find_rom_dir(struct nubus_board* board)
> > -{
> > -	unsigned char *rp;
> > -	unsigned char *romdir;
> > -	struct nubus_dir dir;
> > -	struct nubus_dirent ent;
> > -
> > -	/* Check for the extra directory just under the format block */
> > -	rp = board->fblock;
> > -	nubus_rewind(&rp, 4, board->lanes);
> > -	if (nubus_get_rom(&rp, 4, board->lanes) != NUBUS_TEST_PATTERN) {
> > -		/* OK, the ROM was telling the truth */
> > -		board->directory = board->fblock;
> > -		nubus_move(&board->directory,
> > -			   nubus_expand32(board->doffset),
> > -			   board->lanes);
> > -		return;
> > -	}
> > -
> > -	/* On "slot zero", you have to walk down a few more
> > -	   directories to get to the equivalent of a real card's root
> > -	   directory.  We don't know what they were smoking when they
> > -	   came up with this. */
> > -	romdir = nubus_rom_addr(board->slot);
> > -	nubus_rewind(&romdir, ROM_DIR_OFFSET, board->lanes);
> > -	dir.base = dir.ptr = romdir;
> > -	dir.done = 0;
> > -	dir.mask = board->lanes;
> > -
> > -	/* This one points to an "Unknown Macintosh" directory */
> > -	if (nubus_readdir(&dir, &ent) == -1)
> > -		goto badrom;
> > -
> > -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> > -		printk(KERN_INFO "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> > -	/* This one takes us to where we want to go. */
> > -	if (nubus_readdir(&dir, &ent) == -1)
> > -		goto badrom;
> > -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> > -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> > -	nubus_get_subdir(&ent, &dir);
> > -
> > -	/* Resource ID 01, also an "Unknown Macintosh" */
> > -	if (nubus_readdir(&dir, &ent) == -1)
> > -		goto badrom;
> > -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> > -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> > -
> > -	/* FIXME: the first one is *not* always the right one.  We
> > -	   suspect this has something to do with the ROM revision.
> > -	   "The HORROR ROM" (LC-series) uses 0x7e, while "The HORROR
> > -	   Continues" (Q630) uses 0x7b.  The DAFB Macs evidently use
> > -	   something else.  Please run "Slots" on your Mac (see
> > -	   include/linux/nubus.h for where to get this program) and
> > -	   tell us where the 'SiDirPtr' for Slot 0 is.  If you feel
> > -	   brave, you should also use MacsBug to walk down the ROM
> > -	   directories like this function does and try to find the
> > -	   path to that address... */
> > -	if (nubus_readdir(&dir, &ent) == -1)
> > -		goto badrom;
> > -	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> > -		printk(KERN_DEBUG "nubus_get_rom_dir: entry %02x %06x\n", ent.type, ent.data);
> > -
> > -	/* Bwahahahaha... */
> > -	nubus_get_subdir(&ent, &dir);
> > -	board->directory = dir.base;
> > -	return;
> > -
> > -	/* Even more evil laughter... */
> > - badrom:
> > -	board->directory = board->fblock;
> > -	nubus_move(&board->directory, nubus_expand32(board->doffset), board->lanes);
> > -	printk(KERN_ERR "nubus_get_rom_dir: ROM weirdness!  Notify the developers...\n");
> > -}
> > -
> >  /* Add a board (might be many devices) to the list */
> >  static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
> >  {
> > @@ -828,8 +738,11 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
> >  	 * since the initial Macintosh ROM releases skipped the check.
> >  	 */
> >  
> > -	/* Attempt to work around slot zero weirdness */
> > -	nubus_find_rom_dir(board);
> > +	/* Set up the directory pointer */
> > +	board->directory = board->fblock;
> > +	nubus_move(&board->directory, nubus_expand32(board->doffset),
> > +	           board->lanes);
> > +
> >  	nubus_get_root_dir(board, &dir);
> >  
> >  	/* We're ready to rock */
> > @@ -849,9 +762,6 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
> >  		nubus_get_board_resource(board, slot, &ent);
> >  	}
> >  
> > -	/* Aaaarrrrgghh!  The LC III motherboard has *two* board
> > -	   resources.  I have no idea WTF to do about this. */
> > -
> >  	while (nubus_readdir(&dir, &ent) != -1) {
> >  		struct nubus_dev *dev;
> >  		struct nubus_dev **devp;
> > @@ -922,10 +832,6 @@ void __init nubus_scan_bus(void)
> >  {
> >  	int slot;
> >  
> > -	/* This might not work on your machine */
> > -#ifdef I_WANT_TO_PROBE_SLOT_ZERO
> > -	nubus_probe_slot(0);
> > -#endif
> >  	for (slot = 9; slot < 15; slot++) {
> >  		nubus_probe_slot(slot);
> >  	}
> > @@ -943,13 +849,6 @@ static int __init nubus_init(void)
> >  		via_nubus_init();
> >  	}
> >  
> > -#ifdef TRY_TO_DODGE_WSOD
> > -	/* Rogue Ethernet interrupts can kill the machine if we don't
> > -	   do this.  Obviously this is bogus.  Hopefully the local VIA
> > -	   gurus can fix the real cause of the problem. */
> > -	mdelay(1000);
> > -#endif
> > -
> >  	/* And probe */
> >  	pr_info("NuBus: Scanning NuBus slots.\n");
> >  	nubus_devices = NULL;
> > 
> 

Re: [RFC 1/2] nubus: Remove slot zero probe

[Michael Schmitz]

Hi Finn,

I don't think anyone would want to replace the macintosh_config struct
with something based on the mainboard ROM data (or ROM data plus
hardcoded model info) at this time. So this code can truly go. Thanks
for making the effort to clean up this code.

Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>

Am 25.04.2017 um 17:19 schrieb Finn Thain:
> 
> On Tue, 25 Apr 2017, Michael Schmitz wrote:
> 
>> Hi Finn,
>>
>> what would the onboard ROM resources be potentially used for? Something 
>> that's not in the config struct or can't be added there because no one 
>> remembers how to build the Penguin booter?
>>
> 
> The hardware specs in the Mac bootinfo data are basically ignored by the 
> kernel. So I think the issue is stuff that's not in the config struct and 
> can't be inferred from the gestalt ID at all. (The macintosh_config struct 
> hinges on the gestalt ID.)
> 
> The bootinfo data should offer a normal gestalt ID... assuming the 
> bootloader can obtain one. Theoretically EMILE could be used with a Mac 
> ROM so old that the _Gestalt trap gives the wrong answer, or is simply 
> not available...
> https://www.fenestrated.net/mac/mirrors/Apple Technotes (As of 2002)/ov/ov_16.html
> 
> I strongly suspect that such machines have no slot zero ROM anyway. And 
> this doesn't apply to Penguin at all, since it can assume System 7.1 and 
> therefore it can get a correct gestalt ID.
> 
> The real problem is, even if the slot zero ROM tells you (say) the VRAM 
> size on one model, a different model might not put that information there. 
> And if we want to handle both cases, why bother with the slot zero ROMs at 
> all?
> 
>> Seeing as the original code (that this patch reverts to) was used while 
>> the Mac port was more widely in use and tested argues strongly in favour 
>> of accepting this patch.
> 
> Right.
> 
> Another strong argument stems from all of the hacks (see below) around the 
> various flaws in the various ROMs. If a large number of Mac models need 
> special workarounds for flaws in their ROMs, you might as well just 
> describe the various exceptions in an array of structs, indexed by gestalt 
> ID number ... which is to reinvent the macintosh_config struct.
> 
>> Whoever wants to ever revive this code will find it in the git 
>> history...
>>
> 
> Or the CVS history, since the decision to remove this code was made there 
> too (many years ago).
> 

[RFC 2/2] nubus: Fix pointer validation

[Finn Thain]

Fix bounds checking on slot-space pointer movement.
Remove redundant test for zero byte-lanes value.
Fix broken byte-lanes vs. address validation.

This patch changes the circumstances under which an error is printed to
the console and fixes the address validation.

The validation code should work correctly now: the broken test for a
valid bytelanes value is replaced with a working test (which eliminates
false negatives) and the 24-bit directory offset bounds check is fixed
(which eliminates false positives).

Please see "Designing Cards and Drivers for the Macintosh Family"
ch. 8, "NuBus Card Firmware" for an explanation of the bytelanes check
and directory offset value.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
To fully test this would require a card with a flawed ROM, which is
difficult to find. The valid case has been tested however.
---
 drivers/nubus/nubus.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index bb628de..bb81347 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -92,9 +92,6 @@ static void nubus_rewind(unsigned char **ptr, int len, int map)
 {
 	unsigned char *p = *ptr;
 
-	/* Sanity check */
-	if (len > 65536)
-		pr_err("rewind of 0x%08x!\n", len);
 	while (len) {
 		do {
 			p--;
@@ -108,8 +105,6 @@ static void nubus_advance(unsigned char **ptr, int len, int map)
 {
 	unsigned char *p = *ptr;
 
-	if (len > 65536)
-		pr_err("advance of 0x%08x!\n", len);
 	while (len) {
 		while (not_useful(p, map))
 			p++;
@@ -121,10 +116,15 @@ static void nubus_advance(unsigned char **ptr, int len, int map)
 
 static void nubus_move(unsigned char **ptr, int len, int map)
 {
+	unsigned long slot_space = (unsigned long)*ptr & 0xFF000000;
+
 	if (len > 0)
 		nubus_advance(ptr, len, map);
 	else if (len < 0)
 		nubus_rewind(ptr, -len, map);
+
+	if (((unsigned long)*ptr & 0xFF000000) != slot_space)
+		pr_err("%s: moved out of slot address space!\n", __func__);
 }
 
 /* Now, functions to read the sResource tree */
@@ -808,8 +808,6 @@ void __init nubus_probe_slot(int slot)
 			continue;
 
 		dp = *rp;
-		if(dp == 0)
-			continue;
 
 		/* The last byte of the format block consists of two
 		   nybbles which are "mirror images" of each other.
@@ -818,7 +816,7 @@ void __init nubus_probe_slot(int slot)
 			continue;
 		/* Check that this value is actually *on* one of the
 		   bytelanes it claims are valid! */
-		if ((dp & 0x0F) >= (1 << i))
+		if (not_useful(rp, dp))
 			continue;
 
 		/* Looks promising.  Let's put it on the list. */
-- 
2.10.2

Re: [RFC 2/2] nubus: Fix pointer validation

[Michael Schmitz]

Hi Finn,

I had to look at a few other functions in the nubus driver to figure out
how the address layout looks for th bus address range, but it all made
sense in the end. Can't find any fault with this one. So:

Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>


Am 23.04.2017 um 13:24 schrieb Finn Thain:
> Fix bounds checking on slot-space pointer movement.
> Remove redundant test for zero byte-lanes value.
> Fix broken byte-lanes vs. address validation.
> 
> This patch changes the circumstances under which an error is printed to
> the console and fixes the address validation.
> 
> The validation code should work correctly now: the broken test for a
> valid bytelanes value is replaced with a working test (which eliminates
> false negatives) and the 24-bit directory offset bounds check is fixed
> (which eliminates false positives).
> 
> Please see "Designing Cards and Drivers for the Macintosh Family"
> ch. 8, "NuBus Card Firmware" for an explanation of the bytelanes check
> and directory offset value.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
> To fully test this would require a card with a flawed ROM, which is
> difficult to find. The valid case has been tested however.
> ---
>  drivers/nubus/nubus.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> index bb628de..bb81347 100644
> --- a/drivers/nubus/nubus.c
> +++ b/drivers/nubus/nubus.c
> @@ -92,9 +92,6 @@ static void nubus_rewind(unsigned char **ptr, int len, int map)
>  {
>  	unsigned char *p = *ptr;
>  
> -	/* Sanity check */
> -	if (len > 65536)
> -		pr_err("rewind of 0x%08x!\n", len);
>  	while (len) {
>  		do {
>  			p--;
> @@ -108,8 +105,6 @@ static void nubus_advance(unsigned char **ptr, int len, int map)
>  {
>  	unsigned char *p = *ptr;
>  
> -	if (len > 65536)
> -		pr_err("advance of 0x%08x!\n", len);
>  	while (len) {
>  		while (not_useful(p, map))
>  			p++;
> @@ -121,10 +116,15 @@ static void nubus_advance(unsigned char **ptr, int len, int map)
>  
>  static void nubus_move(unsigned char **ptr, int len, int map)
>  {
> +	unsigned long slot_space = (unsigned long)*ptr & 0xFF000000;
> +
>  	if (len > 0)
>  		nubus_advance(ptr, len, map);
>  	else if (len < 0)
>  		nubus_rewind(ptr, -len, map);
> +
> +	if (((unsigned long)*ptr & 0xFF000000) != slot_space)
> +		pr_err("%s: moved out of slot address space!\n", __func__);
>  }
>  
>  /* Now, functions to read the sResource tree */
> @@ -808,8 +808,6 @@ void __init nubus_probe_slot(int slot)
>  			continue;
>  
>  		dp = *rp;
> -		if(dp == 0)
> -			continue;
>  
>  		/* The last byte of the format block consists of two
>  		   nybbles which are "mirror images" of each other.
> @@ -818,7 +816,7 @@ void __init nubus_probe_slot(int slot)
>  			continue;
>  		/* Check that this value is actually *on* one of the
>  		   bytelanes it claims are valid! */
> -		if ((dp & 0x0F) >= (1 << i))
> +		if (not_useful(rp, dp))
>  			continue;
>  
>  		/* Looks promising.  Let's put it on the list. */
> 

Re: [RFC 0/2] mac68k: NuBus fixes

[Geert Uytterhoeven]

Hi Finn,

On Sun, Apr 23, 2017 at 3:24 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Geert has agreed to merge these two patches if they receive a favorable
> review. Is anyone willing to do a review?
>
>
> David Huggins-Daines (1):
>   nubus: Remove slot zero probe
>
> Finn Thain (1):
>   nubus: Fix pointer validation

Thanks, both applied with Michael's Reviewed-by, and queued for v4.13.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds