[Qemu-devel] [PATCH, RFC] Sparc64: convert APB to qdev

[Qemu-devel] [PATCH, RFC] Sparc64: convert APB to qdev

[Blue Swirl]

I have a problem with APB conversion to qdev. For some reason, with
the patch applied, PCI config register access changes and OpenBIOS
can't find any PCI devices.

I get this output without the patch (CONFIG_DEBUG_PCI enabled for
OpenBIOS to get the PCI device list):

OpenBIOS for Sparc64
Initializing PCI devices...
0:0.0 - 108e:a000 - /pci -
0:1.0 - 108e:5000 - /pci/pci -
0:1.1 - 108e:5000 - /pci/pci/pci -
0:2.0 - 1234:1111 - /pci/pci/pci/QEMU,VGA -
0:3.0 - 108e:1000 - /pci/pci/pci/ebus -
0:4.0 - 10ec:8029 - /pci/pci/pci/NE2000 -
0:5.0 - 1095:646 - /pci/pci/pci/pci-ata -
Configuration device id QEMU version 1 machine id 0
CPUs: 1 x SUNW,UltraSPARC-II
UUID: 00000000-0000-0000-0000-000000000000
Welcome to OpenBIOS v1.0 built on Jul 11 2009 12:00
  Type 'help' for detailed information

[sparc64] Booting file 'disk' with parameters ''

0 > QEMU 0.11.50 monitor - type 'help' for more information
(qemu) info pci
  Bus  0, device   0, function 0:
    Host bridge: PCI device 108e:a000
      id ""
  Bus  0, device   1, function 0:
    PCI bridge: PCI device 108e:5000
      BUS 0.
      id ""
  Bus  0, device   1, function 1:
    PCI bridge: PCI device 108e:5000
      BUS 0.
      id ""
  Bus  0, device   2, function 0:
    VGA controller: PCI device 1234:1111
      BAR0: 32 bit memory at 0x00800000 [0x00ffffff].
      id ""
  Bus  0, device   3, function 0:
    Bridge: PCI device 108e:1000
      BAR0: 32 bit memory at 0x01000000 [0x01ffffff].
      BAR1: 32 bit memory at 0x02000000 [0x027fffff].
      id ""
  Bus  0, device   4, function 0:
    Ethernet controller: PCI device 10ec:8029
      IRQ 0.
      BAR0: I/O at 0x0400 [0x04ff].
      id ""
  Bus  0, device   5, function 0:
    IDE controller: PCI device 1095:0646
      IRQ 1.
      BAR0: I/O at 0x0500 [0x0507].
      BAR1: I/O at 0x0580 [0x0583].
      BAR2: I/O at 0x0600 [0x0607].
      BAR3: I/O at 0x0680 [0x0683].
      BAR4: I/O at 0x0700 [0x070f].
      id ""
(qemu) info qtree
bus: main-system-bus
  type System
  dev: m48t59, id ""
    dev-prop: size = 8192
    dev-prop: type = 59
    dev-prop: io_base = 0x74
    mmio ffffffffffffffff/0000000000002000
  dev: fdc, id ""
    gpio-in 1
    mmio ffffffffffffffff/0000000000000008

With the patch, I get the following:

OpenBIOS for Sparc64
Initializing PCI devices...
Configuration device id QEMU version 1 machine id 0
CPUs: 1 x SUNW,UltraSPARC-II
UUID: 00000000-0000-0000-0000-000000000000
Input device /pci/pci/pci/ebus/su not found.
Output device /pci/pci/pci/ebus/su not found.
Output device /pci/pci/pci/ebus/su not found.
Input device /pci/pci/pci/ebus/su not found.
Output device screen not found.
  Type 'help' for detailed information

[sparc64] Booting file 'disk' with parameters ''

0 > QEMU 0.11.50 monitor - type 'help' for more information
(qemu) info pci
  Bus  0, device   0, function 0:
    Host bridge: PCI device 108e:a000
      id ""
  Bus  0, device   1, function 0:
    PCI bridge: PCI device 108e:5000
      BUS 0.
      id ""
  Bus  0, device   1, function 1:
    PCI bridge: PCI device 108e:5000
      BUS 0.
      id ""
  Bus  0, device   2, function 0:
    VGA controller: PCI device 1234:1111
      BAR0: 32 bit memory at 0xffffffff [0x007ffffe].
      id ""
  Bus  0, device   3, function 0:
    Bridge: PCI device 108e:1000
      BAR0: 32 bit memory at 0xffffffff [0x00fffffe].
      BAR1: 32 bit memory at 0xffffffff [0x007ffffe].
      id ""
  Bus  0, device   4, function 0:
    Ethernet controller: PCI device 10ec:8029
      IRQ 0.
      BAR0: I/O at 0xffffffff [0x00fe].
      id ""
  Bus  0, device   5, function 0:
    IDE controller: PCI device 1095:0646
      IRQ 0.
      BAR0: I/O at 0xffffffff [0x0006].
      BAR1: I/O at 0xffffffff [0x0002].
      BAR2: I/O at 0xffffffff [0x0006].
      BAR3: I/O at 0xffffffff [0x0002].
      BAR4: I/O at 0xffffffff [0x000e].
      id ""
(qemu) info qtree
bus: main-system-bus
  type System
  dev: m48t59, id ""
    dev-prop: size = 8192
    dev-prop: type = 59
    dev-prop: io_base = 0x74
    mmio ffffffffffffffff/0000000000002000
  dev: fdc, id ""
    gpio-in 1
    mmio ffffffffffffffff/0000000000000008
  dev: pbm, id ""
    mmio 000001fe00002000/0000000000000040
    mmio 000001fe02000000/0000000000010000
    mmio 000001fe01000000/0000000000000010
    mmio 000001ff00000000/0000000010000000

DEBUG_ABP enabled so that the config reg accesses are printed (with the patch):
Initializing PCI devices...
APB: config_writel addr 0000000000000000 val 80000000
APB: config_writel addr 0000000000000000 val 80000000
APB: config_writel addr 0000000000000000 val 80000800
APB: config_writel addr 0000000000000000 val 80000800
APB: config_writel addr 0000000000000000 val 80001000
APB: config_writel addr 0000000000000000 val 80001000
APB: config_writel addr 0000000000000000 val 80001800
APB: config_writel addr 0000000000000000 val 80001800
APB: config_writel addr 0000000000000000 val 80002000
APB: config_writel addr 0000000000000000 val 80002000
APB: config_writel addr 0000000000000000 val 80002800
APB: config_writel addr 0000000000000000 val 80002800

Without the patch:
Initializing PCI devices...
APB: config_writel addr 0000000000000000 val 80000000
APB: config_writel addr 0000000000000000 val 80000000
APB: config_writel addr 0000000000000000 val 80000008
APB: config_writel addr 0000000000000000 val 80000008
APB: config_writel addr 0000000000000000 val 80000008
0:0.0 - 108e:a000 - APB: config_writel addr 0000000000000000 val 8000000c
/pci - APB: config_writel addr 0000000000000000 val 8000003c
APB: config_writel addr 0000000000000000 val 80000010
APB: config_writel addr 0000000000000000 val 80000010
APB: config_writel addr 0000000000000000 val 80000010

Strange.

BTW, the config probe is implemented incorrectly for both OpenBIOS and
QEMU, but that shouldn't be the issue because both assume config
mechanism 1.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/apb_pci.c |  103 ++++++++++++++++++++++++++++++++++++++++------------------
 hw/pci.c     |    8 +++--
 2 files changed, 76 insertions(+), 35 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 9f2a44d..627d8d5 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -26,7 +26,7 @@
    Ultrasparc PCI host is called the PCI Bus Module (PBM).  The APB is
    the secondary PCI bridge.  */

-#include "hw.h"
+#include "sysbus.h"
 #include "pci.h"

 /* debug APB */
@@ -42,7 +42,10 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
 typedef target_phys_addr_t pci_addr_t;
 #include "pci_host.h"

-typedef PCIHostState APBState;
+typedef struct APBState {
+    SysBusDevice busdev;
+    PCIHostState host_state;
+} APBState;

 static void pci_apb_config_writel (void *opaque, target_phys_addr_t addr,
                                          uint32_t val)
@@ -54,7 +57,7 @@ static void pci_apb_config_writel (void *opaque,
target_phys_addr_t addr,
 #endif
     APB_DPRINTF("config_writel addr " TARGET_FMT_plx " val %x\n", addr,
                 val);
-    s->config_reg = val;
+    s->host_state.config_reg = val;
 }

 static uint32_t pci_apb_config_readl (void *opaque,
@@ -63,7 +66,7 @@ static uint32_t pci_apb_config_readl (void *opaque,
     APBState *s = opaque;
     uint32_t val;

-    val = s->config_reg;
+    val = s->host_state.config_reg;
 #ifdef TARGET_WORDS_BIGENDIAN
     val = bswap32(val);
 #endif
@@ -225,34 +228,65 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
                      target_phys_addr_t mem_base,
                      qemu_irq *pic, PCIBus **bus2, PCIBus **bus3)
 {
-    APBState *s;
-    PCIDevice *d;
-    int pci_mem_config, pci_mem_data, apb_config, pci_ioport;
+    DeviceState *dev;
+    SysBusDevice *s;
+    APBState *d;

-    s = qemu_mallocz(sizeof(APBState));
     /* Ultrasparc PBM main bus */
-    s->bus = pci_register_bus(NULL, "pci",
-                              pci_apb_set_irq, pci_pbm_map_irq, pic, 0, 32);
+    dev = qdev_create(NULL, "pbm");
+    qdev_init(dev);
+    s = sysbus_from_qdev(dev);
+    /* apb_config */
+    sysbus_mmio_map(s, 0, special_base + 0x2000ULL);
+    /* pci_ioport */
+    sysbus_mmio_map(s, 1, special_base + 0x2000000ULL);
+    /* mem_config: XXX size should be 4G-prom */
+    sysbus_mmio_map(s, 2, special_base + 0x1000000ULL);
+    /* mem_data */
+    sysbus_mmio_map(s, 3, mem_base);
+    d = FROM_SYSBUS(APBState, s);
+    d->host_state.bus = pci_register_bus(NULL, "pci",
+                                         pci_apb_set_irq, pci_pbm_map_irq, pic,
+                                         0, 32);
+    pci_create_simple(d->host_state.bus, 0, "pbm");
+    /* APB secondary busses */
+    *bus2 = pci_bridge_init(d->host_state.bus, 8, PCI_VENDOR_ID_SUN,
+                            PCI_DEVICE_ID_SUN_SIMBA, pci_apb_map_irq,
+                            "Advanced PCI Bus secondary bridge 1");
+    *bus3 = pci_bridge_init(d->host_state.bus, 9, PCI_VENDOR_ID_SUN,
+                            PCI_DEVICE_ID_SUN_SIMBA, pci_apb_map_irq,
+                            "Advanced PCI Bus secondary bridge 2");

-    pci_mem_config = cpu_register_io_memory(pci_apb_config_read,
-                                            pci_apb_config_write, s);
+    return d->host_state.bus;
+}
+
+static void pci_pbm_init_device(SysBusDevice *dev)
+{
+
+    APBState *s;
+    int pci_mem_config, pci_mem_data, apb_config, pci_ioport;
+
+    s = FROM_SYSBUS(APBState, dev);
+    /* apb_config */
     apb_config = cpu_register_io_memory(apb_config_read,
                                         apb_config_write, s);
-    pci_mem_data = cpu_register_io_memory(pci_apb_read,
-                                          pci_apb_write, s);
+    sysbus_init_mmio(dev, 0x40ULL, apb_config);
+    /* pci_ioport */
     pci_ioport = cpu_register_io_memory(pci_apb_ioread,
                                           pci_apb_iowrite, s);
+    sysbus_init_mmio(dev, 0x10000ULL, pci_ioport);
+    /* mem_config  */
+    pci_mem_config = cpu_register_io_memory(pci_apb_config_read,
+                                            pci_apb_config_write, s);
+    sysbus_init_mmio(dev, 0x10ULL, pci_mem_config);
+    /* mem_data */
+    pci_mem_data = cpu_register_io_memory(pci_apb_read,
+                                          pci_apb_write, s);
+    sysbus_init_mmio(dev, 0x10000000ULL, pci_mem_data);
+}

-    cpu_register_physical_memory(special_base + 0x2000ULL, 0x40, apb_config);
-    cpu_register_physical_memory(special_base + 0x1000000ULL, 0x10,
-                                 pci_mem_config);
-    cpu_register_physical_memory(special_base + 0x2000000ULL, 0x10000,
-                                 pci_ioport);
-    cpu_register_physical_memory(mem_base, 0x10000000,
-                                 pci_mem_data); // XXX size should be 4G-prom
-
-    d = pci_register_device(s->bus, "Advanced PCI Bus", sizeof(PCIDevice),
-                            0, NULL, NULL);
+static void pbm_pci_host_init(PCIDevice *d)
+{
     pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_SUN);
     pci_config_set_device_id(d->config, PCI_DEVICE_ID_SUN_SABRE);
     d->config[0x04] = 0x06; // command = bus master, pci mem
@@ -264,13 +298,18 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
     pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
     d->config[0x0D] = 0x10; // latency_timer
     d->config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
+}

-    /* APB secondary busses */
-    *bus2 = pci_bridge_init(s->bus, 8, PCI_VENDOR_ID_SUN,
-                            PCI_DEVICE_ID_SUN_SIMBA, pci_apb_map_irq,
-                            "Advanced PCI Bus secondary bridge 1");
-    *bus3 = pci_bridge_init(s->bus, 9, PCI_VENDOR_ID_SUN,
-                            PCI_DEVICE_ID_SUN_SIMBA, pci_apb_map_irq,
-                            "Advanced PCI Bus secondary bridge 2");
-    return s->bus;
+static PCIDeviceInfo pbm_pci_host_info = {
+    .qdev.name = "pbm",
+    .qdev.size = sizeof(PCIDevice),
+    .init      = pbm_pci_host_init,
+};
+
+static void pbm_register_devices(void)
+{
+    sysbus_register_dev("pbm", sizeof(APBState), pci_pbm_init_device);
+    pci_qdev_register(&pbm_pci_host_info);
 }
+
+device_init(pbm_register_devices)
diff --git a/hw/pci.c b/hw/pci.c
index 3b5a947..b0a2d79 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -145,11 +145,13 @@ PCIBus *pci_register_bus(DeviceState *parent,
const char *name,
     return bus;
 }

-static PCIBus *pci_register_secondary_bus(PCIDevice *dev,
pci_map_irq_fn map_irq)
+static PCIBus *pci_register_secondary_bus(PCIDevice *dev,
+                                          pci_map_irq_fn map_irq,
+                                          const char *name)
 {
     PCIBus *bus;

-    bus = qemu_mallocz(sizeof(PCIBus));
+    bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, &dev->qdev, name));
     bus->map_irq = map_irq;
     bus->parent_dev = dev;
     bus->next = dev->bus->next;
@@ -891,7 +893,7 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn,
uint16_t vid, uint16_t did,
         PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
     s->dev.config[0x1E] = 0xa0; // secondary status

-    s->bus = pci_register_secondary_bus(&s->dev, map_irq);
+    s->bus = pci_register_secondary_bus(&s->dev, map_irq, name);
     return s->bus;
 }

-- 
1.6.2.4

Re: [Qemu-devel] [PATCH, RFC] Sparc64: convert APB to qdev

[Igor Kovalenko]

On Mon, Jul 20, 2009 at 2:26 PM, Blue Swirl<blauwirbel@gmail.com> wrote:
> I have a problem with APB conversion to qdev. For some reason, with
> the patch applied, PCI config register access changes and OpenBIOS
> can't find any PCI devices.

You are using pci_host_data_* which expects opaque pointer to PCIHostState,
so you need to pass appropriate opaque to cpu_register_io_memory.
With this small amendment I verified there are no changes to qemu.log and
serial console output.

Not sure if this requires sign-off :)

Signed-off-by: igor.v.kovalenko@gmail.com

Index: qemu-trunk/hw/apb_pci.c
===================================================================
--- qemu-trunk.orig/hw/apb_pci.c
+++ qemu-trunk/hw/apb_pci.c
@@ -281,7 +281,7 @@ static void pci_pbm_init_device(SysBusDe
     sysbus_init_mmio(dev, 0x10ULL, pci_mem_config);
     /* mem_data */
     pci_mem_data = cpu_register_io_memory(pci_apb_read,
-                                          pci_apb_write, s);
+                                          pci_apb_write, &s->host_state);
     sysbus_init_mmio(dev, 0x10000000ULL, pci_mem_data);
 }



-- 
Kind regards,
Igor V. Kovalenko

Re: [Qemu-devel] [PATCH, RFC] Sparc64: convert APB to qdev

[Blue Swirl]

On Tue, Jul 21, 2009 at 12:19 AM, Igor
Kovalenko<igor.v.kovalenko@gmail.com> wrote:
> On Mon, Jul 20, 2009 at 2:26 PM, Blue Swirl<blauwirbel@gmail.com> wrote:
>> I have a problem with APB conversion to qdev. For some reason, with
>> the patch applied, PCI config register access changes and OpenBIOS
>> can't find any PCI devices.
>
> You are using pci_host_data_* which expects opaque pointer to PCIHostState,
> so you need to pass appropriate opaque to cpu_register_io_memory.
> With this small amendment I verified there are no changes to qemu.log and
> serial console output.

Great, thanks!

> Not sure if this requires sign-off :)

Good question, perhaps the SoB experts should clarify the policy.

But in this case it makes no sense to commit my broken version and
then your patch. I think I'll just give you credit in the message for
the fixed commit.

Re: [Qemu-devel] [PATCH, RFC] Sparc64: convert APB to qdev

[Jason Wessel]

Blue Swirl wrote:
> On Tue, Jul 21, 2009 at 12:19 AM, Igor
> Kovalenko<igor.v.kovalenko@gmail.com> wrote:
>   
>> On Mon, Jul 20, 2009 at 2:26 PM, Blue Swirl<blauwirbel@gmail.com> wrote:
>>     
>>> I have a problem with APB conversion to qdev. For some reason, with
>>> the patch applied, PCI config register access changes and OpenBIOS
>>> can't find any PCI devices.
>>>       
>> You are using pci_host_data_* which expects opaque pointer to PCIHostState,
>> so you need to pass appropriate opaque to cpu_register_io_memory.
>> With this small amendment I verified there are no changes to qemu.log and
>> serial console output.
>>     
>
> Great, thanks!
>
>   
>> Not sure if this requires sign-off :)
>>     
>
> Good question, perhaps the SoB experts should clarify the policy.
>
> But in this case it makes no sense to commit my broken version and
> then your patch. I think I'll just give you credit in the message for
> the fixed commit.
>
>   

If you are looking for some kind of standard to follow based on the
linux kernel use, the document is:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;hb=HEAD

The example given shows a maintainer cleaning up someone else's patch
for merge.

347
<http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=5c555a8b39e553cf0c3117026ebf37d481c588b6;hb=HEAD#l347>
        Signed-off-by: Random J Developer <random@developer.example.org>
348
<http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=5c555a8b39e553cf0c3117026ebf37d481c588b6;hb=HEAD#l348>
        [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h]
349
<http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=5c555a8b39e553cf0c3117026ebf37d481c588b6;hb=HEAD#l349>
        Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>

The pattern is no different if you fold 1 or more patches together.  The
credit can go in the bracket section.

Akpm does this a lot if you wanted to search the linux kernel for any
other examples.
linux-2.6.git % git log |grep "\[akpm" |wc -l
1276


Cheers,
Jason.