This starts from v3 as it contains the previous ATI ROM patch split up into smaller patches and also contains other changes that were submitted as separate patches to keep them together.
After looking at this again I'm not sure about one patch defining register access words as io access which should work for io bars but I'm unsure about memory bars. Does somebody know what these words should do or what they do on other open firmware implementations?
Regards, BALATON Zoltan
BALATON Zoltan (6): Limit binary dump bytes in .properties output Don't add open/close words to unmanaged devices Define register access words drivers/pci: Implement config access words drivers/pci: Fix map-in and map-out words arch/ppc/qemu: Add parse-1hex word for compatibility with Apple OF
arch/ppc/qemu/qemu.fs | 6 +++ drivers/pci.c | 91 ++++++++++++++++++++++++++++++++++------- drivers/pci.fs | 41 +++++++++++++++++++ drivers/vga.fs | 4 +- forth/admin/devices.fs | 7 ++-- forth/device/other.fs | 6 +++ forth/device/pathres.fs | 5 ++- 7 files changed, 140 insertions(+), 20 deletions(-)
Some device trees can contain large chunks of binary data that result in .properties output to get lost in the dump of all binary bytes. Put an upper limit of the bytes dumped. --- forth/admin/devices.fs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/forth/admin/devices.fs b/forth/admin/devices.fs index 38f6ad6..879c271 100644 --- a/forth/admin/devices.fs +++ b/forth/admin/devices.fs @@ -307,15 +307,16 @@
: .p-bytes? ( data len -- 1 | data len 0 ) ." -- " dup . ." : " - swap >r 0 + swap >r dup 100 min 0 begin 2dup > while dup r@ + c@ ( len n ch )
2 0.r space 1+ - repeat - 2drop r> drop 1 + repeat + drop > if ." ..." then + r> drop 1 ;
\ this function tries to heuristically determine the data format
Devices that OpenBIOS does not have a driver for may have an FCode driver that would create the open/close methods but this results in duplicate words due to the empty defaults OpenBIOS adds. These empty defaults are added so that open-dev works (which is needed to run an FCode driver) but to avoid this clash change open-dev to check if open method exists before calling it so we can omit the empty defaults and let an FCode driver install them without getting duplicates. --- drivers/pci.c | 2 +- forth/device/pathres.fs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..3567cc3 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -1863,7 +1863,7 @@ static phandle_t ob_configure_pci_device(const char* parent_path, }
/* if devices haven't supplied open/close words then supply them with simple defaults */ - if (!find_package_method("open", phandle) && !find_package_method("close", phandle)) { + if (pci_dev && !find_package_method("open", phandle) && !find_package_method("close", phandle)) { BIND_NODE_METHODS(phandle, ob_pci_simple_node); }
diff --git a/forth/device/pathres.fs b/forth/device/pathres.fs index a185b95..a95515f 100644 --- a/forth/device/pathres.fs +++ b/forth/device/pathres.fs @@ -423,7 +423,10 @@ constant sinfo.size dup if r@ link-one then 1 = if dup active-package <> my-self >in.interposed ! - r@ invoke-open + " open" my-self >in.device-node @ find-method if + drop + r@ invoke-open + then r@ handle-interposers then active-package!
Define the r[bwl][!@] register access words as io[bwl][!@] for now. I'm not sure this is correct. --- forth/device/other.fs | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/forth/device/other.fs b/forth/device/other.fs index 1bed9b8..0ff34b6 100644 --- a/forth/device/other.fs +++ b/forth/device/other.fs @@ -58,21 +58,27 @@ defer (poke) \ 5.3.7.2 Device-register access
: rb@ ( addr -- byte ) + ioc@ ;
: rw@ ( waddr -- w ) + iow@ ;
: rl@ ( qaddr -- quad ) + iol@ ;
: rb! ( byte addr -- ) + ioc! ;
: rw! ( w waddr -- ) + iow! ;
: rl! ( quad qaddr -- ) + iol! ;
: rx@ ( oaddr - o )
--- drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +} + static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
Fix map-in and map-out words of pci host and bridge devices to return mapped address of BARs --- drivers/pci.c | 35 +++++++++++++++++++---------------- drivers/pci.fs | 41 +++++++++++++++++++++++++++++++++++++++++ drivers/vga.fs | 4 ++-- 3 files changed, 62 insertions(+), 18 deletions(-)
diff --git a/drivers/pci.c b/drivers/pci.c index 6ec3b33..aa8de6b 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -408,20 +408,14 @@ static void ob_pci_unmap(ucell virt, ucell size) { static void ob_pci_bus_map_in(int *idx) { - uint32_t ba; - ucell size; - ucell virt; - - PCI_DPRINTF("ob_pci_bar_map_in idx=%p\n", idx); - - size = POP(); - POP(); - POP(); - ba = POP(); - - virt = ob_pci_map(ba, size); + fword("pci-map-in"); +}
- PUSH(virt); +static void +ob_pci_bus_map_out(int *idx) +{ + /* Should call pci-map-out but nothing to do in it yet */ + fword("2drop"); }
static void @@ -513,7 +507,8 @@ NODE_METHODS(ob_pci_bus_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bus_map_in }, + { "map-in", ob_pci_bus_map_in }, + { "map-out", ob_pci_bus_map_out }, { "config-b@", ob_pci_config_read8 }, { "config-b!", ob_pci_config_write8 }, { "config-w@", ob_pci_config_read16 }, @@ -533,7 +528,14 @@ static void ob_pci_bridge_map_in(int *idx) { /* As per the IEEE-1275 PCI specification, chain up to the parent */ - call_parent_method("pci-map-in"); + call_parent_method("map-in"); +} + +static void +ob_pci_bridge_map_out(int *idx) +{ + /* As per the IEEE-1275 PCI specification, chain up to the parent */ + call_parent_method("map-out"); }
NODE_METHODS(ob_pci_bridge_node) = { @@ -541,7 +543,8 @@ NODE_METHODS(ob_pci_bridge_node) = { { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, - { "pci-map-in", ob_pci_bridge_map_in }, + { "map-in", ob_pci_bridge_map_in }, + { "map-out", ob_pci_bridge_map_out }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, diff --git a/drivers/pci.fs b/drivers/pci.fs index a7b56e1..327c9c9 100644 --- a/drivers/pci.fs +++ b/drivers/pci.fs @@ -37,4 +37,45 @@ then ;
+: pci-map-in ( addr.lo addr.mid addr.hi size -- virt ) + \ Ignore everything but addr.hi and find assigned addr for BAR + drop nip nip + \ Save my-self and set it to same as active package because otherwise + \ decode-phys below seems to be confused. Maybe this should not be needed + my-self swap active-package ihandle>phandle to my-self + " assigned-addresses" active-package get-package-property 0= if + begin + decode-phys ( bar prop prop-len phys.lo phys.mid phys.hi ) + dup 0fffffff and 6 pick 0fffffff and = if + 2drop -rot + decode-int drop decode-int drop 2drop + ( bar addr.lo ) + \ Now translate addr.lo, This may be wrong + \ maybe we should look at parent's ranges instead? + over 18 rshift 3 and 1 = if + " reg" active-package parent get-package-property 0= if + ( bar addr.lo prop prop-len reg.addr reg.size ) + decode-int -rot decode-int 3drop + + + then + then + nip swap to my-self + exit + else + 3drop \ no match, try next + then + \ Drop the size as we don't need it + decode-int drop decode-int drop + dup 0= + until + 3drop + to my-self + -1 \ not found + else + drop + to my-self + -1 \ could not find assigned-addresses + then + ; + [THEN] diff --git a/drivers/vga.fs b/drivers/vga.fs index 53dcff0..f3ffda7 100644 --- a/drivers/vga.fs +++ b/drivers/vga.fs @@ -146,14 +146,14 @@ defer vbe-iow!
: map-fb ( -- ) cfg-bar0 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to fb-addr then ;
: map-mmio ( -- ) cfg-bar2 pci-bar>pci-addr if \ ( pci-addr.lo pci-addr.mid pci-addr.hi size ) - " pci-map-in" $call-parent + " map-in" $call-parent to mmio-addr then ;
--- arch/ppc/qemu/qemu.fs | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/ppc/qemu/qemu.fs b/arch/ppc/qemu/qemu.fs index d683421..107e8ad 100644 --- a/arch/ppc/qemu/qemu.fs +++ b/arch/ppc/qemu/qemu.fs @@ -95,6 +95,12 @@ variable keyboard-phandle 0 keyboard-phandle ! set-defaults ; PREPOST-initializer
+\ ------------------------------------------------------------------------- +\ Mac OF specific words +\ ------------------------------------------------------------------------- + +: parse-1hex parse-hex ; + \ ------------------------------------------------------------------------- \ copyright property handling \ -------------------------------------------------------------------------
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
Define the r[bwl][!@] register access words as io[bwl][!@] for now. I'm not sure this is correct.
After some more testing I think this is not correct but not sure how to implement these correctly, see below for details. Ping for the other patches though that could be merged independently from this one.
So io[bwl][!@] words use functions from include/atch/ppc/io.h which add an isa_io_base offset to the address that maybe only wotks for io BARs but I'm not sure these io words are correct at all as the address should already contain this offset as mac99 has 3 PCI buses and therefore 3 different offsets although QEMU only uses one). Anyway the register access words should work for any address so we can't use the io words here. Moreover these should also byte swap for PCI BARs. SLOF seems to have two implementations which it swaps for little endian versions before running FCode ROMs, see:
https://github.com/aik/SLOF/blob/master/slof/fs/fcode/evaluator.fs#L107 https://github.com/aik/SLOF/blob/master/slof/fs/fcode/tokens.fs#L19
(If these are FCode tokens I don't get when the big endian versions could be used as these should only be called from FCode so not sure this change of rokens is really necessary and we could just do little endian access in these register access words.)
OpenFirmware has a variable called in-little-endian? to set LE or BE:
https://github.com/openbios/openfirmware/blob/master/cpu/ppc/regacc.fth
But it does not seem to set it anywhere but maybe the PPC version wasn't much tested and on LE platforms which were tested this defaults to true.
I think in OpenBIOS we could implement these using in_le32/out_le32 and in_le16/out_le16 functions but before trying to do anything I'd like to get some feedback on what would be an acceptable implementation to avoid doing work which then will be deemed unacceptable and thus wasting my time.
Regards, BALATON Zoltan
Some more sfuff I've discovered:
While debugging these I've added debug printout to these r* words to see how they are called from ATI 7000 ROM. with using [cwl][!@] instead of io[cwl][!@] I get:
0 > 82020040 1 byte-load rl! f2001000 <- 108 rl@ f2001004 ati_mm_read 4 0x8010000 unknown -> 0x0 -> 0 rl! f2001000 <- 110 rl@ f2001004 ati_mm_read 4 0x10010000 unknown -> 0x0 -> 0 rl! 8200023c <- 81000000 ati_mm_write 4 0x23c unknown <- 0x81 rl! 8200033c <- 81000000 ati_mm_write 4 0x33c unknown <- 0x81 rl! 82000148 <- 80ff8100 ati_mm_write 4 0x148 MC_FB_LOCATION <- 0x81ff80 rl! 82000030 <- 5133a340 ati_mm_write 4 0x30 BUS_CNTL <- 0x40a33351 rl! 82000034 <- f ati_mm_write 4 0x34 BUS_CNTL1 <- 0xf000000
The first two accessing f200100[04] addresses try to access the IO BAR1 the rest accessing 8200xxxx uisng the memory mapped register BAR2. Besides the wrong endianness as discussed above the first IO BAR1 access is not working either. This should access ATI regs 0 and 4 (MM_INDEX and MM_DATA to read regs 108 and 110 vie indexed access) but this IO BAR1 is not mapped at f2001000 despite what the assigned-addresses property would suggest so maybe OpenBIOS should also map this BAR but as it goes further anyway maybe this is not needed:
0 > .properties name "pci1002,5159" vendor-id 1002 device-id 5159 revision-id 0 class-code 30000 interrupts 1 min-grant 0 max-latency 0 devsel-speed 0 subsystem-vendor-id 1af4 subsystem-id 1100 cache-line-size 0 assigned-addresses 42007010 00000000 81000000 00000000 01000000 01007014 00000000 00001000 00000000 00000100 02007018 00000000 82000000 00000000 00004000 reg 00007000 00000000 00000000 00000000 00000000 42007010 00000000 00000000 00000000 01000000 01007014 00000000 00000000 00000000 00000100 02007018 00000000 00000000 00000000 00004000
bus: pci.2 type PCI dev: ati-vga, id "" vgamem_mb = 16 (0x10) model = "rv100" x-device-id = 20825 (0x5159) guest_hwcursor = false addr = 0e.0 romfile = "ati-7000.rom" romsize = 131072 (0x20000) rombar = 1 (0x1) multifunction = false x-pcie-lnksta-dllla = true x-pcie-extcap-init = true failover_pair_id = "" acpi-index = 0 (0x0) class VGA controller, addr 00:0e.0, pci id 1002:5159 (sub 1af4:1100) bar 0: mem at 0x81000000 [0x81ffffff] bar 1: i/o at 0xffffffffffffffff [0xfe] bar 2: mem at 0x82000000 [0x82003fff] bar 6: mem at 0x82020000 [0x8203ffff]
memory-region: unin-pci-mmio 0000000000000000-00000000ffffffff (prio 0, i/o): unin-pci-mmio 00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem 0000000080000000-000000008007ffff (prio 1, i/o): macio 0000000080000050-000000008000007f (prio 0, i/o): gpio 0000000080008000-0000000080008fff (prio 0, i/o): dbdma 0000000080012000-00000000800120ff (prio 0, i/o): escc-legacy 0000000080012000-0000000080012001 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000000-0000000000000001 0000000080012002-0000000080012003 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000020-0000000000000021 0000000080012004-0000000080012005 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000010-0000000000000011 0000000080012006-0000000080012007 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000030-0000000000000031 0000000080012008-0000000080012009 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000040-0000000000000041 000000008001200a-000000008001200b (prio 0, i/o): alias escc-legacy-port @escc 0000000000000050-0000000000000051 0000000080012080-0000000080012081 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000080-0000000000000081 0000000080012090-0000000080012091 (prio 0, i/o): alias escc-legacy-port @escc 0000000000000090-0000000000000091 00000000800120a0-00000000800120a1 (prio 0, i/o): alias escc-legacy-port @escc 00000000000000a0-00000000000000a1 00000000800120b0-00000000800120b1 (prio 0, i/o): alias escc-legacy-port @escc 00000000000000b0-00000000000000b1 0000000080013000-000000008001303f (prio 0, i/o): escc 0000000080015000-0000000080015fff (prio 0, i/o): timer 0000000080016000-0000000080017fff (prio 0, i/o): via-pmu 0000000080020000-0000000080020fff (prio 0, i/o): pmac-ide 0000000080021000-0000000080021fff (prio 0, i/o): pmac-ide 0000000080040000-000000008007ffff (prio 0, i/o): openpic 0000000080040000-00000000800410ef (prio 0, i/o): glb 00000000800410f0-000000008004130f (prio 0, i/o): tmr 0000000080050000-0000000080051fff (prio 0, i/o): src 0000000080060000-000000008007f0ff (prio 0, i/o): cpu 0000000080080000-00000000800800ff (prio 1, i/o): ohci 0000000081000000-0000000081ffffff (prio 1, ram): vga.vram 0000000082000000-0000000082003fff (prio 1, i/o): ati.mmregs 0000000082020000-000000008203ffff (prio 1, rom): ati-vga.rom
memory-region: system 0000000000000000-ffffffffffffffff (prio 0, i/o): system 0000000000000000-0000000007ffffff (prio 0, ram): ppc_core99.ram 0000000080000000-000000008fffffff (prio 0, i/o): alias unin-pci-hole @unin-pci-mmio 0000000080000000-000000008fffffff 00000000f0000510-00000000f0000511 (prio 0, i/o): fwcfg.ctl 00000000f0000512-00000000f0000512 (prio 0, i/o): fwcfg.data 00000000f0800000-00000000f0800fff (prio 0, i/o): unin-agp-conf-idx 00000000f0c00000-00000000f0c00fff (prio 0, i/o): unin-agp-conf-data 00000000f2000000-00000000f27fffff (prio 0, i/o): unin-pci-isa-mmio 00000000f20001ce-00000000f20001d1 (prio 0, i/o): vbe 00000000f20003b4-00000000f20003b5 (prio 0, i/o): vga 00000000f20003ba-00000000f20003ba (prio 0, i/o): vga 00000000f20003c0-00000000f20003cf (prio 0, i/o): vga 00000000f20003d4-00000000f20003d5 (prio 0, i/o): vga 00000000f20003da-00000000f20003da (prio 0, i/o): vga 00000000f2800000-00000000f2800fff (prio 0, i/o): unin-pci-conf-idx 00000000f2c00000-00000000f2c00fff (prio 0, i/o): unin-pci-conf-data 00000000f4800000-00000000f4800fff (prio 0, i/o): unin-pci-conf-idx 00000000f4c00000-00000000f4c00fff (prio 0, i/o): unin-pci-conf-data 00000000f8000000-00000000f8000fff (prio 0, i/o): unin 00000000fff00000-00000000ffffffff (prio 0, rom): ppc_core99.bios 00000000fff04000-00000000fff07fff (prio 0, i/o): macio-nvram
I've also enabled PCI_DEBUG in OpenBIOS to see what it does with the BARs but the output did not make much sense to me, inlcuding it here in case it helps; not sure why it did not map BAR1:
Cannot manage 'VGA controller' PCI device type 'display': 1002 5159 (3 0 0) 0:e.0 - 1002:5159 - /pci@f2000000/pci1002,5159 - ob_pci_encode_unit space=0 dev=14 fn=0 buf=e ob_pci_decode_unit idx=fff62198 ob_pci_decode_unit idx=fff62198 addr=00000000 00000000 00007000 changing mem_base from 0x80090000 to 0x82000000 Configuring BARs for /pci@f2000000/pci1002,5159: reloc 0x81000000 omask 0x8 io_base 0x1000 mem_base 0x82000000 size 0x1000000 changing io_base from 0x1000 to 0x1100 Configuring BARs for /pci@f2000000/pci1002,5159: reloc 0x1000 omask 0x1 io_base 0x1100 mem_base 0x82000000 size 0x100 changing mem_base from 0x82000000 to 0x82010000 Configuring BARs for /pci@f2000000/pci1002,5159: reloc 0x82000000 omask 0x0 io_base 0x1100 mem_base 0x82010000 size 0x10000 changing mem_base from 0x82010000 to 0x82040000 Configuring BARs for /pci@f2000000/pci1002,5159: reloc 0x82020000 omask 0x1 io_base 0x1100 mem_base 0x82040000 size 0x20000 *** missing pci_dev
ob_pci_encode_unit space=0 dev=14 fn=0 buf=e
=== CHANGED === package path old=/pci@f2000000/pci1002,5159 new=/pci@f2000000/pci1002,5159@e pci_set_reg reg 00007000 00000000 00000000 00000000 00000000 42007010 00000000 00000000 00000000 01000000 01007014 00000000 0000000
forth/device/other.fs | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/forth/device/other.fs b/forth/device/other.fs index 1bed9b8..0ff34b6 100644 --- a/forth/device/other.fs +++ b/forth/device/other.fs @@ -58,21 +58,27 @@ defer (poke) \ 5.3.7.2 Device-register access
: rb@ ( addr -- byte )
- ioc@ ;
: rw@ ( waddr -- w )
- iow@ ;
: rl@ ( qaddr -- quad )
- iol@ ;
: rb! ( byte addr -- )
- ioc! ;
: rw! ( w waddr -- )
- iow! ;
: rl! ( quad qaddr -- )
- iol! ;
: rx@ ( oaddr - o )
Hi!
On Sun, Oct 09, 2022 at 09:41:21PM +0200, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
Define the r[bwl][!@] register access words as io[bwl][!@] for now. I'm not sure this is correct.
https://github.com/aik/SLOF/blob/master/slof/fs/fcode/evaluator.fs#L107 https://github.com/aik/SLOF/blob/master/slof/fs/fcode/tokens.fs#L19
(If these are FCode tokens I don't get when the big endian versions could be used as these should only be called from FCode so not sure this change of rokens is really necessary and we could just do little endian access in these register access words.)
And the user interface rb@ etc. should use get-token (or equivalent):
rb@ (user interface) ( addr -- byte ) Fetch a byte from device register at addr.
Compilation: ( -- ) Perform the equivalent of the phrase: h# 230 get-token if execute else compile, then
Interpretation: ( addr -- byte ) Perform the equivalent of the phrase: h# 230 get-token drop execute
NOTE— A bus device can substitute (see set-token) a bus-specific implementation of rb@ for use by its children. This is sometimes necessary to correctly implement its semantics with respect to bit-order and write-buffer flushing. The given user interface semantics of rb@ ensure that such substitutions are visible at the user interface level.
This is one of the historical weirdosities of OF :-)
Segher
On Mon, 10 Oct 2022, Segher Boessenkool wrote:
Hi!
On Sun, Oct 09, 2022 at 09:41:21PM +0200, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
Define the r[bwl][!@] register access words as io[bwl][!@] for now. I'm not sure this is correct.
https://github.com/aik/SLOF/blob/master/slof/fs/fcode/evaluator.fs#L107 https://github.com/aik/SLOF/blob/master/slof/fs/fcode/tokens.fs#L19
(If these are FCode tokens I don't get when the big endian versions could be used as these should only be called from FCode so not sure this change of rokens is really necessary and we could just do little endian access in these register access words.)
And the user interface rb@ etc. should use get-token (or equivalent):
rb@ (user interface) ( addr -- byte ) Fetch a byte from device register at addr.
Compilation: ( -- ) Perform the equivalent of the phrase: h# 230 get-token if execute else compile, then
Interpretation: ( addr -- byte ) Perform the equivalent of the phrase: h# 230 get-token drop execute
NOTE— A bus device can substitute (see set-token) a bus-specific implementation of rb@ for use by its children. This is sometimes necessary to correctly implement its semantics with respect to bit-order and write-buffer flushing. The given user interface semantics of rb@ ensure that such substitutions are visible at the user interface level.
OK so what does all that mean? We could do what you say in openbios/forth/device/other.fs for r[bwl][!@] words. There are already rx[!@] words that seem to do that so I could just copy that with modified token numbers for the other words. But where to assign the tokens for PCI then? Or is it OK for now to just add little endian versions which should work for FCode ROMs and nothing else uses these words otherwise as they are currently unimplemented.
Regards, BALATON Zoltan
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Ping? This was just ignored without any comment so far.
Regards, BALATON Zoltan
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- uint8_t val = pci_config_read8(addr, hi & 0xff);
- PUSH(val);
+}
+static void +ob_pci_config_write8(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- cell val = POP();
- pci_config_write8(addr, hi & 0xff, val);
+}
+static void +ob_pci_config_read16(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- uint16_t val = pci_config_read16(addr, hi & 0xff);
- PUSH(val);
+}
+static void +ob_pci_config_write16(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- cell val = POP();
- pci_config_write16(addr, hi & 0xff, val);
+}
+static void +ob_pci_config_read32(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- uint32_t val = pci_config_read32(addr, hi & 0xff);
- PUSH(val);
+}
+static void +ob_pci_config_write32(int *idx) +{
- cell hi = POP();
- pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
- cell val = POP();
- pci_config_write32(addr, hi & 0xff, val);
+}
static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in },
- { "config-b@", ob_pci_config_read8 },
- { "config-b!", ob_pci_config_write8 },
- { "config-w@", ob_pci_config_read16 },
- { "config-w!", ob_pci_config_write16 },
- { "config-l@", ob_pci_config_read32 },
- { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
On 17/01/2023 18:15, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Ping? This was just ignored without any comment so far.
Regards, BALATON Zoltan
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +}
static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I gave this patch a test, and whilst it basically works it seems to be missing some logic around PCI bridges. For example to read the PCI vendor ID on PPC:
0 > cd /pci@80000000/mac-io@10 ok 0 > " /pci@80000000/mac-io@10" open-dev to my-self ok 0 > my-space " config-w@" $call-parent ok 1 > u. 106b ok
and on SPARC64:
0 > cd /pci@1fe,0/pci@1,1/QEMU,VGA@2 ok 0 > " /pci@1fe,0/pci@1,1/QEMU,VGA@2" open-dev to my-self ok 0 > my-space " config-w@" $call-parent undefined method.
I think this needs fixing since assuming the QEMU New World Mac machine moves towards a G4 AGP machine, re-introducing the DEC PCI bridge present in real Macs would cause this to break again.
ATB,
Mark.
On Thu, 26 Jan 2023, Mark Cave-Ayland wrote:
On 17/01/2023 18:15, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Ping? This was just ignored without any comment so far.
Regards, BALATON Zoltan
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +}
static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I gave this patch a test, and whilst it basically works it seems to be missing some logic around PCI bridges. For example to read the PCI vendor ID on PPC:
0 > cd /pci@80000000/mac-io@10 ok 0 > " /pci@80000000/mac-io@10" open-dev to my-self ok 0 > my-space " config-w@" $call-parent ok 1 > u. 106b ok
and on SPARC64:
0 > cd /pci@1fe,0/pci@1,1/QEMU,VGA@2 ok 0 > " /pci@1fe,0/pci@1,1/QEMU,VGA@2" open-dev to my-self ok 0 > my-space " config-w@" $call-parent undefined method.
I think this needs fixing since assuming the QEMU New World Mac machine moves towards a G4 AGP machine, re-introducing the DEC PCI bridge present in real Macs would cause this to break again.
I've handled that for the map-in and map-out words in this patch:
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/message/DRXS...
so similar should work for config-* as well but I wonder if there's a more generic way for a bridge to just try to forward every word it does not know about to it's parent so we don't add these one by one. Any ideas?
Regards, BALATON Zoltan
On 26/01/2023 23:43, BALATON Zoltan wrote:
On Thu, 26 Jan 2023, Mark Cave-Ayland wrote:
On 17/01/2023 18:15, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Ping? This was just ignored without any comment so far.
Regards, BALATON Zoltan
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +}
static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I gave this patch a test, and whilst it basically works it seems to be missing some logic around PCI bridges. For example to read the PCI vendor ID on PPC:
0 > cd /pci@80000000/mac-io@10 ok 0 > " /pci@80000000/mac-io@10" open-dev to my-self ok 0 > my-space " config-w@" $call-parent ok 1 > u. 106b ok
and on SPARC64:
0 > cd /pci@1fe,0/pci@1,1/QEMU,VGA@2 ok 0 > " /pci@1fe,0/pci@1,1/QEMU,VGA@2" open-dev to my-self ok 0 > my-space " config-w@" $call-parent undefined method.
I think this needs fixing since assuming the QEMU New World Mac machine moves towards a G4 AGP machine, re-introducing the DEC PCI bridge present in real Macs would cause this to break again.
I've handled that for the map-in and map-out words in this patch:
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/message/DRXS...
so similar should work for config-* as well but I wonder if there's a more generic way for a bridge to just try to forward every word it does not know about to it's parent so we don't add these one by one. Any ideas?
From my experience that's quite a common pattern to invoke $call-parent up to the next bus node (I see plenty of examples in Sun's OpenFirmware) so I think it's the right way to do things in a Forth-based firmware environment.
ATB,
Mark.
On Sat, 28 Jan 2023, Mark Cave-Ayland wrote:
On 26/01/2023 23:43, BALATON Zoltan wrote:
On Thu, 26 Jan 2023, Mark Cave-Ayland wrote:
On 17/01/2023 18:15, BALATON Zoltan wrote:
On Sun, 25 Sep 2022, BALATON Zoltan wrote:
drivers/pci.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Ping? This was just ignored without any comment so far.
Regards, BALATON Zoltan
diff --git a/drivers/pci.c b/drivers/pci.c index 3567cc3..6ec3b33 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); }
+static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +}
+static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +}
+static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +}
static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I gave this patch a test, and whilst it basically works it seems to be missing some logic around PCI bridges. For example to read the PCI vendor ID on PPC:
0 > cd /pci@80000000/mac-io@10 ok 0 > " /pci@80000000/mac-io@10" open-dev to my-self ok 0 > my-space " config-w@" $call-parent ok 1 > u. 106b ok
and on SPARC64:
0 > cd /pci@1fe,0/pci@1,1/QEMU,VGA@2 ok 0 > " /pci@1fe,0/pci@1,1/QEMU,VGA@2" open-dev to my-self ok 0 > my-space " config-w@" $call-parent undefined method.
I think this needs fixing since assuming the QEMU New World Mac machine moves towards a G4 AGP machine, re-introducing the DEC PCI bridge present in real Macs would cause this to break again.
I've handled that for the map-in and map-out words in this patch:
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/message/DRXS...
so similar should work for config-* as well but I wonder if there's a more generic way for a bridge to just try to forward every word it does not know about to it's parent so we don't add these one by one. Any ideas?
From my experience that's quite a common pattern to invoke $call-parent up to the next bus node (I see plenty of examples in Sun's OpenFirmware) so I think it's the right way to do things in a Forth-based firmware environment.
I could not find an easier way so I've sent a v4 adding more of these one line functions for now. But I think eventually these should be cleaned up and replaced with one common function to make this file less cluttered. There's an is-call-parent word in Forth to define such methods but it depends on passing the method name as a string so it's just a shortcut for avoiding writing the method name twice (which some places do even in the same file using is-call-parent). To do that in C the method function should either get the name as an argument or be able to find its own name but I could not figure out how that could be done. So let's go with the ugly version that works and we may clean that up later.
Regards, BALATON Zoltan
-
BALATON Zoltan -
Mark Cave-Ayland -
Segher Boessenkool