On 20/07/2019 17:36, BALATON Zoltan wrote:
On Sat, 20 Jul 2019, Mark Cave-Ayland wrote:
On 19/07/2019 15:57, BALATON Zoltan wrote:
Why is this patch not in master yet?
Well sometimes I have been known to have a life outside of QEMU/OpenBIOS... but
Sure, although we only know about this side of your life here :-). I did not mean to complain about you being late with this just asked what's the reason this is a separate patch and not merged yet. It was forwarded to me by Jd and I though this was done when he was trying to run the NVidia ROM a few months ago so I thought it's an older patch that could have been merged unless there's some problem with it that kept it out of master.
really quickly there are 2 reasons: 1) it's a bit of a hack (the real solution is to build the DT in order rather than constantly switching around active package/current instance, otherwise your instance variables can be initialised incorrectly) and 2)
OK, is that something getting the device tree from QEMU as FDT then unflattening it to construct the OF device-tree would help with or is it only about the pci devices part of the tree? Still I think getting a complete tree from QEMU and only adding some properties as SLOF does is better then the current situation where OpenBIOS needs knowledge about every change in QEMU so I think the way to avoid this is to do what SLOF does. This is more work once but then helps to get rid of a lot of hacks and make it less work later because a lot of things can be done in QEMU alone not requiring an OpenBIOS update as well so would make OpenBIOS more stable.
As I mentioned in my previous email I am currently working on a patchset to implement
- properly for PCI devices which I hope to post to the list soon.
Is there more info available on this? If you don't have time to work on it please share the info with the list at least so someone else could have a chance to help (although I get the impression there's no one else who cares about OpenBIOS besides you here).
The patchset I've just posted is the proper fix for this - please test and let me know how you get on.
Great work, thanks! I've retried with this patch set instead of the openbios-pci-set-args.patch I was using before but it does not seem to fix that, neither the map-in problem. I've also found that these new patches break vga-ndrv?=false:
Welcome to OpenBIOS v1.1 built on Jul 20 2019 16:18
0 > printenv name "options" boot-args "" boot-device "hd:,\:tbxi hd:,\ppc\bootinfo.txt hd:,%BOOT" use-generic? "false" boot-script "" boot-screen "" vga-ndrv? "false" [...]
0 > cd . .. .. .dev /pci@f2000000/ATY ok 0 > .properties name "ATY" vendor-id 1002 device-id 5046 revision-id 0 class-code 30000 min-grant 0 max-latency 0 devsel-speed 0 subsystem-vendor-id 1af4 subsystem-id 1100 cache-line-size 0 device_type "display" model "ATY Rage128" compatible "VGA" assigned-addresses -- 3c : 42 00 78 10 00 00 00 00 81 00 00 00 00 00 00 00 01 00 00 00 01 00 78 14 00 00 00 00 00 00 10 00 00 00 00 00 00 00 01 00 02 00 78 18 00 00 00 00 82 00 00 00 00 00 00 00 00 00 40 00 reg 00007800 00000000 00000000 00000000 00000000 42007810 00000000 00000000 00000000 01000000 01007814 00000000 00000000 00000000 00000100 02007818 00000000 00000000 00000000 00004000 width 320 height 258 depth 20 linebytes c80 driver,AAPL,MacOS,PowerPC -- 4941 : 4a 6f 79 21 70 65 66 66 70 77 70 63 00
^^^ This driver should not be here.
Ah I see. This is because vga_config_cb() gets called not just for a specific device/vendor id but for any PCI display device. It might be worth renaming it to bochs_config_cb() and tying it down to the QEMU device/vendor id so that it doesn't interfere with external FCode ROMs.
I've decorated the map_in function with some more debug messages now:
0 > "/pci@f2000000/ATY" open-dev to my-self
ob_pci_bar_map_in idx=1fc5ac54 ob_pci_map: ba=0x82000000 size=16384 ob_pci_map: after decode flags=0x0 space_code=0x2 mask=0xf pci_bus_addr_to_host_addr: space=0x2 ba=0x82000000 ob_pci_map: phys=0x82000000 size=16384 ob_pci_bar_map_in idx=1fc5ac54 ob_pci_map: ba=0x81000000 size=16777216 ob_pci_map: after decode flags=0x0 space_code=0x2 mask=0xf pci_bus_addr_to_host_addr: space=0x2 ba=0x81000000 ob_pci_map: phys=0x81000000 size=16777216
ok 0 > my-unit ok 3 > . 7800 ok 2 > . 0 ok 1 > . 0 ok 0 > my-space . 0 ok
^ this is still wrong, should be 7800
0 > my-address . . 0 0 ok 0 > my-address h# 1000014 h# 7800 + h# 100 ok 4 > .s <4> 0 0 1007814 100 ok 4 > " map-in" $call-parent
ob_pci_bar_map_in idx=1fc5ac54 ob_pci_map: ba=0x00000000 size=256 ob_pci_map: after decode flags=0x0 space_code=0x2 mask=0xf pci_bus_addr_to_host_addr: space=0x2 ba=0x00000000 ob_pci_map: phys=0x00000000 size=256
ok
I had a fresh read of the relevant parts of the IEEE-1275 specification this morning and the fallback behaviour related to the reg property I was looking at is only related to my-unit, i.e. after the package has been created. So yes, even with the new code we still need an explicit set-args in there since this is the only thing that can set the probe address which is used for my-space and my-address.
Attached is a lightly tested patch to be applied on top of the existing patchset which uses the instance chain to encode the unit number correctly before invoking set-args.
ATB,
Mark.