6 Aug
2019
6 Aug
'19
6:15 a.m.
Signed-off-by: BALATON Zoltan <balaton(a)eik.bme.hu>
---
drivers/pci.c | 51 +++++++++++++++++++++++++++++++++++++--------------
drivers/pci.fs | 41 +++++++++++++++++++++++++++++++++++++++++
drivers/vga.fs | 4 ++--
forth/device/other.fs | 6 ++++++
4 files changed, 86 insertions(+), 16 deletions(-)
This is on top of Mark's previous patches from the openbios mailing
list applied to openbios master:
9a32ed9 Implement missing words for the ATI FCode ROM to test ati-vga emulation
b90ac41 virtio: use instance value to initialise C instance parameter
ac467a3 lsi: use instance value to hold sd_private_t pointer
177684a pc_kbd: use instance value to initialise C instance parameter
d5fdb98 pc_serial: remove separate init word
7c20840 pci: call set-args before configuring PCI device nodes
f37460e pci: remove explicit setting of my-self from PCI devices
b548924 pci: remove explicit find-device from PCI devices
b1ca78f x86: set active package and current instance to root device node before probe
6222a47 SPARC64: set active package and current instance to root device node before probe
5cd41e5 ppc: set active package and current instance to root device node before probe
e68504f ppc: move New World uninorth and nvram device node creation to the root device
86aa740 nvram: ensure that NVRAM configuration is separate from NVRAM node creation
e959bfd libopenbios: remove REGISTER_NAMED_NODE and REGISTER_NAMED_NODE_PHANDLE macros
abc4553 pci: remove ob_pci_initialize() and ob_pci_empty_node
0a63c74 pci: convert to use BIND_NODE_METHODS() macro
57ff6ea virtio: convert to use BIND_NODE_METHODS() macro
ea3a8eb lsi: convert to use BIND_NODE_METHODS() macro
32ff529 lsi: don't change active package when setting device alias
f63f5ee nvram: convert to use BIND_NODE_METHODS() macro
03c53be usbhid: convert to use BIND_NODE_METHODS() macro
365b2c1 pmu: convert to use BIND_NODE_METHODS() macro
60125f5 macio: convert to use BIND_NODE_METHODS() macro
7d23b31 cuda: convert to use BIND_NODE_METHODS() macro
305d629 escc: convert to use BIND_NODE_METHODS() macro
4a0aea1 adb: convert to use BIND_NODE_METHODS() macro
0081f71 ide: convert to use BIND_NODE_METHODS() macro
79a241c floppy: convert to use BIND_NODE_METHODS() macro
01ea2e4 pc_serial: convert to use BIND_NODE_METHODS() macro
9eb09a9 pc_kbd: convert to use BIND_NODE_METHODS() macro
8945ab4 libopenbios: introduce BIND_NODE_METHODS() macro
e9491ea admin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses property
c79e0ec (origin/master, origin/HEAD) SPARC64: use serial console when QEMU is launched
with -vga none
admin/devices.fs: Format assigned-addresses property is
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/thread/WHL7…
and additionally to the above I also have another patch from Mark that
I can't find now in the list archives even though I remember I've replyed
to it and had a discussion about having 3 or 2 cases if-else statement.
diff --git a/drivers/pci.c b/drivers/pci.c
index 4bc02c9..eb5f7d3 100644
--- a/drivers/pci.c
+++ b/drivers/pci.c
@@ -408,20 +408,32 @@ 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);
+ fword("pci-map-in");
+}
- size = POP();
- POP();
- POP();
- ba = POP();
+static void
+ob_pci_bus_map_out(int *idx)
+{
+ /* Should call pci-map-out but nothing to do in it yet */
+ fword("2drop");
+}
- virt = ob_pci_map(ba, size);
+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);
+}
- PUSH(virt);
+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 & 0xff);
}
static void
@@ -459,7 +471,10 @@ 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 },
{ "dma-alloc", ob_pci_dma_alloc },
{ "dma-free", ob_pci_dma_free },
{ "dma-map-in", ob_pci_dma_map_in },
@@ -473,7 +488,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) = {
@@ -481,7 +503,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
;
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 )
--
2.13.7
26 Oct
26 Oct
9:41 p.m.
On 05/08/2019 21:15, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan
<balaton(a)eik.bme.hu>
---
drivers/pci.c | 51 +++++++++++++++++++++++++++++++++++++--------------
drivers/pci.fs | 41 +++++++++++++++++++++++++++++++++++++++++
drivers/vga.fs | 4 ++--
forth/device/other.fs | 6 ++++++
4 files changed, 86 insertions(+), 16 deletions(-)
This is on top of Mark's previous patches from the openbios mailing
list applied to openbios master:
9a32ed9 Implement missing words for the ATI FCode ROM to test ati-vga emulation
b90ac41 virtio: use instance value to initialise C instance parameter
ac467a3 lsi: use instance value to hold sd_private_t pointer
177684a pc_kbd: use instance value to initialise C instance parameter
d5fdb98 pc_serial: remove separate init word
7c20840 pci: call set-args before configuring PCI device nodes
f37460e pci: remove explicit setting of my-self from PCI devices
b548924 pci: remove explicit find-device from PCI devices
b1ca78f x86: set active package and current instance to root device node before probe
6222a47 SPARC64: set active package and current instance to root device node before
probe
5cd41e5 ppc: set active package and current instance to root device node before probe
e68504f ppc: move New World uninorth and nvram device node creation to the root device
86aa740 nvram: ensure that NVRAM configuration is separate from NVRAM node creation
e959bfd libopenbios: remove REGISTER_NAMED_NODE and REGISTER_NAMED_NODE_PHANDLE macros
abc4553 pci: remove ob_pci_initialize() and ob_pci_empty_node
0a63c74 pci: convert to use BIND_NODE_METHODS() macro
57ff6ea virtio: convert to use BIND_NODE_METHODS() macro
ea3a8eb lsi: convert to use BIND_NODE_METHODS() macro
32ff529 lsi: don't change active package when setting device alias
f63f5ee nvram: convert to use BIND_NODE_METHODS() macro
03c53be usbhid: convert to use BIND_NODE_METHODS() macro
365b2c1 pmu: convert to use BIND_NODE_METHODS() macro
60125f5 macio: convert to use BIND_NODE_METHODS() macro
7d23b31 cuda: convert to use BIND_NODE_METHODS() macro
305d629 escc: convert to use BIND_NODE_METHODS() macro
4a0aea1 adb: convert to use BIND_NODE_METHODS() macro
0081f71 ide: convert to use BIND_NODE_METHODS() macro
79a241c floppy: convert to use BIND_NODE_METHODS() macro
01ea2e4 pc_serial: convert to use BIND_NODE_METHODS() macro
9eb09a9 pc_kbd: convert to use BIND_NODE_METHODS() macro
8945ab4 libopenbios: introduce BIND_NODE_METHODS() macro
e9491ea admin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses propertyadmin/devices.fs: Format
assigned-addresses propertyadmin/devices.fs: Format assigned-addresses
propertyadmin/devices.fs: Format assigned-addresses property
c79e0ec (origin/master, origin/HEAD) SPARC64: use serial console when QEMU is launched
with -vga none
admin/devices.fs: Format assigned-addresses property is
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/thread/WHL7…
and additionally to the above I also have another patch from Mark that
I can't find now in the list archives even though I remember I've replyed
to it and had a discussion about having 3 or 2 cases if-else statement.
diff --git a/drivers/pci.c b/drivers/pci.c
index 4bc02c9..eb5f7d3 100644
--- a/drivers/pci.c
+++ b/drivers/pci.c
@@ -408,20 +408,32 @@ 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);
+ fword("pci-map-in");
+}
- size = POP();
- POP();
- POP();
- ba = POP();
+static void
+ob_pci_bus_map_out(int *idx)
+{
+ /* Should call pci-map-out but nothing to do in it yet */
+ fword("2drop");
+}
- virt = ob_pci_map(ba, size);
+static void
+ob_pci_config_read8(int *idx)
+{
+ cell hi = POP();
Should this not be ucell?
+ 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);
+}
- PUSH(virt);
+static void
+ob_pci_config_write8(int *idx)
+{
+ cell hi = POP();
and here too?
+ pci_addr addr = PCI_ADDR(PCI_BUS(hi),
PCI_DEV(hi), PCI_FN(hi));
+ cell val = POP();
+ pci_config_write8(addr, hi & 0xff, val & 0xff);
}
What about the other config-* words? Given that you're done the byte implementation
then you might as well do the same for the other accesses.
static void
@@ -459,7 +471,10 @@ 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 },
{ "dma-alloc", ob_pci_dma_alloc },
{ "dma-free", ob_pci_dma_free },
{ "dma-map-in", ob_pci_dma_map_in },
@@ -473,7 +488,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) = {
@@ -481,7 +503,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]
A couple of comments here: using Forth here makes this the only PCI word that is
implemented in Forth rather than C which tends to make debugging a lot more
difficult. I'd much prefer to have the implementation in one place and then consider
a conversion later if required.
Secondly this doesn't seem to be correct in that it doesn't take the physical
address
and map it to a virtual address and so it assumes this area has been pre-mapped. Is
this another piece of code borrowed from SLOF, since it runs in real mode rather than
virtual mode?
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
;
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 )
I've had a read of the specifications and it's not very clear what type of
addresses
these are supposed to take - are they PCI addresses or virtual addresses? Can you
tell from your ATI FCode ROM?
The other issue is that I'm not convinced that these words belong in this file, since
you're overriding the default implementations with ones that assume little-endian
access. I think the correct solution here is implement these words as part of the PCI
bus node and use set-token to override the FCode implementation as detailed in the
IEEE-1275 specification.
ATB,
Mark.
27 Oct
27 Oct
8:04 a.m.
Hello,
Thanks for the review.
On Sat, 26 Oct 2019, Mark Cave-Ayland wrote:
Those were not needed for the ROM I've tried to run so I did not bother in
this first version that I've submitted for comments and to let others also
test with the ROM. I can look at those as well when I get some time for it
unless someone is willing to contribute them later based on this example.
Since this accesses properties this seemed to be easier in Forth than C I
think but can't remember the details now.
I can't but here are the relevant parts in case someone can tell:
1316: b(lit) 0x100
1321: new-token 0x88b
1324: b(value)
5814: new-token select-reg-indexed \ 0x990
5817: b(:)
5818: dup
5819: 3
5820: invert
5821: and
5822: var_mapped_mmio_base
5824: const_REG_MM_INDEX
5826: +
5827: rl!
5829: 3
5830: and
5831: var_mapped_mmio_base
5833: const_REG_MM_DATA
5835: +
5836: +
5837: b(;)
5838: new-token mapped-io-base+ \ 0x991
5841: b(:)
5842: var_mapped_mmio_base
5844: +
5845: b(;)
5846: new-token select-reg \ 0x992, uses index/data reg until regs are memory mapped
5849: b(:)
5850: dup
5851: (unnamed-fcode) [0x88b]
5853: <
5854: var_mapped_ram_base
5856: 0<>
5857: or
5858: b?branch 0x0008 ( dest = 5867 )
5861: mapped-io-base+
5863: bbranch 0x0006 ( dest = 5870 )
5866: b(>resolve)
5867: select-reg-indexed
5869: b(>resolve)
5870: b(;)
5871: new-token ati-reg-l@ \ 0x993
5874: b(:)
5875: select-reg
5877: rl@
5879: b(;)
5880: new-token ati-reg-w@ \ 0x994
5883: b(:)
5884: select-reg
5886: rw@
5888: b(;)
5889: new-token ati-reg-b@ \ 0x995
5892: b(:)
5893: select-reg
5895: rb@
5897: b(;)
5898: new-token ati-reg-l! \ 0x996
5901: b(:)
5902: select-reg
5904: rl!
5906: b(;)
5907: new-token ati-reg-w! \ 0x997
5910: b(:)
5911: select-reg
5913: rw!
5915: b(;)
5916: new-token ati-reg-b! \ 0x998
5919: b(:)
5920: select-reg
5922: rb!
5924: b(;)
I think it's memory address but not sure. Also this is on PPC where
everything is MMIO so not sure what it would do on other platforms.
On 05/08/2019 21:15, BALATON Zoltan wrote:
Not sure, I must have copied these from somewhere but I can't remember
from where. Maybe from ob_pci_encode_unit? I can change it to ucell if
that makes more sense, I can't tell.
diff --git a/drivers/pci.c b/drivers/pci.c
index 4bc02c9..eb5f7d3 100644
--- a/drivers/pci.c
+++ b/drivers/pci.c
@@ -408,20 +408,32 @@ 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);
+ fword("pci-map-in");
+}
- size = POP();
- POP();
- POP();
- ba = POP();
+static void
+ob_pci_bus_map_out(int *idx)
+{
+ /* Should call pci-map-out but nothing to do in it yet */
+ fword("2drop");
+}
- virt = ob_pci_map(ba, size);
+static void
+ob_pci_config_read8(int *idx)
+{
+ cell hi = POP();
Should this not be ucell?
+ 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);
+}
- PUSH(virt);
+static void
+ob_pci_config_write8(int *idx)
+{
+ cell hi = POP();
and here too? + pci_addr
addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi));
+ cell val = POP();
+ pci_config_write8(addr, hi & 0xff, val & 0xff);
}
What about the other config-* words? Given that you're done the byte implementation
then you might as well do the same for the other accesses. static void
@@ -459,7 +471,10 @@ 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 },
{ "dma-alloc", ob_pci_dma_alloc },
{ "dma-free", ob_pci_dma_free },
{ "dma-map-in", ob_pci_dma_map_in },
@@ -473,7 +488,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) = {
@@ -481,7 +503,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]
A couple of comments here: using Forth here makes this the only PCI word that is
implemented in Forth rather than C which tends to make debugging a lot more
difficult. I'd much prefer to have the implementation in one place and then consider
a conversion later if required. Secondly this doesn't seem to be correct in that
it doesn't take the physical address
and map it to a virtual address and so it assumes this area has been pre-mapped. Is
this another piece of code borrowed from SLOF, since it runs in real mode rather than
virtual mode?
The idea is from SLOF but I think the I had to rewrite it for OpenBIOS so
the code is not the same. It does translate phys address using the reg
property (although as the comment says I wasn't sure if that's the correct
way) but it does not try to map an unmapped bar. But that's probably not
needed because OpenBIOS will map BARs on detecting devices so this word
only needs to return the correct address for the already mapped bar
(despite what its name may suggest).
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 )
I've had a read of the specifications and it's not very clear what type of
addresses
these are supposed to take - are they PCI addresses or virtual addresses? Can you
tell from your ATI FCode ROM? The other issue is that I'm not convinced that
these words belong in this file, since
you're overriding the default implementations with ones that assume little-endian
access. I think the correct solution here is implement these words as part of the PCI
bus node and use set-token to override the FCode implementation as detailed in the
IEEE-1275 specification.
Any examples how to do that? Searching for set-token did not turn up
anything useful for me. I also remember Segher had similar suggestions in
this thread:
https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/message/HBB…
but I've never understood how to implement that. He also said the existing
io* words may not be correct on PPC as some of those might need sync or
something simlilar but that's a preexisting problem independent of this
patch and I don't know what would be the correct way to access mmio so I
did not try to fix that (also it appears that's not needed on QEMU and
that's only what I care about so I just mention it as a reminder; I don't
intend to attempt to fix it because I don't know PPC well enough and can't
test it).
Regards,
BALATON Zoltan
450
days inactive
532
days old
2 comments
2 participants
participants (2)
-
BALATON Zoltan -
Mark Cave-Ayland