Signed-off-by: BALATON Zoltan balaton@eik.bme.hu --- v4: Added call parent word for bridge devices
drivers/pci.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..084ebe9 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 }, @@ -476,12 +536,54 @@ ob_pci_bridge_map_in(int *idx) call_parent_method("pci-map-in"); }
+static void +ob_pci_bridge_cread8(void) +{ + call_parent_method("config-b@"); +} + +static void +ob_pci_bridge_cwrite8(void) +{ + call_parent_method("config-b!"); +} + +static void +ob_pci_bridge_cread16(void) +{ + call_parent_method("config-w@"); +} + +static void +ob_pci_bridge_cwrite16(void) +{ + call_parent_method("config-w!"); +} + +static void +ob_pci_bridge_cread32(void) +{ + call_parent_method("config-l@"); +} + +static void +ob_pci_bridge_cwrite32(void) +{ + call_parent_method("config-l!"); +} + NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in }, + { "config-b@", ob_pci_bridge_cread8 }, + { "config-b!", ob_pci_bridge_cwrite8 }, + { "config-w@", ob_pci_bridge_cread16 }, + { "config-w!", ob_pci_bridge_cwrite16 }, + { "config-l@", ob_pci_bridge_cread32 }, + { "config-l!", ob_pci_bridge_cwrite32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
On 28/01/2023 22:42, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
v4: Added call parent word for bridge devices
drivers/pci.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..084ebe9 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 },
@@ -476,12 +536,54 @@ ob_pci_bridge_map_in(int *idx) call_parent_method("pci-map-in"); }
+static void +ob_pci_bridge_cread8(void) +{
- call_parent_method("config-b@");
+}
+static void +ob_pci_bridge_cwrite8(void) +{
- call_parent_method("config-b!");
+}
+static void +ob_pci_bridge_cread16(void) +{
- call_parent_method("config-w@");
+}
+static void +ob_pci_bridge_cwrite16(void) +{
- call_parent_method("config-w!");
+}
+static void +ob_pci_bridge_cread32(void) +{
- call_parent_method("config-l@");
+}
+static void +ob_pci_bridge_cwrite32(void) +{
- call_parent_method("config-l!");
+}
- NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in },
- { "config-b@", ob_pci_bridge_cread8 },
- { "config-b!", ob_pci_bridge_cwrite8 },
- { "config-w@", ob_pci_bridge_cread16 },
- { "config-w!", ob_pci_bridge_cwrite16 },
- { "config-l@", ob_pci_bridge_cread32 },
- { "config-l!", ob_pci_bridge_cwrite32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I've given this a try, and whilst the patch looks right to me, there is still something wrong with regard to the PCI bridge:
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 ok 1 > u. ffff ok
ffff normally indicates a failed read/write to PCI configuration space so it seems like the offset isn't being calculated correctly.
ATB,
Mark.
On Tue, 31 Jan 2023, Mark Cave-Ayland wrote:
On 28/01/2023 22:42, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
v4: Added call parent word for bridge devices
drivers/pci.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..084ebe9 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 },
@@ -476,12 +536,54 @@ ob_pci_bridge_map_in(int *idx) call_parent_method("pci-map-in"); } +static void +ob_pci_bridge_cread8(void) +{
- call_parent_method("config-b@");
+}
+static void +ob_pci_bridge_cwrite8(void) +{
- call_parent_method("config-b!");
+}
+static void +ob_pci_bridge_cread16(void) +{
- call_parent_method("config-w@");
+}
+static void +ob_pci_bridge_cwrite16(void) +{
- call_parent_method("config-w!");
+}
+static void +ob_pci_bridge_cread32(void) +{
- call_parent_method("config-l@");
+}
+static void +ob_pci_bridge_cwrite32(void) +{
- call_parent_method("config-l!");
+}
- NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in },
- { "config-b@", ob_pci_bridge_cread8 },
- { "config-b!", ob_pci_bridge_cwrite8 },
- { "config-w@", ob_pci_bridge_cread16 },
- { "config-w!", ob_pci_bridge_cwrite16 },
- { "config-l@", ob_pci_bridge_cread32 },
- { "config-l!", ob_pci_bridge_cwrite32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I've given this a try, and whilst the patch looks right to me, there is still something wrong with regard to the PCI bridge:
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 ok 1 > u. ffff ok
ffff normally indicates a failed read/write to PCI configuration space so it seems like the offset isn't being calculated correctly.
No idea what could be wrong with that but sent a new version with a different approach which adds these via Forth so we avoid going back and ... well, forth :-) between C and Forth (Forth is even confusing just to talk about) so if that was causing a problem somhow corrupting the stack this should avoid that and also avoids the one line functions so if this works I'd add another patch to replace the other similar dma-* words too. Can you test this please?
Regards, BALATON Zoltan
On 31/01/2023 22:22, Mark Cave-Ayland wrote:
On 28/01/2023 22:42, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
v4: Added call parent word for bridge devices
drivers/pci.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..084ebe9 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 }, @@ -476,12 +536,54 @@ ob_pci_bridge_map_in(int *idx) call_parent_method("pci-map-in"); } +static void +ob_pci_bridge_cread8(void) +{ + call_parent_method("config-b@"); +}
+static void +ob_pci_bridge_cwrite8(void) +{ + call_parent_method("config-b!"); +}
+static void +ob_pci_bridge_cread16(void) +{ + call_parent_method("config-w@"); +}
+static void +ob_pci_bridge_cwrite16(void) +{ + call_parent_method("config-w!"); +}
+static void +ob_pci_bridge_cread32(void) +{ + call_parent_method("config-l@"); +}
+static void +ob_pci_bridge_cwrite32(void) +{ + call_parent_method("config-l!"); +}
NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in }, + { "config-b@", ob_pci_bridge_cread8 }, + { "config-b!", ob_pci_bridge_cwrite8 }, + { "config-w@", ob_pci_bridge_cread16 }, + { "config-w!", ob_pci_bridge_cwrite16 }, + { "config-l@", ob_pci_bridge_cread32 }, + { "config-l!", ob_pci_bridge_cwrite32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in },
I've given this a try, and whilst the patch looks right to me, there is still something wrong with regard to the PCI bridge:
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 ok 1 > u. ffff ok
ffff normally indicates a failed read/write to PCI configuration space so it seems like the offset isn't being calculated correctly.
I spent a few more hours testing this patch over the weekend, and at least for SPARC64 something is wrong when calculating the address offsets for configuration space. I don't think it is directly caused by this patch, more that it exposes an existing bug somewhere in OpenBIOS.
There are still a few more days before QEMU freeze so I'll try and dig further into this to try and understand what is happening.
ATB,
Mark.
-
BALATON Zoltan -
Mark Cave-Ayland