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@ilande.co.uk
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