[PATCH 0/2] SPARC32: fix crash when starting qemu-system-sparc with no framebuffer

[Mark Cave-Ayland]

      A recent patch to QEMU that allows sun4m machines to be launched with -vga none
revealed a crash on startup due to the missing framebuffer.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;

Mark Cave-Ayland (2):
  sbus: don't use internal fallback TCX driver with QEMU
  SPARC32: use serial console when QEMU is launched with -vga none

 arch/sparc32/init.fs    |  7 +++++--
 arch/sparc32/openbios.c |  8 ++++++++
 drivers/sbus.c          | 11 +++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.11.0

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[Mark Cave-Ayland]

      OpenBIOS SPARC32 has always maintained a fallback option of an internal TCX
driver if an FCode driver cannot be located on the TCX card itself.

Since QEMU's TCX adapter has supported FCode payloads for a long time then
we only require this logic for non-QEMU builds. Otherwise we unintentionally
install the fallback driver and device node even if the framebuffer card hasn't
been installed into the machine.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;
---
 arch/sparc32/init.fs |  7 +++++--
 drivers/sbus.c       | 11 +++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/sparc32/init.fs b/arch/sparc32/init.fs
index 814c720..5e8805b 100644
--- a/arch/sparc32/init.fs
+++ b/arch/sparc32/init.fs
@@ -78,6 +78,9 @@ defer ignore-dfault
 
 \ Load TCX FCode driver blob
 [IFDEF] CONFIG_DRIVER_SBUS
-  -1 value tcx-driver-fcode
-  " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
+  [IFDEF] CONFIG_QEMU
+  [ELSE]
+    -1 value tcx-driver-fcode
+    " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
+  [THEN]
 [THEN]
diff --git a/drivers/sbus.c b/drivers/sbus.c
index 9cce89f..d59963e 100644
--- a/drivers/sbus.c
+++ b/drivers/sbus.c
@@ -138,6 +138,7 @@ ob_le_init(unsigned int slot, uint64_t base, unsigned long leoffset,
unsigned lo
 
 uint16_t graphic_depth;
 
+#if !defined(CONFIG_QEMU)
 static void
 ob_tcx_init(unsigned int slot, const char *path)
 {
@@ -162,6 +163,7 @@ ob_tcx_init(unsigned int slot, const char *path)
     /* Restore */
     feval("to my-self active-package!");
 }
+#endif
 
 static void
 ob_apc_init(unsigned int slot, unsigned long base)
@@ -298,9 +300,12 @@ sbus_probe_slot_ss5(unsigned int slot, uint64_t base)
     }
 
     switch(slot) {
+#if !defined(CONFIG_QEMU)
+    /* QEMU always uses the FCode driver */
     case 3: // SUNW,tcx
         ob_tcx_init(slot, "/iommu/sbus/SUNW,tcx");
         break;
+#endif
     case 4:
         // SUNW,CS4231
         ob_cs4231_init(slot);
@@ -327,9 +332,12 @@ sbus_probe_slot_ss10(unsigned int slot, uint64_t base)
     }
 
     switch(slot) {
+#if !defined(CONFIG_QEMU)
+    /* QEMU always uses the FCode driver */
     case 2: // SUNW,tcx
         ob_tcx_init(slot, "/iommu/sbus/SUNW,tcx");
         break;
+#endif
     case 0xf: // le, esp, bpp, power-management
         ob_macio_init(slot, base, 0);
         // Power management (APC) XXX should not exist
@@ -352,9 +360,12 @@ sbus_probe_slot_ss600mp(unsigned int slot, uint64_t base)
     }
 
     switch(slot) {
+#if !defined(CONFIG_QEMU)
+    /* QEMU always uses the FCode driver */
     case 2: // SUNW,tcx
         ob_tcx_init(slot, "/iommu/sbus/SUNW,tcx");
         break;
+#endif
     case 0xf: // le, esp, bpp, power-management
 #ifdef CONFIG_DRIVER_ESP
         ob_esp_init(slot, base, SS600MP_ESP, SS600MP_ESPDMA);
-- 
2.11.0

[PATCH 2/2] SPARC32: use serial console when QEMU is launched with -vga none

[Mark Cave-Ayland]

      If no display device has been installed in the DT e.g. passing -vga none into
the QEMU command line then fall back to using the serial console as already
occurs when launching QEMU in -nographic mode.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;
---
 arch/sparc32/openbios.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/sparc32/openbios.c b/arch/sparc32/openbios.c
index 78005e0..9ec28bf 100644
--- a/arch/sparc32/openbios.c
+++ b/arch/sparc32/openbios.c
@@ -758,8 +758,16 @@ static void setup_stdio(void)
 {
     char nographic;
     const char *stdin, *stdout;
+    phandle_t display_ph;
 
     fw_cfg_read(FW_CFG_NOGRAPHIC, &nographic, 1);
+
+    /* Check to see if any framebuffer present */
+    display_ph = dt_iterate_type(0, "display");
+    if (display_ph == 0) {
+        nographic = 1;
+    }
+
     if (nographic) {
         obp_stdin = PROMDEV_TTYA;
         obp_stdout = PROMDEV_TTYA;
-- 
2.11.0

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[BALATON Zoltan]

      On Wed, 26 Jun 2019, Mark Cave-Ayland wrote:
  On 14/06/2019 21:42, BALATON Zoltan wrote:
  But that's just my view and I don't
really mind either way, so don't take this as
 saying not to apply your patches.
 Thanks for the feedback. Having reflected on this over the past week, I'm still
happy
 with the approach taken (since it's consistent with what I've been doing for a
while
 now) and given that it's coming up to QEMU freeze I'd like to have the fix
included
 in the next release. So this is now pushed to master.
Apropos freeze, is there a chance to include my patch as well:

https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/JX...

which is needed to run MorphOS on mac99 and have working USB? I have a 
patched OpenBIOS binary on my QEMU Amiga page which I haven't updated for 
a while so it may stop working at some point with newer QEMU releases. So 
having this in official OpenBIOS that comes with QEMU would simplify 
things for users. Unless there's something that breaks with it it's not 
worse than the other work arounds already in OpenBIOS IMO.

Regards,
BALATON Zoltan

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[Mark Cave-Ayland]

      On 14/06/2019 21:42, BALATON Zoltan wrote:

  On Fri, 14 Jun 2019, Mark Cave-Ayland wrote:
  On 13/06/2019 21:01, BALATON Zoltan wrote:
  1. If QEMU always supplies the driver as card ROM
which will then be installed, when
 is this fallback necessary at all? Wouldn't it be simpler to just remove it
 altogether? On QEMU it's not needed, other configs probably don't work anyway and
if
 they worked you may get the same problem as in the QEMU case before this patch when
 card is missing so getting rid of this fallback would simplify code and may only
 break real hardware case which is likely already broken anyway.

 or

 2. If you think fallback still makes sense and is used by some real case then the
 correct test may be to check if TCX card is present or not, not if running on QEMU or
 not. Maybe you should do this when setting up the card during sbus scan. The
 drivers/sbus.c seems to create device nodes when some slots are occupied (? is this
 correct? are these cards hardwired to these slots or what if they are not in the
 expected slot or something else is there). Anyway, you could check if these nodes are
 existing but ROM is not valid to decide to install the driver fallback which seems
 more correct. Or you could try to fall back when mapping the ROM or interpreting it
 has failed.
 And this patchset implements option 1 :)
 
 Sort of, but instead of removing the fall back altogether and thus simplifying code
 it only disables it on QEMU which does not get rid of the the code but makes it even
 harder to follow by having a QEMU driver now only used when not on QEMU. So maybe a
 comment would be useful to explain this at least. If this work around is still useful
 for Temlib then that's a valid use case that might justify keeping it but otherwise
 I'd be for simpifyng code instead of keeping around now unused and unneeded hacks
 that may have helped in the past but now just makes it more difficult to follow the
 code. (Mixing different QEMU and OpenBIOS versions may not be a frequent use case
 either and may not work due to changes in fw_cfg anyway.) So if there's nothing left
 needing it any more it would be better to remove such fall backs/work arounds, rather
 than keep everything in fear of breaking some theoretical use case as that results in
 more and more complex code.
 
 But that's just my view and I don't really mind either way, so don't take
this as
 saying not to apply your patches.
Thanks for the feedback. Having reflected on this over the past week, I'm still happy
with the approach taken (since it's consistent with what I've been doing for a
while
now) and given that it's coming up to QEMU freeze I'd like to have the fix
included
in the next release. So this is now pushed to master.


ATB,

Mark.

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[BALATON Zoltan]

      On Fri, 14 Jun 2019, Mark Cave-Ayland wrote:
  On 13/06/2019 21:01, BALATON Zoltan wrote:
  1. If QEMU always supplies the driver as card ROM
which will then be installed, when
 is this fallback necessary at all? Wouldn't it be simpler to just remove it
 altogether? On QEMU it's not needed, other configs probably don't work anyway and
if
 they worked you may get the same problem as in the QEMU case before this patch when
 card is missing so getting rid of this fallback would simplify code and may only
 break real hardware case which is likely already broken anyway.

 or

 2. If you think fallback still makes sense and is used by some real case then the
 correct test may be to check if TCX card is present or not, not if running on QEMU or
 not. Maybe you should do this when setting up the card during sbus scan. The
 drivers/sbus.c seems to create device nodes when some slots are occupied (? is this
 correct? are these cards hardwired to these slots or what if they are not in the
 expected slot or something else is there). Anyway, you could check if these nodes are
 existing but ROM is not valid to decide to install the driver fallback which seems
 more correct. Or you could try to fall back when mapping the ROM or interpreting it
 has failed.
 And this patchset implements option 1 :)
Sort of, but instead of removing the fall back altogether and thus 
simplifying code it only disables it on QEMU which does not get rid of the 
the code but makes it even harder to follow by having a QEMU driver now 
only used when not on QEMU. So maybe a comment would be useful to explain 
this at least. If this work around is still useful for Temlib then that's 
a valid use case that might justify keeping it but otherwise I'd be for 
simpifyng code instead of keeping around now unused and unneeded hacks 
that may have helped in the past but now just makes it more difficult to 
follow the code. (Mixing different QEMU and OpenBIOS versions may not be a 
frequent use case either and may not work due to changes in fw_cfg 
anyway.) So if there's nothing left needing it any more it would be better 
to remove such fall backs/work arounds, rather than keep everything in 
fear of breaking some theoretical use case as that results in more and 
more complex code.

But that's just my view and I don't really mind either way, so don't take 
this as saying not to apply your patches.

  But I think a
proper fix for the problem would be to fix FCode ROM support instead of
 adding fallbacks for everything. This reminds me of the QEMU VGA ndrv fallback which
 is also installed for every VGA device regardless if it works with them or not and I
 had to disable it to try a card ROM or driver with the ati-vga device. Along the same
 lines, that fallback (and the fw_cfg hack to download the driver) could also be
 removed if QEMU set the card ROM to the ndrv but the problem with that is that this
 is the std vga device and we would need to patch ROM only when this is used in a ppc
 mac emulation for which there're no functions in QEMU yet. But changing QEMU to
allow
 board code to specify card ROM may be cleaner than the current way of patching in the
 ndrv for all vga devices and that would allow removing a lot of hacks from OpenBIOS:
 the fw_cfg download, ndrv installation and vga-ndrv? switch and would make it simpler
 until proper FCode support could be implemented some time later.

 I hope at least some of what I said makes sense.
 Indeed, although this is a little off-topic since SBus used by SPARC is separate from
 PCI. Note that you can disable the VGA ndrv driver via -prom-env
'vga-ndrv?=false' on
 the command line if you need it for testing.
Yes, this is off-topic but I was reminded of this as this is similar 
problem and that's what I've done as explained at the end of this page:

https://osdn.net/projects/qmiga/wiki/SubprojectAti

  It is already possible to provide an option ROM to the
VGA device in QEMU, the
 slightly tricky part is that you need to add some routines to QEMU to dynamically
 create the option ROM by creating a "dummy" option ROM header and then
injecting the
 FCode payload. Then it should just be a case of mapping to ROM into the PCI device,
 applying a small patch to OpenBIOS and then you should be good.
I've found that FCode ROMs don't work but vga_config_cb() in 
openbios/drivers/pci.c seems to have other workarounds which installs an 
NDRV or a driver executable directly supplied as a card ROM where MacOS 
expects them. This is the same as how the QEMU,ndrv.bin hack installs the 
driver in openbios/drivers/vga.fs so the latter may not be needed if QEMU 
would supply QEMU,ndrv.bin as the card ROM. This is what I meant above and 
I think I've tried to explain this before but not sure we're on the same 
page yet.

I don't think we need to create option ROM in QEMU (expecially no FCode 
header would be needed as FCode ROMs don't work anyway), we just need a 
way to set the card ROM to QEMU,ndrv.bin when the card is in a 
mac_oldworld or mac_newworld machine. We can set option rom with romfile 
but that's for the card globally and not for the machine so we can't use 
that because the pc machines still need the vgabios option ROM, only mac 
machines need different ROM. But there seem to be no way to set it when 
the card is created through the -vga option. Maybe if the board code could 
set the property of the card before realizing it somehow that could work 
but I don't know how to do that and could not figure out an easy way last 
I've looked at it.

Regards,
BALATON Zoltan

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[BALATON Zoltan]

      On Fri, 28 Jun 2019, Mark Cave-Ayland wrote:
  On 26/06/2019 21:59, BALATON Zoltan wrote:
 
https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/JX...


 which is needed to run MorphOS on mac99 and have working USB? I have a patched
 OpenBIOS binary on my QEMU Amiga page which I haven't updated for a while so it may
 stop working at some point with newer QEMU releases. So having this in official
 OpenBIOS that comes with QEMU would simplify things for users. Unless there's
 something that breaks with it it's not worse than the other work arounds already in
 OpenBIOS IMO.
 From what I can see there is no follow-up patch to deal with fixing up bus-range for
 subsequent PCI buses? Whilst I agree with you that OpenBIOS does have a fair number
 of workarounds, I don't want to introduce something that deliberately breaks the
 conformance of the DT to the specifications when the existing code makes considerable
 efforts to do the right thing.
I've sent a v5 to the list which also patches bus-range of second pci bus 
to match a PowerMac3,1 device tree. It works with MorphOS both ways as 
long as /pci@f0000000 is present and is before /pci@f2000000 (I don't 
think MorphOS considers bus-range property just reads these sequentially). 
I think adding this missing pci bus also fixes the bus-master bit for the 
network cards that needed a hack in OpenBIOS to work so those work arounds 
could be reverted now too but I haven't tested that.

Regards,
BALATON Zoltan

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[BALATON Zoltan]

      On Thu, 13 Jun 2019, Mark Cave-Ayland wrote:
  On 13/06/2019 10:30, BALATON Zoltan wrote:
  On Wed, 12 Jun 2019, Mark Cave-Ayland wrote:
  OpenBIOS SPARC32 has always maintained a fallback
option of an internal TCX
 driver if an FCode driver cannot be located on the TCX card itself.

 Since QEMU's TCX adapter has supported FCode payloads for a long time then
 we only require this logic for non-QEMU builds. Otherwise we unintentionally
 install the fallback driver and device node even if the framebuffer card hasn't
 been installed into the machine.

 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;
 ---
 arch/sparc32/init.fs |? 7 +++++--
 drivers/sbus.c?????? | 11 +++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

 diff --git a/arch/sparc32/init.fs b/arch/sparc32/init.fs
 index 814c720..5e8805b 100644
 --- a/arch/sparc32/init.fs
 +++ b/arch/sparc32/init.fs
 @@ -78,6 +78,9 @@ defer ignore-dfault

 \ Load TCX FCode driver blob
 [IFDEF] CONFIG_DRIVER_SBUS
 -? -1 value tcx-driver-fcode
 -? " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +? [IFDEF] CONFIG_QEMU
 +? [ELSE]
 +??? -1 value tcx-driver-fcode
 +??? " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +? [THEN]
 [THEN]
 It looks strange that the driver is called QEMU,tcx but is not used on QEMU. Does it
 make sense to use it when not in QEMU at all? Should it be completely removed instead
 or maybe renamed to some other name when it's not related to QEMU any more?
 The driver is still used in QEMU, it's just that the image is memory-mapped into the
 TCX card address space by QEMU where it is located and executed during SBus probe
 (QEMU contains its own copy of QEMU,tcx.bin in the pc-bios directory).
So the driver copy from QEMU is used not this one. They may be identical 
but if I got that right this driver is now only used when not using QEMU 
and with QEMU the one from QEMU is used not this driver. That's what I 
meant by saying this driver is not used in QEMU as this one is not used 
but an identical copy from QEMU.

Since according to its name this is a QEMU driver this looks strange. 
Either the driver is not QEMU specific and would work with real hardware 
so it makes sense to install it when not using QEMU but then maybe it 
could be renamed to not have QEMU in its name or at least put a comment 
somewhere saying that this is a copy of QEMU's driver for the case when 
QEMU is not available.

Or this driver is QEMU specific in which case it does not make sense to 
use it when QEMU is not available.

  Also the
commit message says that the problem is that driver should not be installed
 when TCX hardware is not present so maybe you should test for hardware presence
 instead of QEMU or not which may be the wrong conditional to decide if the driver is
 needed or not if I got this correctly.

 (But it's possible I haven't understood something in which case just disregard
my
 comment.)
 The problem here is that the fallback works by "executing" an in-built copy of
the
 FCode if a valid ROM can't be located during the SBus probe. But with -vga none this
 causes issues because it is the FCode that generates the TCX device node, and so you
 end up with the case where the DT node exists yet the underlying hardware isn't
there
 and so things rapidly start to go downhill...
Is this driver installed on real hardware because OpenBIOS cannot run the 
FCode ROM of the card so this is used as a fallback instead? Or what is a 
valid ROM? With testing for QEMU instead of hardware presence wouldn't you 
get the same problem on real hardware: driver would be installed even 
without card present? (Of course OpenBIOS probably only works with QEMU 
and likely nobody uses it on real hardware so this is theoretical but 
testing for QEMU still does not look like the right thing to do here.) I 
can see two ways:

1. If QEMU always supplies the driver as card ROM which will then be 
installed, when is this fallback necessary at all? Wouldn't it be simpler 
to just remove it altogether? On QEMU it's not needed, other configs 
probably don't work anyway and if they worked you may get the same problem 
as in the QEMU case before this patch when card is missing so getting rid 
of this fallback would simplify code and may only break real hardware case 
which is likely already broken anyway.

or

2. If you think fallback still makes sense and is used by some real case 
then the correct test may be to check if TCX card is present or not, not 
if running on QEMU or not. Maybe you should do this when setting up the 
card during sbus scan. The drivers/sbus.c seems to create device nodes 
when some slots are occupied (? is this correct? are these cards hardwired 
to these slots or what if they are not in the expected slot or something 
else is there). Anyway, you could check if these nodes are existing but 
ROM is not valid to decide to install the driver fallback which seems more 
correct. Or you could try to fall back when mapping the ROM or 
interpreting it has failed.

But I think a proper fix for the problem would be to fix FCode ROM support 
instead of adding fallbacks for everything. This reminds me of the QEMU 
VGA ndrv fallback which is also installed for every VGA device regardless 
if it works with them or not and I had to disable it to try a card ROM or 
driver with the ati-vga device. Along the same lines, that fallback (and 
the fw_cfg hack to download the driver) could also be removed if QEMU set 
the card ROM to the ndrv but the problem with that is that this is the std 
vga device and we would need to patch ROM only when this is used in a ppc 
mac emulation for which there're no functions in QEMU yet. But changing 
QEMU to allow board code to specify card ROM may be cleaner than the 
current way of patching in the ndrv for all vga devices and that would 
allow removing a lot of hacks from OpenBIOS: the fw_cfg download, ndrv 
installation and vga-ndrv? switch and would make it simpler until proper 
FCode support could be implemented some time later.

I hope at least some of what I said makes sense.

Regards,
BALATON Zoltan

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[Mark Cave-Ayland]

      On 13/06/2019 10:30, BALATON Zoltan wrote:

  On Wed, 12 Jun 2019, Mark Cave-Ayland wrote:
  OpenBIOS SPARC32 has always maintained a fallback
option of an internal TCX
 driver if an FCode driver cannot be located on the TCX card itself.

 Since QEMU's TCX adapter has supported FCode payloads for a long time then
 we only require this logic for non-QEMU builds. Otherwise we unintentionally
 install the fallback driver and device node even if the framebuffer card hasn't
 been installed into the machine.

 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;
 ---
 arch/sparc32/init.fs |  7 +++++--
 drivers/sbus.c       | 11 +++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

 diff --git a/arch/sparc32/init.fs b/arch/sparc32/init.fs
 index 814c720..5e8805b 100644
 --- a/arch/sparc32/init.fs
 +++ b/arch/sparc32/init.fs
 @@ -78,6 +78,9 @@ defer ignore-dfault

 \ Load TCX FCode driver blob
 [IFDEF] CONFIG_DRIVER_SBUS
 -  -1 value tcx-driver-fcode
 -  " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +  [IFDEF] CONFIG_QEMU
 +  [ELSE]
 +    -1 value tcx-driver-fcode
 +    " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +  [THEN]
 [THEN]
 
 It looks strange that the driver is called QEMU,tcx but is not used on QEMU. Does it
 make sense to use it when not in QEMU at all? Should it be completely removed instead
 or maybe renamed to some other name when it's not related to QEMU any more?
The driver is still used in QEMU, it's just that the image is memory-mapped into the
TCX card address space by QEMU where it is located and executed during SBus probe
(QEMU contains its own copy of QEMU,tcx.bin in the pc-bios directory).

  Also the commit message says that the problem is that
driver should not be installed
 when TCX hardware is not present so maybe you should test for hardware presence
 instead of QEMU or not which may be the wrong conditional to decide if the driver is
 needed or not if I got this correctly.
 
 (But it's possible I haven't understood something in which case just disregard
my
 comment.)
The problem here is that the fallback works by "executing" an in-built copy of
the
FCode if a valid ROM can't be located during the SBus probe. But with -vga none this
causes issues because it is the FCode that generates the TCX device node, and so you
end up with the case where the DT node exists yet the underlying hardware isn't there
and so things rapidly start to go downhill...


ATB,

Mark.

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[BALATON Zoltan]

      On Wed, 12 Jun 2019, Mark Cave-Ayland wrote:
  OpenBIOS SPARC32 has always maintained a fallback
option of an internal TCX
 driver if an FCode driver cannot be located on the TCX card itself.

 Since QEMU's TCX adapter has supported FCode payloads for a long time then
 we only require this logic for non-QEMU builds. Otherwise we unintentionally
 install the fallback driver and device node even if the framebuffer card hasn't
 been installed into the machine.

 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk&gt;
 ---
 arch/sparc32/init.fs |  7 +++++--
 drivers/sbus.c       | 11 +++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

 diff --git a/arch/sparc32/init.fs b/arch/sparc32/init.fs
 index 814c720..5e8805b 100644
 --- a/arch/sparc32/init.fs
 +++ b/arch/sparc32/init.fs
 @@ -78,6 +78,9 @@ defer ignore-dfault

 \ Load TCX FCode driver blob
 [IFDEF] CONFIG_DRIVER_SBUS
 -  -1 value tcx-driver-fcode
 -  " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +  [IFDEF] CONFIG_QEMU
 +  [ELSE]
 +    -1 value tcx-driver-fcode
 +    " QEMU,tcx.bin" $encode-file to tcx-driver-fcode
 +  [THEN]
 [THEN]
It looks strange that the driver is called QEMU,tcx but is not used on 
QEMU. Does it make sense to use it when not in QEMU at all? Should it be 
completely removed instead or maybe renamed to some other name when it's 
not related to QEMU any more?

Also the commit message says that the problem is that driver should not be 
installed when TCX hardware is not present so maybe you should test for 
hardware presence instead of QEMU or not which may be the wrong 
conditional to decide if the driver is needed or not if I got this 
correctly.

(But it's possible I haven't understood something in which case just 
disregard my comment.)

Regards,
BALATON Zoltan

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[Mark Cave-Ayland]

      On 13/06/2019 21:01, BALATON Zoltan wrote:

  The problem
here is that the fallback works by "executing" an in-built copy of the
 FCode if a valid ROM can't be located during the SBus probe. But with -vga none this
 causes issues because it is the FCode that generates the TCX device node, and so you
 end up with the case where the DT node exists yet the underlying hardware isn't
there
 and so things rapidly start to go downhill...
 
 Is this driver installed on real hardware because OpenBIOS cannot run the FCode ROM
 of the card so this is used as a fallback instead? Or what is a valid ROM? With
 testing for QEMU instead of hardware presence wouldn't you get the same problem on
 real hardware: driver would be installed even without card present? (Of course
 OpenBIOS probably only works with QEMU and likely nobody uses it on real hardware so
 this is theoretical but testing for QEMU still does not look like the right thing to
 do here.) I can see two ways:
Right. I decided to do this for 2 reasons: firstly it would mean no change in
behaviour for other projects that use OpenBIOS (e.g. temlib) and secondly during the
transition then it would allow newer OpenBIOS binaries to be tested on older QEMU
versions.

  1. If QEMU always supplies the driver as card ROM
which will then be installed, when
 is this fallback necessary at all? Wouldn't it be simpler to just remove it
 altogether? On QEMU it's not needed, other configs probably don't work anyway and
if
 they worked you may get the same problem as in the QEMU case before this patch when
 card is missing so getting rid of this fallback would simplify code and may only
 break real hardware case which is likely already broken anyway.
 
 or
 
 2. If you think fallback still makes sense and is used by some real case then the
 correct test may be to check if TCX card is present or not, not if running on QEMU or
 not. Maybe you should do this when setting up the card during sbus scan. The
 drivers/sbus.c seems to create device nodes when some slots are occupied (? is this
 correct? are these cards hardwired to these slots or what if they are not in the
 expected slot or something else is there). Anyway, you could check if these nodes are
 existing but ROM is not valid to decide to install the driver fallback which seems
 more correct. Or you could try to fall back when mapping the ROM or interpreting it
 has failed.
And this patchset implements option 1 :)

  But I think a proper fix for the problem would be to
fix FCode ROM support instead of
 adding fallbacks for everything. This reminds me of the QEMU VGA ndrv fallback which
 is also installed for every VGA device regardless if it works with them or not and I
 had to disable it to try a card ROM or driver with the ati-vga device. Along the same
 lines, that fallback (and the fw_cfg hack to download the driver) could also be
 removed if QEMU set the card ROM to the ndrv but the problem with that is that this
 is the std vga device and we would need to patch ROM only when this is used in a ppc
 mac emulation for which there're no functions in QEMU yet. But changing QEMU to
allow
 board code to specify card ROM may be cleaner than the current way of patching in the
 ndrv for all vga devices and that would allow removing a lot of hacks from OpenBIOS:
 the fw_cfg download, ndrv installation and vga-ndrv? switch and would make it simpler
 until proper FCode support could be implemented some time later.
 
 I hope at least some of what I said makes sense.
Indeed, although this is a little off-topic since SBus used by SPARC is separate from
PCI. Note that you can disable the VGA ndrv driver via -prom-env 'vga-ndrv?=false'
on
the command line if you need it for testing.

It is already possible to provide an option ROM to the VGA device in QEMU, the
slightly tricky part is that you need to add some routines to QEMU to dynamically
create the option ROM by creating a "dummy" option ROM header and then injecting
the
FCode payload. Then it should just be a case of mapping to ROM into the PCI device,
applying a small patch to OpenBIOS and then you should be good.


ATB,

Mark.

[PATCH 1/2] sbus: don't use internal fallback TCX driver with QEMU

[Mark Cave-Ayland]

      On 26/06/2019 21:59, BALATON Zoltan wrote:

  On Wed, 26 Jun 2019, Mark Cave-Ayland wrote:
  On 14/06/2019 21:42, BALATON Zoltan wrote:
  But that's just my view and I don't
really mind either way, so don't take this as
 saying not to apply your patches.
 Thanks for the feedback. Having reflected on this over the past week, I'm still
happy
 with the approach taken (since it's consistent with what I've been doing for a
while
 now) and given that it's coming up to QEMU freeze I'd like to have the fix
included
 in the next release. So this is now pushed to master.
 
 Apropos freeze, is there a chance to include my patch as well:
 

https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/JX...
 
 
 which is needed to run MorphOS on mac99 and have working USB? I have a patched
 OpenBIOS binary on my QEMU Amiga page which I haven't updated for a while so it may
 stop working at some point with newer QEMU releases. So having this in official
 OpenBIOS that comes with QEMU would simplify things for users. Unless there's
 something that breaks with it it's not worse than the other work arounds already in
 OpenBIOS IMO.
From what I can see there is no follow-up patch to deal with fixing up bus-range for
subsequent PCI buses? Whilst I agree with you that OpenBIOS does have a fair number
of workarounds, I don't want to introduce something that deliberately breaks the
conformance of the DT to the specifications when the existing code makes considerable
efforts to do the right thing.


ATB,

Mark.