[Qemu-devel] [PATCH 00/15] sun4u: tidy-up CPU, APB and ebus

[Qemu-devel] [PATCH 00/15] sun4u: tidy-up CPU, APB and ebus

[Mark Cave-Ayland]

This patchset for 2.12 continues with tidying up the sun4u CPU, APB and
ebus devices by encapsulating the ebus ISA bus within the ebus QOM device,
allowing APB and ebus devices to be instantiated directly via QOM, and
formally wiring up the device IRQs using qdev GPIOs rather than passing
around arrays of qemu_irq via various _init() functions.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Mark Cave-Ayland (15):
  apb: move QOM macros and typedefs from apb.c to apb.h
  sun4u: ebus QOMify tidy-up
  sun4u: move ISABus inside of EBusState
  sun4u: remove pci_ebus_init() function
  sun4u: move initialisation of all ISABus devices into ebus_realize()
  apb: APB QOMify tidy-up
  apb: return APBState from pci_apb_init() rather then PCIBus
  apb: use gpios to wire up the apb device to the SPARC CPU IRQs
  apb: move the two secondary PCI bridges objects into APBState
  apb: remove pci_apb_init() and instantiate APB device using qdev
  apb: split pci_pbm_map_irq() into separate functions for bus A and
    bus B
  ebus: wire up OBIO interrupts to APB pbm via qdev GPIOs
  apb: replace OBIO interrupt numbers in pci_pbmA_map_irq() with
    constants
  sparc64: introduce trace-events for hw/sparc64
  sun4u: switch from EBUS_DPRINTF() macro to trace-events

 Makefile.objs              |    1 +
 hw/pci-host/apb.c          |  279 ++++++++++++++------------------------------
 hw/sparc64/sparc64.c       |    2 +
 hw/sparc64/sun4u.c         |  184 ++++++++++++++++-------------
 hw/sparc64/trace-events    |    4 +
 include/hw/pci-host/apb.h  |  101 +++++++++++++++-
 include/hw/sparc/sparc64.h |    2 +
 7 files changed, 297 insertions(+), 276 deletions(-)
 create mode 100644 hw/sparc64/trace-events

-- 
1.7.10.4

[Qemu-devel] [PATCH 01/15] apb: move QOM macros and typedefs from apb.c to apb.h

[Mark Cave-Ayland]

This also includes the related IOMMUState typedef and defines.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |   85 --------------------------------------------
 include/hw/pci-host/apb.h |   86 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 85 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 64025cd..f743a4e 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -82,91 +82,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
 #define MAX_IVEC 0x40
 #define NO_IRQ_REQUEST (MAX_IVEC + 1)
 
-#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
-#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
-#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
-#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
-
-#define IOMMU_NREGS             3
-
-#define IOMMU_CTRL              0x0
-#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
-#define IOMMU_CTRL_MMU_EN       (1ULL)
-
-#define IOMMU_CTRL_TSB_SHIFT    16
-
-#define IOMMU_BASE              0x8
-#define IOMMU_FLUSH             0x10
-
-#define IOMMU_TTE_DATA_V        (1ULL << 63)
-#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
-#define IOMMU_TTE_DATA_W        (1ULL << 1)
-
-#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
-#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
-
-#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
-#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
-
-#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
-#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
-#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
-#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
-#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
-#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
-
-typedef struct IOMMUState {
-    AddressSpace iommu_as;
-    IOMMUMemoryRegion iommu;
-
-    uint64_t regs[IOMMU_NREGS];
-} IOMMUState;
-
-#define TYPE_APB "pbm"
-
-#define APB_DEVICE(obj) \
-    OBJECT_CHECK(APBState, (obj), TYPE_APB)
-
-#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
-
-typedef struct APBState {
-    PCIHostState parent_obj;
-
-    MemoryRegion apb_config;
-    MemoryRegion pci_config;
-    MemoryRegion pci_mmio;
-    MemoryRegion pci_ioport;
-    uint64_t pci_irq_in;
-    IOMMUState iommu;
-    uint32_t pci_control[16];
-    uint32_t pci_irq_map[8];
-    uint32_t pci_err_irq_map[4];
-    uint32_t obio_irq_map[32];
-    qemu_irq *pbm_irqs;
-    qemu_irq *ivec_irqs;
-    unsigned int irq_request;
-    uint32_t reset_control;
-    unsigned int nr_resets;
-} APBState;
-
-#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
-#define PBM_PCI_BRIDGE(obj) \
-    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
-
-typedef struct PBMPCIBridge {
-    /*< private >*/
-    PCIBridge parent_obj;
-
-    /* Is this busA with in-built devices (ebus)? */
-    bool busA;
-} PBMPCIBridge;
-
 static inline void pbm_set_request(APBState *s, unsigned int irq_num)
 {
     APB_DPRINTF("%s: request irq %d\n", __func__, irq_num);
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index b19bd55..5d39c03 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -2,6 +2,92 @@
 #define PCI_HOST_APB_H
 
 #include "qemu-common.h"
+#include "hw/pci/pci_host.h"
+
+#define IOMMU_NREGS             3
+
+#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
+#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
+#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
+#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
+
+#define IOMMU_CTRL              0x0
+#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
+#define IOMMU_CTRL_MMU_EN       (1ULL)
+
+#define IOMMU_CTRL_TSB_SHIFT    16
+
+#define IOMMU_BASE              0x8
+#define IOMMU_FLUSH             0x10
+
+#define IOMMU_TTE_DATA_V        (1ULL << 63)
+#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
+#define IOMMU_TTE_DATA_W        (1ULL << 1)
+
+#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
+#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
+
+#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
+#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
+
+#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
+#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
+#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
+#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
+#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
+#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
+
+typedef struct IOMMUState {
+    AddressSpace iommu_as;
+    IOMMUMemoryRegion iommu;
+
+    uint64_t regs[IOMMU_NREGS];
+} IOMMUState;
+
+#define TYPE_APB "pbm"
+
+#define APB_DEVICE(obj) \
+    OBJECT_CHECK(APBState, (obj), TYPE_APB)
+
+#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
+
+typedef struct APBState {
+    PCIHostState parent_obj;
+
+    MemoryRegion apb_config;
+    MemoryRegion pci_config;
+    MemoryRegion pci_mmio;
+    MemoryRegion pci_ioport;
+    uint64_t pci_irq_in;
+    IOMMUState iommu;
+    uint32_t pci_control[16];
+    uint32_t pci_irq_map[8];
+    uint32_t pci_err_irq_map[4];
+    uint32_t obio_irq_map[32];
+    qemu_irq *pbm_irqs;
+    qemu_irq *ivec_irqs;
+    unsigned int irq_request;
+    uint32_t reset_control;
+    unsigned int nr_resets;
+} APBState;
+
+typedef struct PBMPCIBridge {
+    /*< private >*/
+    PCIBridge parent_obj;
+
+    /* Is this busA with in-built devices (ebus)? */
+    bool busA;
+} PBMPCIBridge;
+
+#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
+#define PBM_PCI_BRIDGE(obj) \
+    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
 
 PCIBus *pci_apb_init(hwaddr special_base,
                      hwaddr mem_base,
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 01/15] apb: move QOM macros and typedefs from apb.c to apb.h

[Artyom Tarasenko]

Hi Mark,

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This also includes the related IOMMUState typedef and defines.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |   85 --------------------------------------------
>  include/hw/pci-host/apb.h |   86 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+), 85 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 64025cd..f743a4e 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -82,91 +82,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>  #define MAX_IVEC 0x40
>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>
> -#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
> -#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
> -#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
> -#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
> -
> -#define IOMMU_NREGS             3
> -
> -#define IOMMU_CTRL              0x0
> -#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
> -#define IOMMU_CTRL_MMU_EN       (1ULL)
> -
> -#define IOMMU_CTRL_TSB_SHIFT    16
> -
> -#define IOMMU_BASE              0x8
> -#define IOMMU_FLUSH             0x10
> -
> -#define IOMMU_TTE_DATA_V        (1ULL << 63)
> -#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
> -#define IOMMU_TTE_DATA_W        (1ULL << 1)
> -
> -#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
> -#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
> -
> -#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
> -
> -#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
> -
> -typedef struct IOMMUState {
> -    AddressSpace iommu_as;
> -    IOMMUMemoryRegion iommu;
> -
> -    uint64_t regs[IOMMU_NREGS];
> -} IOMMUState;
> -
> -#define TYPE_APB "pbm"
> -
> -#define APB_DEVICE(obj) \
> -    OBJECT_CHECK(APBState, (obj), TYPE_APB)
> -
> -#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
> -
> -typedef struct APBState {
> -    PCIHostState parent_obj;
> -
> -    MemoryRegion apb_config;
> -    MemoryRegion pci_config;
> -    MemoryRegion pci_mmio;
> -    MemoryRegion pci_ioport;
> -    uint64_t pci_irq_in;
> -    IOMMUState iommu;
> -    uint32_t pci_control[16];
> -    uint32_t pci_irq_map[8];
> -    uint32_t pci_err_irq_map[4];
> -    uint32_t obio_irq_map[32];
> -    qemu_irq *pbm_irqs;
> -    qemu_irq *ivec_irqs;
> -    unsigned int irq_request;
> -    uint32_t reset_control;
> -    unsigned int nr_resets;
> -} APBState;
> -
> -#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
> -#define PBM_PCI_BRIDGE(obj) \
> -    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
> -
> -typedef struct PBMPCIBridge {
> -    /*< private >*/
> -    PCIBridge parent_obj;
> -
> -    /* Is this busA with in-built devices (ebus)? */
> -    bool busA;
> -} PBMPCIBridge;
> -
>  static inline void pbm_set_request(APBState *s, unsigned int irq_num)
>  {
>      APB_DPRINTF("%s: request irq %d\n", __func__, irq_num);
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index b19bd55..5d39c03 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -2,6 +2,92 @@
>  #define PCI_HOST_APB_H
>
>  #include "qemu-common.h"
> +#include "hw/pci/pci_host.h"
> +
> +#define IOMMU_NREGS             3
> +
> +#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
> +#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
> +#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
> +#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
> +
> +#define IOMMU_CTRL              0x0
> +#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
> +#define IOMMU_CTRL_MMU_EN       (1ULL)

While at it, I think the naming for the model-specific constants
should be more explicit.
How about US2I_IOMMU_ or SABRE_IOMMU_?

> +
> +#define IOMMU_CTRL_TSB_SHIFT    16
> +
> +#define IOMMU_BASE              0x8
> +#define IOMMU_FLUSH             0x10
> +
> +#define IOMMU_TTE_DATA_V        (1ULL << 63)
> +#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
> +#define IOMMU_TTE_DATA_W        (1ULL << 1)
> +
> +#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
> +#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
> +
> +#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
> +
> +#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL

Maybe it's possible to compute the mask from the bits, similarly to
how it's done in the Niagara model?
If so, these defines would be not necessary.

> +
> +typedef struct IOMMUState {
> +    AddressSpace iommu_as;
> +    IOMMUMemoryRegion iommu;
> +
> +    uint64_t regs[IOMMU_NREGS];
> +} IOMMUState;
> +
> +#define TYPE_APB "pbm"
> +
> +#define APB_DEVICE(obj) \
> +    OBJECT_CHECK(APBState, (obj), TYPE_APB)
> +
> +#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
> +
> +typedef struct APBState {
> +    PCIHostState parent_obj;
> +
> +    MemoryRegion apb_config;
> +    MemoryRegion pci_config;
> +    MemoryRegion pci_mmio;
> +    MemoryRegion pci_ioport;
> +    uint64_t pci_irq_in;
> +    IOMMUState iommu;
> +    uint32_t pci_control[16];
> +    uint32_t pci_irq_map[8];
> +    uint32_t pci_err_irq_map[4];
> +    uint32_t obio_irq_map[32];
> +    qemu_irq *pbm_irqs;
> +    qemu_irq *ivec_irqs;
> +    unsigned int irq_request;
> +    uint32_t reset_control;
> +    unsigned int nr_resets;
> +} APBState;
> +
> +typedef struct PBMPCIBridge {
> +    /*< private >*/
> +    PCIBridge parent_obj;
> +
> +    /* Is this busA with in-built devices (ebus)? */
> +    bool busA;
> +} PBMPCIBridge;
> +
> +#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
> +#define PBM_PCI_BRIDGE(obj) \
> +    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 01/15] apb: move QOM macros and typedefs from apb.c to apb.h

[Mark Cave-Ayland]

On 17/11/17 14:24, Artyom Tarasenko wrote:

> Hi Mark,
> 
> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> This also includes the related IOMMUState typedef and defines.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/pci-host/apb.c         |   85 --------------------------------------------
>>  include/hw/pci-host/apb.h |   86 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 86 insertions(+), 85 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 64025cd..f743a4e 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -82,91 +82,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>>  #define MAX_IVEC 0x40
>>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>>
>> -#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
>> -#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
>> -#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
>> -#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
>> -
>> -#define IOMMU_NREGS             3
>> -
>> -#define IOMMU_CTRL              0x0
>> -#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
>> -#define IOMMU_CTRL_MMU_EN       (1ULL)
>> -
>> -#define IOMMU_CTRL_TSB_SHIFT    16
>> -
>> -#define IOMMU_BASE              0x8
>> -#define IOMMU_FLUSH             0x10
>> -
>> -#define IOMMU_TTE_DATA_V        (1ULL << 63)
>> -#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
>> -#define IOMMU_TTE_DATA_W        (1ULL << 1)
>> -
>> -#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
>> -#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
>> -
>> -#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
>> -#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
>> -
>> -#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
>> -#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
>> -#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
>> -#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
>> -#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
>> -#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
>> -
>> -typedef struct IOMMUState {
>> -    AddressSpace iommu_as;
>> -    IOMMUMemoryRegion iommu;
>> -
>> -    uint64_t regs[IOMMU_NREGS];
>> -} IOMMUState;
>> -
>> -#define TYPE_APB "pbm"
>> -
>> -#define APB_DEVICE(obj) \
>> -    OBJECT_CHECK(APBState, (obj), TYPE_APB)
>> -
>> -#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
>> -
>> -typedef struct APBState {
>> -    PCIHostState parent_obj;
>> -
>> -    MemoryRegion apb_config;
>> -    MemoryRegion pci_config;
>> -    MemoryRegion pci_mmio;
>> -    MemoryRegion pci_ioport;
>> -    uint64_t pci_irq_in;
>> -    IOMMUState iommu;
>> -    uint32_t pci_control[16];
>> -    uint32_t pci_irq_map[8];
>> -    uint32_t pci_err_irq_map[4];
>> -    uint32_t obio_irq_map[32];
>> -    qemu_irq *pbm_irqs;
>> -    qemu_irq *ivec_irqs;
>> -    unsigned int irq_request;
>> -    uint32_t reset_control;
>> -    unsigned int nr_resets;
>> -} APBState;
>> -
>> -#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
>> -#define PBM_PCI_BRIDGE(obj) \
>> -    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>> -
>> -typedef struct PBMPCIBridge {
>> -    /*< private >*/
>> -    PCIBridge parent_obj;
>> -
>> -    /* Is this busA with in-built devices (ebus)? */
>> -    bool busA;
>> -} PBMPCIBridge;
>> -
>>  static inline void pbm_set_request(APBState *s, unsigned int irq_num)
>>  {
>>      APB_DPRINTF("%s: request irq %d\n", __func__, irq_num);
>> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
>> index b19bd55..5d39c03 100644
>> --- a/include/hw/pci-host/apb.h
>> +++ b/include/hw/pci-host/apb.h
>> @@ -2,6 +2,92 @@
>>  #define PCI_HOST_APB_H
>>
>>  #include "qemu-common.h"
>> +#include "hw/pci/pci_host.h"
>> +
>> +#define IOMMU_NREGS             3
>> +
>> +#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
>> +#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
>> +#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
>> +#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
>> +
>> +#define IOMMU_CTRL              0x0
>> +#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
>> +#define IOMMU_CTRL_MMU_EN       (1ULL)
> 
> While at it, I think the naming for the model-specific constants
> should be more explicit.
> How about US2I_IOMMU_ or SABRE_IOMMU_?

I'm actually planning to follow this up with another patchset to split
out the IOMMU into a separate file (and QOM object) in a similar way to
sun4m_iommu.c so I can do it there rather than in this patchset if you
agree?

>> +
>> +#define IOMMU_CTRL_TSB_SHIFT    16
>> +
>> +#define IOMMU_BASE              0x8
>> +#define IOMMU_FLUSH             0x10
>> +
>> +#define IOMMU_TTE_DATA_V        (1ULL << 63)
>> +#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
>> +#define IOMMU_TTE_DATA_W        (1ULL << 1)
>> +
>> +#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
>> +#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
>> +
>> +#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
>> +#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
>> +
>> +#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
>> +#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
>> +#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
>> +#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
>> +#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
>> +#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
> 
> Maybe it's possible to compute the mask from the bits, similarly to
> how it's done in the Niagara model?
> If so, these defines would be not necessary.

Hmmm yes that could be possible as part of the follow-up patchset too (I
think I wrote this code several years ago). I'll have a look once these
patches are merged in their final form to see what the result looks like.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 01/15] apb: move QOM macros and typedefs from apb.c to apb.h

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 4:40 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 17/11/17 14:24, Artyom Tarasenko wrote:
>
>> Hi Mark,
>>
>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> This also includes the related IOMMUState typedef and defines.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>  hw/pci-host/apb.c         |   85 --------------------------------------------
>>>  include/hw/pci-host/apb.h |   86 +++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 86 insertions(+), 85 deletions(-)
>>>
>>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>>> index 64025cd..f743a4e 100644
>>> --- a/hw/pci-host/apb.c
>>> +++ b/hw/pci-host/apb.c
>>> @@ -82,91 +82,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>>>  #define MAX_IVEC 0x40
>>>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>>>
>>> -#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
>>> -#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
>>> -#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
>>> -#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
>>> -
>>> -#define IOMMU_NREGS             3
>>> -
>>> -#define IOMMU_CTRL              0x0
>>> -#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
>>> -#define IOMMU_CTRL_MMU_EN       (1ULL)
>>> -
>>> -#define IOMMU_CTRL_TSB_SHIFT    16
>>> -
>>> -#define IOMMU_BASE              0x8
>>> -#define IOMMU_FLUSH             0x10
>>> -
>>> -#define IOMMU_TTE_DATA_V        (1ULL << 63)
>>> -#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
>>> -#define IOMMU_TTE_DATA_W        (1ULL << 1)
>>> -
>>> -#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
>>> -#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
>>> -
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
>>> -#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
>>> -
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
>>> -#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
>>> -
>>> -typedef struct IOMMUState {
>>> -    AddressSpace iommu_as;
>>> -    IOMMUMemoryRegion iommu;
>>> -
>>> -    uint64_t regs[IOMMU_NREGS];
>>> -} IOMMUState;
>>> -
>>> -#define TYPE_APB "pbm"
>>> -
>>> -#define APB_DEVICE(obj) \
>>> -    OBJECT_CHECK(APBState, (obj), TYPE_APB)
>>> -
>>> -#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
>>> -
>>> -typedef struct APBState {
>>> -    PCIHostState parent_obj;
>>> -
>>> -    MemoryRegion apb_config;
>>> -    MemoryRegion pci_config;
>>> -    MemoryRegion pci_mmio;
>>> -    MemoryRegion pci_ioport;
>>> -    uint64_t pci_irq_in;
>>> -    IOMMUState iommu;
>>> -    uint32_t pci_control[16];
>>> -    uint32_t pci_irq_map[8];
>>> -    uint32_t pci_err_irq_map[4];
>>> -    uint32_t obio_irq_map[32];
>>> -    qemu_irq *pbm_irqs;
>>> -    qemu_irq *ivec_irqs;
>>> -    unsigned int irq_request;
>>> -    uint32_t reset_control;
>>> -    unsigned int nr_resets;
>>> -} APBState;
>>> -
>>> -#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
>>> -#define PBM_PCI_BRIDGE(obj) \
>>> -    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>>> -
>>> -typedef struct PBMPCIBridge {
>>> -    /*< private >*/
>>> -    PCIBridge parent_obj;
>>> -
>>> -    /* Is this busA with in-built devices (ebus)? */
>>> -    bool busA;
>>> -} PBMPCIBridge;
>>> -
>>>  static inline void pbm_set_request(APBState *s, unsigned int irq_num)
>>>  {
>>>      APB_DPRINTF("%s: request irq %d\n", __func__, irq_num);
>>> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
>>> index b19bd55..5d39c03 100644
>>> --- a/include/hw/pci-host/apb.h
>>> +++ b/include/hw/pci-host/apb.h
>>> @@ -2,6 +2,92 @@
>>>  #define PCI_HOST_APB_H
>>>
>>>  #include "qemu-common.h"
>>> +#include "hw/pci/pci_host.h"
>>> +
>>> +#define IOMMU_NREGS             3
>>> +
>>> +#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
>>> +#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
>>> +#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
>>> +#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
>>> +
>>> +#define IOMMU_CTRL              0x0
>>> +#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
>>> +#define IOMMU_CTRL_MMU_EN       (1ULL)
>>
>> While at it, I think the naming for the model-specific constants
>> should be more explicit.
>> How about US2I_IOMMU_ or SABRE_IOMMU_?
>
> I'm actually planning to follow this up with another patchset to split
> out the IOMMU into a separate file (and QOM object) in a similar way to
> sun4m_iommu.c so I can do it there rather than in this patchset if you
> agree?

Sounds good.

>>> +
>>> +#define IOMMU_CTRL_TSB_SHIFT    16
>>> +
>>> +#define IOMMU_BASE              0x8
>>> +#define IOMMU_FLUSH             0x10
>>> +
>>> +#define IOMMU_TTE_DATA_V        (1ULL << 63)
>>> +#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
>>> +#define IOMMU_TTE_DATA_W        (1ULL << 1)
>>> +
>>> +#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
>>> +#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
>>> +
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
>>> +#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
>>> +
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
>>> +#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
>>
>> Maybe it's possible to compute the mask from the bits, similarly to
>> how it's done in the Niagara model?
>> If so, these defines would be not necessary.
>
> Hmmm yes that could be possible as part of the follow-up patchset too (I
> think I wrote this code several years ago). I'll have a look once these
> patches are merged in their final form to see what the result looks like.

ok, no rush.

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 01/15] apb: move QOM macros and typedefs from apb.c to apb.h

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This also includes the related IOMMUState typedef and defines.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/pci-host/apb.c         |   85 --------------------------------------------
>  include/hw/pci-host/apb.h |   86 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+), 85 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 64025cd..f743a4e 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -82,91 +82,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>  #define MAX_IVEC 0x40
>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>  
> -#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
> -#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
> -#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
> -#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
> -
> -#define IOMMU_NREGS             3
> -
> -#define IOMMU_CTRL              0x0
> -#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
> -#define IOMMU_CTRL_MMU_EN       (1ULL)
> -
> -#define IOMMU_CTRL_TSB_SHIFT    16
> -
> -#define IOMMU_BASE              0x8
> -#define IOMMU_FLUSH             0x10
> -
> -#define IOMMU_TTE_DATA_V        (1ULL << 63)
> -#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
> -#define IOMMU_TTE_DATA_W        (1ULL << 1)
> -
> -#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
> -#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
> -
> -#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
> -#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
> -
> -#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
> -#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
> -
> -typedef struct IOMMUState {
> -    AddressSpace iommu_as;
> -    IOMMUMemoryRegion iommu;
> -
> -    uint64_t regs[IOMMU_NREGS];
> -} IOMMUState;
> -
> -#define TYPE_APB "pbm"
> -
> -#define APB_DEVICE(obj) \
> -    OBJECT_CHECK(APBState, (obj), TYPE_APB)
> -
> -#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
> -
> -typedef struct APBState {
> -    PCIHostState parent_obj;
> -
> -    MemoryRegion apb_config;
> -    MemoryRegion pci_config;
> -    MemoryRegion pci_mmio;
> -    MemoryRegion pci_ioport;
> -    uint64_t pci_irq_in;
> -    IOMMUState iommu;
> -    uint32_t pci_control[16];
> -    uint32_t pci_irq_map[8];
> -    uint32_t pci_err_irq_map[4];
> -    uint32_t obio_irq_map[32];
> -    qemu_irq *pbm_irqs;
> -    qemu_irq *ivec_irqs;
> -    unsigned int irq_request;
> -    uint32_t reset_control;
> -    unsigned int nr_resets;
> -} APBState;
> -
> -#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
> -#define PBM_PCI_BRIDGE(obj) \
> -    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
> -
> -typedef struct PBMPCIBridge {
> -    /*< private >*/
> -    PCIBridge parent_obj;
> -
> -    /* Is this busA with in-built devices (ebus)? */
> -    bool busA;
> -} PBMPCIBridge;
> -
>  static inline void pbm_set_request(APBState *s, unsigned int irq_num)
>  {
>      APB_DPRINTF("%s: request irq %d\n", __func__, irq_num);
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index b19bd55..5d39c03 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -2,6 +2,92 @@
>  #define PCI_HOST_APB_H
>  
>  #include "qemu-common.h"
> +#include "hw/pci/pci_host.h"
> +
> +#define IOMMU_NREGS             3
> +
> +#define IOMMU_PAGE_SIZE_8K      (1ULL << 13)
> +#define IOMMU_PAGE_MASK_8K      (~(IOMMU_PAGE_SIZE_8K - 1))
> +#define IOMMU_PAGE_SIZE_64K     (1ULL << 16)
> +#define IOMMU_PAGE_MASK_64K     (~(IOMMU_PAGE_SIZE_64K - 1))
> +
> +#define IOMMU_CTRL              0x0
> +#define IOMMU_CTRL_TBW_SIZE     (1ULL << 2)
> +#define IOMMU_CTRL_MMU_EN       (1ULL)
> +
> +#define IOMMU_CTRL_TSB_SHIFT    16
> +
> +#define IOMMU_BASE              0x8
> +#define IOMMU_FLUSH             0x10
> +
> +#define IOMMU_TTE_DATA_V        (1ULL << 63)
> +#define IOMMU_TTE_DATA_SIZE     (1ULL << 61)
> +#define IOMMU_TTE_DATA_W        (1ULL << 1)
> +
> +#define IOMMU_TTE_PHYS_MASK_8K  0x1ffffffe000ULL
> +#define IOMMU_TTE_PHYS_MASK_64K 0x1ffffff8000ULL
> +
> +#define IOMMU_TSB_8K_OFFSET_MASK_8M    0x00000000007fe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_16M   0x0000000000ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_32M   0x0000000001ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_64M   0x0000000003ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_128M  0x0000000007ffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_256M  0x000000000fffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_512M  0x000000001fffe000ULL
> +#define IOMMU_TSB_8K_OFFSET_MASK_1G    0x000000003fffe000ULL
> +
> +#define IOMMU_TSB_64K_OFFSET_MASK_64M  0x0000000003ff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_128M 0x0000000007ff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_256M 0x000000000fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_512M 0x000000001fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_1G   0x000000003fff0000ULL
> +#define IOMMU_TSB_64K_OFFSET_MASK_2G   0x000000007fff0000ULL
> +
> +typedef struct IOMMUState {
> +    AddressSpace iommu_as;
> +    IOMMUMemoryRegion iommu;
> +
> +    uint64_t regs[IOMMU_NREGS];
> +} IOMMUState;
> +
> +#define TYPE_APB "pbm"
> +
> +#define APB_DEVICE(obj) \
> +    OBJECT_CHECK(APBState, (obj), TYPE_APB)
> +
> +#define TYPE_APB_IOMMU_MEMORY_REGION "pbm-iommu-memory-region"
> +
> +typedef struct APBState {
> +    PCIHostState parent_obj;
> +
> +    MemoryRegion apb_config;
> +    MemoryRegion pci_config;
> +    MemoryRegion pci_mmio;
> +    MemoryRegion pci_ioport;
> +    uint64_t pci_irq_in;
> +    IOMMUState iommu;
> +    uint32_t pci_control[16];
> +    uint32_t pci_irq_map[8];
> +    uint32_t pci_err_irq_map[4];
> +    uint32_t obio_irq_map[32];
> +    qemu_irq *pbm_irqs;
> +    qemu_irq *ivec_irqs;
> +    unsigned int irq_request;
> +    uint32_t reset_control;
> +    unsigned int nr_resets;
> +} APBState;
> +
> +typedef struct PBMPCIBridge {
> +    /*< private >*/
> +    PCIBridge parent_obj;
> +
> +    /* Is this busA with in-built devices (ebus)? */
> +    bool busA;
> +} PBMPCIBridge;
> +
> +#define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
> +#define PBM_PCI_BRIDGE(obj) \
> +    OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>  
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> 

[Qemu-devel] [PATCH 02/15] sun4u: ebus QOMify tidy-up

[Mark Cave-Ayland]

The main change here is to introduce the proper TYPE_EBUS/EBUS QOM macros
and remove the use of DO_UPCAST.

Alongside this there are some a couple of minor cosmetic changes and a rename
of pci_ebus_realize() to ebus_realize() since the ebus device is always what
is effectively a PCI-ISA bridge.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc64/sun4u.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 1672f25..394b7d6 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -81,11 +81,16 @@ struct hwdef {
 };
 
 typedef struct EbusState {
-    PCIDevice pci_dev;
+    /*< private >*/
+    PCIDevice parent_obj;
+
     MemoryRegion bar0;
     MemoryRegion bar1;
 } EbusState;
 
+#define TYPE_EBUS "ebus"
+#define EBUS(obj) OBJECT_CHECK(EbusState, (obj), TYPE_EBUS)
+
 void DMA_init(ISABus *bus, int high_page_enable)
 {
 }
@@ -236,9 +241,9 @@ pci_ebus_init(PCIDevice *pci_dev, qemu_irq *irqs)
     return isa_bus;
 }
 
-static void pci_ebus_realize(PCIDevice *pci_dev, Error **errp)
+static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
-    EbusState *s = DO_UPCAST(EbusState, pci_dev, pci_dev);
+    EbusState *s = EBUS(pci_dev);
 
     if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
                      pci_address_space_io(pci_dev), errp)) {
@@ -264,7 +269,7 @@ static void ebus_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->realize = pci_ebus_realize;
+    k->realize = ebus_realize;
     k->vendor_id = PCI_VENDOR_ID_SUN;
     k->device_id = PCI_DEVICE_ID_SUN_EBUS;
     k->revision = 0x01;
@@ -272,10 +277,10 @@ static void ebus_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ebus_info = {
-    .name          = "ebus",
+    .name          = TYPE_EBUS,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(EbusState),
     .class_init    = ebus_class_init,
+    .instance_size = sizeof(EbusState),
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },
@@ -463,7 +468,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     pci_busA->slot_reserved_mask = 0xfffffff1;
     pci_busB->slot_reserved_mask = 0xfffffff0;
 
-    ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, "ebus");
+    ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
     qdev_init_nofail(DEVICE(ebus));
 
     isa_bus = pci_ebus_init(ebus, pbm_irqs);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 02/15] sun4u: ebus QOMify tidy-up

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> The main change here is to introduce the proper TYPE_EBUS/EBUS QOM macros
> and remove the use of DO_UPCAST.
> 
> Alongside this there are some a couple of minor cosmetic changes and a rename
> of pci_ebus_realize() to ebus_realize() since the ebus device is always what
> is effectively a PCI-ISA bridge.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/sparc64/sun4u.c |   19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 1672f25..394b7d6 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -81,11 +81,16 @@ struct hwdef {
>  };
>  
>  typedef struct EbusState {
> -    PCIDevice pci_dev;
> +    /*< private >*/
> +    PCIDevice parent_obj;
> +
>      MemoryRegion bar0;
>      MemoryRegion bar1;
>  } EbusState;
>  
> +#define TYPE_EBUS "ebus"
> +#define EBUS(obj) OBJECT_CHECK(EbusState, (obj), TYPE_EBUS)
> +
>  void DMA_init(ISABus *bus, int high_page_enable)
>  {
>  }
> @@ -236,9 +241,9 @@ pci_ebus_init(PCIDevice *pci_dev, qemu_irq *irqs)
>      return isa_bus;
>  }
>  
> -static void pci_ebus_realize(PCIDevice *pci_dev, Error **errp)
> +static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
> -    EbusState *s = DO_UPCAST(EbusState, pci_dev, pci_dev);
> +    EbusState *s = EBUS(pci_dev);
>  
>      if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
>                       pci_address_space_io(pci_dev), errp)) {
> @@ -264,7 +269,7 @@ static void ebus_class_init(ObjectClass *klass, void *data)
>  {
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->realize = pci_ebus_realize;
> +    k->realize = ebus_realize;
>      k->vendor_id = PCI_VENDOR_ID_SUN;
>      k->device_id = PCI_DEVICE_ID_SUN_EBUS;
>      k->revision = 0x01;
> @@ -272,10 +277,10 @@ static void ebus_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo ebus_info = {
> -    .name          = "ebus",
> +    .name          = TYPE_EBUS,
>      .parent        = TYPE_PCI_DEVICE,
> -    .instance_size = sizeof(EbusState),
>      .class_init    = ebus_class_init,
> +    .instance_size = sizeof(EbusState),
>      .interfaces = (InterfaceInfo[]) {
>          { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>          { },
> @@ -463,7 +468,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      pci_busA->slot_reserved_mask = 0xfffffff1;
>      pci_busB->slot_reserved_mask = 0xfffffff0;
>  
> -    ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, "ebus");
> +    ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
>      qdev_init_nofail(DEVICE(ebus));
>  
>      isa_bus = pci_ebus_init(ebus, pbm_irqs);
> 

[Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Mark Cave-Ayland]

Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
should be contained within the PCI bridge itself.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc64/sun4u.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 394b7d6..63b4aaa 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -84,6 +84,7 @@ typedef struct EbusState {
     /*< private >*/
     PCIDevice parent_obj;
 
+    ISABus *isa_bus;
     MemoryRegion bar0;
     MemoryRegion bar1;
 } EbusState;
@@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
     EbusState *s = EBUS(pci_dev);
 
-    if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
-                     pci_address_space_io(pci_dev), errp)) {
+    s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
+                             pci_address_space_io(pci_dev), errp);
+    if (!s->isa_bus) {
+        error_setg(errp, "unable to instantiate EBUS ISA bus");
         return;
     }
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
> should be contained within the PCI bridge itself.

While it's like that on the Sabre chipset, the Spitfire chipset (which
I hope to add at some point) has the EBus, but no PCI, so maybe it's
better to model it separately.
On the other hand, the Spitfire has different EBus devices
(particularly different type of the serial ports), so I'm not sure.

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/sparc64/sun4u.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 394b7d6..63b4aaa 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -84,6 +84,7 @@ typedef struct EbusState {
>      /*< private >*/
>      PCIDevice parent_obj;
>
> +    ISABus *isa_bus;
>      MemoryRegion bar0;
>      MemoryRegion bar1;
>  } EbusState;
> @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
>
> -    if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> -                     pci_address_space_io(pci_dev), errp)) {
> +    s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> +                             pci_address_space_io(pci_dev), errp);
> +    if (!s->isa_bus) {
> +        error_setg(errp, "unable to instantiate EBUS ISA bus");
>          return;
>      }
>
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Mark Cave-Ayland]

On 17/11/17 14:53, Artyom Tarasenko wrote:

> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
>> should be contained within the PCI bridge itself.
> 
> While it's like that on the Sabre chipset, the Spitfire chipset (which
> I hope to add at some point) has the EBus, but no PCI, so maybe it's
> better to model it separately.
> On the other hand, the Spitfire has different EBus devices
> (particularly different type of the serial ports), so I'm not sure.

Oh I didn't realise you had more plans in this area :)  Any idea when
you'll be able to work on the them? TBH as you probably already know,
even the patchset in its current form with the ISA bus encapsulation is
so much better than what is already there, so I'd prefer to merge it and
help you work through any problems later unless you feel particularly
strongly?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 4:46 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 17/11/17 14:53, Artyom Tarasenko wrote:
>
>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
>>> should be contained within the PCI bridge itself.
>>
>> While it's like that on the Sabre chipset, the Spitfire chipset (which
>> I hope to add at some point) has the EBus, but no PCI, so maybe it's
>> better to model it separately.
>> On the other hand, the Spitfire has different EBus devices
>> (particularly different type of the serial ports), so I'm not sure.
>
> Oh I didn't realise you had more plans in this area :)  Any idea when
> you'll be able to work on the them?

After I make AIX boot. :-)

> TBH as you probably already know,
> even the patchset in its current form with the ISA bus encapsulation is
> so much better than what is already there, so I'd prefer to merge it and
> help you work through any problems later unless you feel particularly
> strongly?

Ok, let's do it.

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
> should be contained within the PCI bridge itself.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/sparc64/sun4u.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 394b7d6..63b4aaa 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -84,6 +84,7 @@ typedef struct EbusState {
>      /*< private >*/
>      PCIDevice parent_obj;
>  
> +    ISABus *isa_bus;
>      MemoryRegion bar0;
>      MemoryRegion bar1;
>  } EbusState;
> @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
>  
> -    if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> -                     pci_address_space_io(pci_dev), errp)) {
> +    s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> +                             pci_address_space_io(pci_dev), errp);
> +    if (!s->isa_bus) {
> +        error_setg(errp, "unable to instantiate EBUS ISA bus");
>          return;
>      }
>  
> 

[Qemu-devel] [PATCH 04/15] sun4u: remove pci_ebus_init() function

[Mark Cave-Ayland]

This is initialisation that should really take place in the ebus realize
function. As part of this we also rework the ebus IRQ mapping so that
instead of having to pass in the array of pbm_irqs, we obtain a reference
to them by looking up the APB device during ebus realize.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |    4 +---
 hw/sparc64/sun4u.c        |   29 ++++++++++++++---------------
 include/hw/pci-host/apb.h |    3 +--
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index f743a4e..b0f80f6 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -614,8 +614,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
 
 PCIBus *pci_apb_init(hwaddr special_base,
                      hwaddr mem_base,
-                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB,
-                     qemu_irq **pbm_irqs)
+                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -646,7 +645,6 @@ PCIBus *pci_apb_init(hwaddr special_base,
     memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
     memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
 
-    *pbm_irqs = d->pbm_irqs;
     d->ivec_irqs = ivec_irqs;
 
     pci_create_simple(phb->bus, 0, "pbm-pci");
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 63b4aaa..f3203ea 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -230,21 +230,11 @@ static void isa_irq_handler(void *opaque, int n, int level)
 }
 
 /* EBUS (Eight bit bus) bridge */
-static ISABus *
-pci_ebus_init(PCIDevice *pci_dev, qemu_irq *irqs)
-{
-    qemu_irq *isa_irq;
-    ISABus *isa_bus;
-
-    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
-    isa_irq = qemu_allocate_irqs(isa_irq_handler, irqs, 16);
-    isa_bus_irqs(isa_bus, isa_irq);
-    return isa_bus;
-}
-
 static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
     EbusState *s = EBUS(pci_dev);
+    APBState *apb;
+    qemu_irq *isa_irq;
 
     s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
                              pci_address_space_io(pci_dev), errp);
@@ -253,6 +243,15 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
+    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));
+    if (!apb) {
+        error_setg(errp, "unable to locate APB PCI host bridge");
+        return;
+    }
+
+    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
+    isa_bus_irqs(s->isa_bus, isa_irq);
+
     pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
     pci_dev->config[0x05] = 0x00;
     pci_dev->config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
@@ -443,7 +442,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     PCIDevice *ebus, *pci_dev;
     ISABus *isa_bus;
     SysBusDevice *s;
-    qemu_irq *ivec_irqs, *pbm_irqs;
+    qemu_irq *ivec_irqs;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DriveInfo *fd[MAX_FD];
     DeviceState *dev;
@@ -462,7 +461,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
     pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
-                           &pci_busB, &pbm_irqs);
+                           &pci_busB);
 
     /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
        reserved (leaving no slots free after on-board devices) however slots
@@ -474,7 +473,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
     qdev_init_nofail(DEVICE(ebus));
 
-    isa_bus = pci_ebus_init(ebus, pbm_irqs);
+    isa_bus = EBUS(ebus)->isa_bus;
 
     i = 0;
     if (hwdef->console_serial_base) {
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index 5d39c03..35d7d5a 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -91,6 +91,5 @@ typedef struct PBMPCIBridge {
 
 PCIBus *pci_apb_init(hwaddr special_base,
                      hwaddr mem_base,
-                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3,
-                     qemu_irq **pbm_irqs);
+                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
 #endif
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 04/15] sun4u: remove pci_ebus_init() function

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This is initialisation that should really take place in the ebus realize
> function. As part of this we also rework the ebus IRQ mapping so that
> instead of having to pass in the array of pbm_irqs, we obtain a reference
> to them by looking up the APB device during ebus realize.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/pci-host/apb.c         |    4 +---
>  hw/sparc64/sun4u.c        |   29 ++++++++++++++---------------
>  include/hw/pci-host/apb.h |    3 +--
>  3 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index f743a4e..b0f80f6 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -614,8 +614,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB,
> -                     qemu_irq **pbm_irqs)
> +                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -646,7 +645,6 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
>      memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
>
> -    *pbm_irqs = d->pbm_irqs;
>      d->ivec_irqs = ivec_irqs;
>
>      pci_create_simple(phb->bus, 0, "pbm-pci");
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 63b4aaa..f3203ea 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -230,21 +230,11 @@ static void isa_irq_handler(void *opaque, int n, int level)
>  }
>
>  /* EBUS (Eight bit bus) bridge */
> -static ISABus *
> -pci_ebus_init(PCIDevice *pci_dev, qemu_irq *irqs)
> -{
> -    qemu_irq *isa_irq;
> -    ISABus *isa_bus;
> -
> -    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
> -    isa_irq = qemu_allocate_irqs(isa_irq_handler, irqs, 16);
> -    isa_bus_irqs(isa_bus, isa_irq);
> -    return isa_bus;
> -}
> -
>  static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
> +    APBState *apb;
> +    qemu_irq *isa_irq;
>
>      s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
>                               pci_address_space_io(pci_dev), errp);
> @@ -253,6 +243,15 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>          return;
>      }
>
> +    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));
> +    if (!apb) {
> +        error_setg(errp, "unable to locate APB PCI host bridge");
> +        return;
> +    }
> +
> +    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
> +    isa_bus_irqs(s->isa_bus, isa_irq);
> +
>      pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
>      pci_dev->config[0x05] = 0x00;
>      pci_dev->config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> @@ -443,7 +442,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      PCIDevice *ebus, *pci_dev;
>      ISABus *isa_bus;
>      SysBusDevice *s;
> -    qemu_irq *ivec_irqs, *pbm_irqs;
> +    qemu_irq *ivec_irqs;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      DriveInfo *fd[MAX_FD];
>      DeviceState *dev;
> @@ -462,7 +461,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>
>      ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
>      pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
> -                           &pci_busB, &pbm_irqs);
> +                           &pci_busB);
>
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
>         reserved (leaving no slots free after on-board devices) however slots
> @@ -474,7 +473,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
>      qdev_init_nofail(DEVICE(ebus));
>
> -    isa_bus = pci_ebus_init(ebus, pbm_irqs);
> +    isa_bus = EBUS(ebus)->isa_bus;
>
>      i = 0;
>      if (hwdef->console_serial_base) {
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index 5d39c03..35d7d5a 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -91,6 +91,5 @@ typedef struct PBMPCIBridge {
>
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3,
> -                     qemu_irq **pbm_irqs);
> +                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
>  #endif
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 04/15] sun4u: remove pci_ebus_init() function

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This is initialisation that should really take place in the ebus realize
> function. As part of this we also rework the ebus IRQ mapping so that
> instead of having to pass in the array of pbm_irqs, we obtain a reference
> to them by looking up the APB device during ebus realize.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |    4 +---
>  hw/sparc64/sun4u.c        |   29 ++++++++++++++---------------
>  include/hw/pci-host/apb.h |    3 +--
>  3 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index f743a4e..b0f80f6 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -614,8 +614,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>  
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB,
> -                     qemu_irq **pbm_irqs)
> +                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -646,7 +645,6 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
>      memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
>  
> -    *pbm_irqs = d->pbm_irqs;
>      d->ivec_irqs = ivec_irqs;
>  
>      pci_create_simple(phb->bus, 0, "pbm-pci");
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 63b4aaa..f3203ea 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -230,21 +230,11 @@ static void isa_irq_handler(void *opaque, int n, int level)
>  }
>  
>  /* EBUS (Eight bit bus) bridge */
> -static ISABus *
> -pci_ebus_init(PCIDevice *pci_dev, qemu_irq *irqs)
> -{
> -    qemu_irq *isa_irq;
> -    ISABus *isa_bus;
> -
> -    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
> -    isa_irq = qemu_allocate_irqs(isa_irq_handler, irqs, 16);
> -    isa_bus_irqs(isa_bus, isa_irq);
> -    return isa_bus;
> -}
> -
>  static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
> +    APBState *apb;
> +    qemu_irq *isa_irq;
>  
>      s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
>                               pci_address_space_io(pci_dev), errp);
> @@ -253,6 +243,15 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>          return;
>      }
>  
> +    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));

:)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +    if (!apb) {
> +        error_setg(errp, "unable to locate APB PCI host bridge");
> +        return;
> +    }
> +
> +    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
> +    isa_bus_irqs(s->isa_bus, isa_irq);
> +
>      pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
>      pci_dev->config[0x05] = 0x00;
>      pci_dev->config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> @@ -443,7 +442,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      PCIDevice *ebus, *pci_dev;
>      ISABus *isa_bus;
>      SysBusDevice *s;
> -    qemu_irq *ivec_irqs, *pbm_irqs;
> +    qemu_irq *ivec_irqs;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      DriveInfo *fd[MAX_FD];
>      DeviceState *dev;
> @@ -462,7 +461,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>  
>      ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
>      pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
> -                           &pci_busB, &pbm_irqs);
> +                           &pci_busB);
>  
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
>         reserved (leaving no slots free after on-board devices) however slots
> @@ -474,7 +473,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
>      qdev_init_nofail(DEVICE(ebus));
>  
> -    isa_bus = pci_ebus_init(ebus, pbm_irqs);
> +    isa_bus = EBUS(ebus)->isa_bus;
>  
>      i = 0;
>      if (hwdef->console_serial_base) {
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index 5d39c03..35d7d5a 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -91,6 +91,5 @@ typedef struct PBMPCIBridge {
>  
>  PCIBus *pci_apb_init(hwaddr special_base,
>                       hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3,
> -                     qemu_irq **pbm_irqs);
> +                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
>  #endif
> 

[Qemu-devel] [PATCH 05/15] sun4u: move initialisation of all ISABus devices into ebus_realize()

[Mark Cave-Ayland]

This belongs in the PCI-ISA bridge rather than at the machine level.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc64/sun4u.c |   78 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index f3203ea..b441f1e 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -85,6 +85,7 @@ typedef struct EbusState {
     PCIDevice parent_obj;
 
     ISABus *isa_bus;
+    uint64_t console_serial_base;
     MemoryRegion bar0;
     MemoryRegion bar1;
 } EbusState;
@@ -234,7 +235,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
     EbusState *s = EBUS(pci_dev);
     APBState *apb;
+    DeviceState *dev;
     qemu_irq *isa_irq;
+    DriveInfo *fd[MAX_FD];
+    int i;
 
     s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
                              pci_address_space_io(pci_dev), errp);
@@ -252,6 +256,38 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
     isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
     isa_bus_irqs(s->isa_bus, isa_irq);
 
+    /* Serial ports */
+    i = 0;
+    if (s->console_serial_base) {
+        serial_mm_init(pci_address_space(pci_dev), s->console_serial_base,
+                       0, NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN);
+        i++;
+    }
+    serial_hds_isa_init(s->isa_bus, i, MAX_SERIAL_PORTS);
+
+    /* Parallel ports */
+    parallel_hds_isa_init(s->isa_bus, MAX_PARALLEL_PORTS);
+
+    /* Keyboard */
+    isa_create_simple(s->isa_bus, "i8042");
+
+    /* Floppy */
+    for (i = 0; i < MAX_FD; i++) {
+        fd[i] = drive_get(IF_FLOPPY, 0, i);
+    }
+    dev = DEVICE(isa_create(s->isa_bus, TYPE_ISA_FDC));
+    if (fd[0]) {
+        qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fd[0]),
+                            &error_abort);
+    }
+    if (fd[1]) {
+        qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fd[1]),
+                            &error_abort);
+    }
+    qdev_prop_set_uint32(dev, "dma", -1);
+    qdev_init_nofail(dev);
+
+    /* PCI */
     pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
     pci_dev->config[0x05] = 0x00;
     pci_dev->config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
@@ -267,15 +303,23 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
     pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
 }
 
+static Property ebus_properties[] = {
+    DEFINE_PROP_UINT64("console-serial-base", EbusState,
+                       console_serial_base, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ebus_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->realize = ebus_realize;
     k->vendor_id = PCI_VENDOR_ID_SUN;
     k->device_id = PCI_DEVICE_ID_SUN_EBUS;
     k->revision = 0x01;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
+    dc->props = ebus_properties;
 }
 
 static const TypeInfo ebus_info = {
@@ -440,11 +484,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, kernel_entry;
     PCIBus *pci_bus, *pci_busA, *pci_busB;
     PCIDevice *ebus, *pci_dev;
-    ISABus *isa_bus;
     SysBusDevice *s;
     qemu_irq *ivec_irqs;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    DriveInfo *fd[MAX_FD];
     DeviceState *dev;
     FWCfgState *fw_cfg;
     NICInfo *nd;
@@ -471,20 +513,10 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     pci_busB->slot_reserved_mask = 0xfffffff0;
 
     ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
+    qdev_prop_set_uint64(DEVICE(ebus), "console-serial-base",
+                         hwdef->console_serial_base);
     qdev_init_nofail(DEVICE(ebus));
 
-    isa_bus = EBUS(ebus)->isa_bus;
-
-    i = 0;
-    if (hwdef->console_serial_base) {
-        serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
-                       NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN);
-        i++;
-    }
-
-    serial_hds_isa_init(isa_bus, i, MAX_SERIAL_PORTS);
-    parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
-
     pci_dev = pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
 
     memset(&macaddr, 0, sizeof(MACAddr));
@@ -523,24 +555,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     qdev_init_nofail(&pci_dev->qdev);
     pci_ide_create_devs(pci_dev, hd);
 
-    isa_create_simple(isa_bus, "i8042");
-
-    /* Floppy */
-    for(i = 0; i < MAX_FD; i++) {
-        fd[i] = drive_get(IF_FLOPPY, 0, i);
-    }
-    dev = DEVICE(isa_create(isa_bus, TYPE_ISA_FDC));
-    if (fd[0]) {
-        qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fd[0]),
-                            &error_abort);
-    }
-    if (fd[1]) {
-        qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fd[1]),
-                            &error_abort);
-    }
-    qdev_prop_set_uint32(dev, "dma", -1);
-    qdev_init_nofail(dev);
-
     /* Map NVRAM into I/O (ebus) space */
     nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
     s = SYS_BUS_DEVICE(nvram);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 05/15] sun4u: move initialisation of all ISABus devices into ebus_realize()

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This belongs in the PCI-ISA bridge rather than at the machine level.

nice, this helps me in another series (clean out i386/pc, refactor
superio devices).

> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/sparc64/sun4u.c |   78 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 46 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index f3203ea..b441f1e 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -85,6 +85,7 @@ typedef struct EbusState {
>      PCIDevice parent_obj;
>  
>      ISABus *isa_bus;
> +    uint64_t console_serial_base;

I'd rather use hwaddr here, anyway:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      MemoryRegion bar0;
>      MemoryRegion bar1;
>  } EbusState;
> @@ -234,7 +235,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
>      APBState *apb;
> +    DeviceState *dev;
>      qemu_irq *isa_irq;
> +    DriveInfo *fd[MAX_FD];
> +    int i;
>  
>      s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
>                               pci_address_space_io(pci_dev), errp);
> @@ -252,6 +256,38 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>      isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
>      isa_bus_irqs(s->isa_bus, isa_irq);
>  
> +    /* Serial ports */
> +    i = 0;
> +    if (s->console_serial_base) {
> +        serial_mm_init(pci_address_space(pci_dev), s->console_serial_base,
> +                       0, NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN);
> +        i++;
> +    }
> +    serial_hds_isa_init(s->isa_bus, i, MAX_SERIAL_PORTS);
> +
> +    /* Parallel ports */
> +    parallel_hds_isa_init(s->isa_bus, MAX_PARALLEL_PORTS);
> +
> +    /* Keyboard */
> +    isa_create_simple(s->isa_bus, "i8042");
> +
> +    /* Floppy */
> +    for (i = 0; i < MAX_FD; i++) {
> +        fd[i] = drive_get(IF_FLOPPY, 0, i);
> +    }
> +    dev = DEVICE(isa_create(s->isa_bus, TYPE_ISA_FDC));
> +    if (fd[0]) {
> +        qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fd[0]),
> +                            &error_abort);
> +    }
> +    if (fd[1]) {
> +        qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fd[1]),
> +                            &error_abort);
> +    }
> +    qdev_prop_set_uint32(dev, "dma", -1);
> +    qdev_init_nofail(dev);
> +
> +    /* PCI */
>      pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
>      pci_dev->config[0x05] = 0x00;
>      pci_dev->config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> @@ -267,15 +303,23 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>      pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
>  }
>  
> +static Property ebus_properties[] = {
> +    DEFINE_PROP_UINT64("console-serial-base", EbusState,
> +                       console_serial_base, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void ebus_class_init(ObjectClass *klass, void *data)
>  {
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      k->realize = ebus_realize;
>      k->vendor_id = PCI_VENDOR_ID_SUN;
>      k->device_id = PCI_DEVICE_ID_SUN_EBUS;
>      k->revision = 0x01;
>      k->class_id = PCI_CLASS_BRIDGE_OTHER;
> +    dc->props = ebus_properties;
>  }
>  
>  static const TypeInfo ebus_info = {
> @@ -440,11 +484,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, kernel_entry;
>      PCIBus *pci_bus, *pci_busA, *pci_busB;
>      PCIDevice *ebus, *pci_dev;
> -    ISABus *isa_bus;
>      SysBusDevice *s;
>      qemu_irq *ivec_irqs;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> -    DriveInfo *fd[MAX_FD];
>      DeviceState *dev;
>      FWCfgState *fw_cfg;
>      NICInfo *nd;
> @@ -471,20 +513,10 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      pci_busB->slot_reserved_mask = 0xfffffff0;
>  
>      ebus = pci_create_multifunction(pci_busA, PCI_DEVFN(1, 0), true, TYPE_EBUS);
> +    qdev_prop_set_uint64(DEVICE(ebus), "console-serial-base",
> +                         hwdef->console_serial_base);
>      qdev_init_nofail(DEVICE(ebus));
>  
> -    isa_bus = EBUS(ebus)->isa_bus;
> -
> -    i = 0;
> -    if (hwdef->console_serial_base) {
> -        serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
> -                       NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN);
> -        i++;
> -    }
> -
> -    serial_hds_isa_init(isa_bus, i, MAX_SERIAL_PORTS);
> -    parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
> -
>      pci_dev = pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
>  
>      memset(&macaddr, 0, sizeof(MACAddr));
> @@ -523,24 +555,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      qdev_init_nofail(&pci_dev->qdev);
>      pci_ide_create_devs(pci_dev, hd);
>  
> -    isa_create_simple(isa_bus, "i8042");
> -
> -    /* Floppy */
> -    for(i = 0; i < MAX_FD; i++) {
> -        fd[i] = drive_get(IF_FLOPPY, 0, i);
> -    }
> -    dev = DEVICE(isa_create(isa_bus, TYPE_ISA_FDC));
> -    if (fd[0]) {
> -        qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fd[0]),
> -                            &error_abort);
> -    }
> -    if (fd[1]) {
> -        qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fd[1]),
> -                            &error_abort);
> -    }
> -    qdev_prop_set_uint32(dev, "dma", -1);
> -    qdev_init_nofail(dev);
> -
>      /* Map NVRAM into I/O (ebus) space */
>      nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
>      s = SYS_BUS_DEVICE(nvram);
> 

Re: [Qemu-devel] [PATCH 05/15] sun4u: move initialisation of all ISABus devices into ebus_realize()

[Mark Cave-Ayland]

On 20/11/17 00:47, Philippe Mathieu-Daudé wrote:

> On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
>> This belongs in the PCI-ISA bridge rather than at the machine level.
> 
> nice, this helps me in another series (clean out i386/pc, refactor
> superio devices).

Awesome!

>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/sparc64/sun4u.c |   78 +++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 46 insertions(+), 32 deletions(-)
>>
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index f3203ea..b441f1e 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -85,6 +85,7 @@ typedef struct EbusState {
>>      PCIDevice parent_obj;
>>  
>>      ISABus *isa_bus;
>> +    uint64_t console_serial_base;
> 
> I'd rather use hwaddr here, anyway:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Yeah it could potentially be either, however to me it feels nicer to
have the value as uint64_t so it "matches" qdev_prop_set_uint64().


ATB,

Mark.

[Qemu-devel] [PATCH 06/15] apb: APB QOMify tidy-up

[Mark Cave-Ayland]

Use DeviceClass rather than SysBusDeviceClass in pbm_host_class_init() and
adjust pci_pbm_init_device() accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index b0f80f6..c7837ef 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -705,12 +705,12 @@ static const MemoryRegionOps pci_config_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static int pci_pbm_init_device(SysBusDevice *dev)
+static int pci_pbm_init_device(DeviceState *dev)
 {
-    APBState *s;
+    APBState *s = APB_DEVICE(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
     unsigned int i;
 
-    s = APB_DEVICE(dev);
     for (i = 0; i < 8; i++) {
         s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
     }
@@ -728,18 +728,18 @@ static int pci_pbm_init_device(SysBusDevice *dev)
     memory_region_init_io(&s->apb_config, OBJECT(s), &apb_config_ops, s,
                           "apb-config", 0x10000);
     /* at region 0 */
-    sysbus_init_mmio(dev, &s->apb_config);
+    sysbus_init_mmio(sbd, &s->apb_config);
 
     memory_region_init_io(&s->pci_config, OBJECT(s), &pci_config_ops, s,
                           "apb-pci-config", 0x1000000);
     /* at region 1 */
-    sysbus_init_mmio(dev, &s->pci_config);
+    sysbus_init_mmio(sbd, &s->pci_config);
 
     /* pci_ioport */
     memory_region_init(&s->pci_ioport, OBJECT(s), "apb-pci-ioport", 0x1000000);
 
     /* at region 2 */
-    sysbus_init_mmio(dev, &s->pci_ioport);
+    sysbus_init_mmio(sbd, &s->pci_ioport);
 
     return 0;
 }
@@ -783,11 +783,10 @@ static const TypeInfo pbm_pci_host_info = {
 static void pbm_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = pci_pbm_init_device;
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->init = pci_pbm_init_device;
     dc->reset = pci_pbm_reset;
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
 static const TypeInfo pbm_host_info = {
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 06/15] apb: APB QOMify tidy-up

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Use DeviceClass rather than SysBusDeviceClass in pbm_host_class_init() and
> adjust pci_pbm_init_device() accordingly.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/pci-host/apb.c |   17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index b0f80f6..c7837ef 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -705,12 +705,12 @@ static const MemoryRegionOps pci_config_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> -static int pci_pbm_init_device(SysBusDevice *dev)
> +static int pci_pbm_init_device(DeviceState *dev)
>  {
> -    APBState *s;
> +    APBState *s = APB_DEVICE(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>      unsigned int i;
>  
> -    s = APB_DEVICE(dev);
>      for (i = 0; i < 8; i++) {
>          s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
>      }
> @@ -728,18 +728,18 @@ static int pci_pbm_init_device(SysBusDevice *dev)
>      memory_region_init_io(&s->apb_config, OBJECT(s), &apb_config_ops, s,
>                            "apb-config", 0x10000);
>      /* at region 0 */
> -    sysbus_init_mmio(dev, &s->apb_config);
> +    sysbus_init_mmio(sbd, &s->apb_config);
>  
>      memory_region_init_io(&s->pci_config, OBJECT(s), &pci_config_ops, s,
>                            "apb-pci-config", 0x1000000);
>      /* at region 1 */
> -    sysbus_init_mmio(dev, &s->pci_config);
> +    sysbus_init_mmio(sbd, &s->pci_config);
>  
>      /* pci_ioport */
>      memory_region_init(&s->pci_ioport, OBJECT(s), "apb-pci-ioport", 0x1000000);
>  
>      /* at region 2 */
> -    sysbus_init_mmio(dev, &s->pci_ioport);
> +    sysbus_init_mmio(sbd, &s->pci_ioport);
>  
>      return 0;
>  }
> @@ -783,11 +783,10 @@ static const TypeInfo pbm_pci_host_info = {
>  static void pbm_host_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = pci_pbm_init_device;
> -    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->init = pci_pbm_init_device;
>      dc->reset = pci_pbm_reset;
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
>  static const TypeInfo pbm_host_info = {
> 

[Qemu-devel] [PATCH 07/15] apb: return APBState from pci_apb_init() rather then PCIBus

[Mark Cave-Ayland]

This is a first step towards removing pci_apb_init() completely.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |    8 ++++----
 hw/sparc64/sun4u.c        |    6 ++++--
 include/hw/pci-host/apb.h |    6 +++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index c7837ef..0c70999 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -612,9 +612,9 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
     pci_bridge_update_mappings(PCI_BRIDGE(br));
 }
 
-PCIBus *pci_apb_init(hwaddr special_base,
-                     hwaddr mem_base,
-                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
+APBState *pci_apb_init(hwaddr special_base,
+                       hwaddr mem_base,
+                       qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -675,7 +675,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
     qdev_init_nofail(&pci_dev->qdev);
     *busA = pci_bridge_get_sec_bus(br);
 
-    return phb->bus;
+    return d;
 }
 
 static void pci_pbm_reset(DeviceState *d)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index b441f1e..a64ddc5 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -482,6 +482,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     Nvram *nvram;
     unsigned int i;
     uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, kernel_entry;
+    APBState *apb;
     PCIBus *pci_bus, *pci_busA, *pci_busB;
     PCIDevice *ebus, *pci_dev;
     SysBusDevice *s;
@@ -502,8 +503,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     prom_init(hwdef->prom_addr, bios_name);
 
     ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
-    pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
-                           &pci_busB);
+    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
+                       &pci_busB);
+    pci_bus = PCI_HOST_BRIDGE(apb)->bus;
 
     /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
        reserved (leaving no slots free after on-board devices) however slots
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index 35d7d5a..a4ef51a 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -89,7 +89,7 @@ typedef struct PBMPCIBridge {
 #define PBM_PCI_BRIDGE(obj) \
     OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
 
-PCIBus *pci_apb_init(hwaddr special_base,
-                     hwaddr mem_base,
-                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
+APBState *pci_apb_init(hwaddr special_base,
+                       hwaddr mem_base,
+                       qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
 #endif
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 07/15] apb: return APBState from pci_apb_init() rather then PCIBus

[Philippe Mathieu-Daudé]

"rather then PCIBus" -> "rather than PCIBus"

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This is a first step towards removing pci_apb_init() completely.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/pci-host/apb.c         |    8 ++++----
>  hw/sparc64/sun4u.c        |    6 ++++--
>  include/hw/pci-host/apb.h |    6 +++---
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index c7837ef..0c70999 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -612,9 +612,9 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>      pci_bridge_update_mappings(PCI_BRIDGE(br));
>  }
>  
> -PCIBus *pci_apb_init(hwaddr special_base,
> -                     hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
> +APBState *pci_apb_init(hwaddr special_base,
> +                       hwaddr mem_base,
> +                       qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -675,7 +675,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
>      qdev_init_nofail(&pci_dev->qdev);
>      *busA = pci_bridge_get_sec_bus(br);
>  
> -    return phb->bus;
> +    return d;
>  }
>  
>  static void pci_pbm_reset(DeviceState *d)
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b441f1e..a64ddc5 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -482,6 +482,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      Nvram *nvram;
>      unsigned int i;
>      uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, kernel_entry;
> +    APBState *apb;
>      PCIBus *pci_bus, *pci_busA, *pci_busB;
>      PCIDevice *ebus, *pci_dev;
>      SysBusDevice *s;
> @@ -502,8 +503,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      prom_init(hwdef->prom_addr, bios_name);
>  
>      ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
> -    pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
> -                           &pci_busB);
> +    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
> +                       &pci_busB);
> +    pci_bus = PCI_HOST_BRIDGE(apb)->bus;
>  
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
>         reserved (leaving no slots free after on-board devices) however slots
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index 35d7d5a..a4ef51a 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -89,7 +89,7 @@ typedef struct PBMPCIBridge {
>  #define PBM_PCI_BRIDGE(obj) \
>      OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>  
> -PCIBus *pci_apb_init(hwaddr special_base,
> -                     hwaddr mem_base,
> -                     qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
> +APBState *pci_apb_init(hwaddr special_base,
> +                       hwaddr mem_base,
> +                       qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
>  #endif
> 

[Qemu-devel] [PATCH 08/15] apb: use gpios to wire up the apb device to the SPARC CPU IRQs

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c          |    6 ++----
 hw/sparc64/sparc64.c       |    2 ++
 hw/sparc64/sun4u.c         |   12 ++++++++----
 include/hw/pci-host/apb.h  |    6 ++++--
 include/hw/sparc/sparc64.h |    2 ++
 5 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 0c70999..c0b97e4 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -79,7 +79,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
 #define RESET_WCMASK 0x98000000
 #define RESET_WMASK  0x60000000
 
-#define MAX_IVEC 0x40
 #define NO_IRQ_REQUEST (MAX_IVEC + 1)
 
 static inline void pbm_set_request(APBState *s, unsigned int irq_num)
@@ -614,7 +613,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
 
 APBState *pci_apb_init(hwaddr special_base,
                        hwaddr mem_base,
-                       qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
+                       PCIBus **busA, PCIBus **busB)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -645,8 +644,6 @@ APBState *pci_apb_init(hwaddr special_base,
     memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
     memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
 
-    d->ivec_irqs = ivec_irqs;
-
     pci_create_simple(phb->bus, 0, "pbm-pci");
 
     /* APB IOMMU */
@@ -721,6 +718,7 @@ static int pci_pbm_init_device(DeviceState *dev)
         s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
     }
     s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
+    qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
     s->irq_request = NO_IRQ_REQUEST;
     s->pci_irq_in = 0ULL;
 
diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index 9453e2c..95a06f0 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -350,6 +350,8 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, uint64_t prom_addr)
     uint32_t hstick_frequency = 100 * 1000000;
 
     cpu = SPARC_CPU(cpu_create(cpu_type));
+    qdev_init_gpio_in_named(DEVICE(cpu), sparc64_cpu_set_ivec_irq,
+                            "ivec-irq", IVEC_MAX);
     env = &cpu->env;
 
     env->tick = cpu_timer_create("tick", cpu, tick_irq,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index a64ddc5..2afd3f2 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -486,7 +486,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     PCIBus *pci_bus, *pci_busA, *pci_busB;
     PCIDevice *ebus, *pci_dev;
     SysBusDevice *s;
-    qemu_irq *ivec_irqs;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DeviceState *dev;
     FWCfgState *fw_cfg;
@@ -502,9 +501,14 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     prom_init(hwdef->prom_addr, bios_name);
 
-    ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
-    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
-                       &pci_busB);
+    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, &pci_busA, &pci_busB);
+
+    /* Wire up PCI interrupts to CPU */
+    for (i = 0; i < IVEC_MAX; i++) {
+        qdev_connect_gpio_out_named(DEVICE(apb), "ivec-irq", i,
+            qdev_get_gpio_in_named(DEVICE(cpu), "ivec-irq", i));
+    }
+
     pci_bus = PCI_HOST_BRIDGE(apb)->bus;
 
     /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index a4ef51a..f7ead68 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -50,6 +50,8 @@ typedef struct IOMMUState {
     uint64_t regs[IOMMU_NREGS];
 } IOMMUState;
 
+#define MAX_IVEC 0x40
+
 #define TYPE_APB "pbm"
 
 #define APB_DEVICE(obj) \
@@ -71,7 +73,7 @@ typedef struct APBState {
     uint32_t pci_err_irq_map[4];
     uint32_t obio_irq_map[32];
     qemu_irq *pbm_irqs;
-    qemu_irq *ivec_irqs;
+    qemu_irq ivec_irqs[MAX_IVEC];
     unsigned int irq_request;
     uint32_t reset_control;
     unsigned int nr_resets;
@@ -91,5 +93,5 @@ typedef struct PBMPCIBridge {
 
 APBState *pci_apb_init(hwaddr special_base,
                        hwaddr mem_base,
-                       qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
+                       PCIBus **bus2, PCIBus **bus3);
 #endif
diff --git a/include/hw/sparc/sparc64.h b/include/hw/sparc/sparc64.h
index ca3bb4b..5af4344 100644
--- a/include/hw/sparc/sparc64.h
+++ b/include/hw/sparc/sparc64.h
@@ -1,4 +1,6 @@
 
+#define IVEC_MAX             0x40
+
 SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, uint64_t prom_addr);
 
 void sparc64_cpu_set_ivec_irq(void *opaque, int irq, int level);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 08/15] apb: use gpios to wire up the apb device to the SPARC CPU IRQs

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/pci-host/apb.c          |    6 ++----
>  hw/sparc64/sparc64.c       |    2 ++
>  hw/sparc64/sun4u.c         |   12 ++++++++----
>  include/hw/pci-host/apb.h  |    6 ++++--
>  include/hw/sparc/sparc64.h |    2 ++
>  5 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 0c70999..c0b97e4 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -79,7 +79,6 @@ do { printf("IOMMU: " fmt , ## __VA_ARGS__); } while (0)
>  #define RESET_WCMASK 0x98000000
>  #define RESET_WMASK  0x60000000
>  
> -#define MAX_IVEC 0x40
>  #define NO_IRQ_REQUEST (MAX_IVEC + 1)
>  
>  static inline void pbm_set_request(APBState *s, unsigned int irq_num)
> @@ -614,7 +613,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>  
>  APBState *pci_apb_init(hwaddr special_base,
>                         hwaddr mem_base,
> -                       qemu_irq *ivec_irqs, PCIBus **busA, PCIBus **busB)
> +                       PCIBus **busA, PCIBus **busB)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -645,8 +644,6 @@ APBState *pci_apb_init(hwaddr special_base,
>      memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
>      memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
>  
> -    d->ivec_irqs = ivec_irqs;
> -
>      pci_create_simple(phb->bus, 0, "pbm-pci");
>  
>      /* APB IOMMU */
> @@ -721,6 +718,7 @@ static int pci_pbm_init_device(DeviceState *dev)
>          s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
>      }
>      s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
> +    qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
>      s->irq_request = NO_IRQ_REQUEST;
>      s->pci_irq_in = 0ULL;
>  
> diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
> index 9453e2c..95a06f0 100644
> --- a/hw/sparc64/sparc64.c
> +++ b/hw/sparc64/sparc64.c
> @@ -350,6 +350,8 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, uint64_t prom_addr)
>      uint32_t hstick_frequency = 100 * 1000000;
>  
>      cpu = SPARC_CPU(cpu_create(cpu_type));
> +    qdev_init_gpio_in_named(DEVICE(cpu), sparc64_cpu_set_ivec_irq,
> +                            "ivec-irq", IVEC_MAX);
>      env = &cpu->env;
>  
>      env->tick = cpu_timer_create("tick", cpu, tick_irq,
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index a64ddc5..2afd3f2 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -486,7 +486,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      PCIBus *pci_bus, *pci_busA, *pci_busB;
>      PCIDevice *ebus, *pci_dev;
>      SysBusDevice *s;
> -    qemu_irq *ivec_irqs;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      DeviceState *dev;
>      FWCfgState *fw_cfg;
> @@ -502,9 +501,14 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>  
>      prom_init(hwdef->prom_addr, bios_name);
>  
> -    ivec_irqs = qemu_allocate_irqs(sparc64_cpu_set_ivec_irq, cpu, IVEC_MAX);
> -    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_busA,
> -                       &pci_busB);
> +    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, &pci_busA, &pci_busB);
> +
> +    /* Wire up PCI interrupts to CPU */
> +    for (i = 0; i < IVEC_MAX; i++) {
> +        qdev_connect_gpio_out_named(DEVICE(apb), "ivec-irq", i,
> +            qdev_get_gpio_in_named(DEVICE(cpu), "ivec-irq", i));
> +    }
> +
>      pci_bus = PCI_HOST_BRIDGE(apb)->bus;
>  
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index a4ef51a..f7ead68 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -50,6 +50,8 @@ typedef struct IOMMUState {
>      uint64_t regs[IOMMU_NREGS];
>  } IOMMUState;
>  
> +#define MAX_IVEC 0x40
> +
>  #define TYPE_APB "pbm"
>  
>  #define APB_DEVICE(obj) \
> @@ -71,7 +73,7 @@ typedef struct APBState {
>      uint32_t pci_err_irq_map[4];
>      uint32_t obio_irq_map[32];
>      qemu_irq *pbm_irqs;
> -    qemu_irq *ivec_irqs;
> +    qemu_irq ivec_irqs[MAX_IVEC];
>      unsigned int irq_request;
>      uint32_t reset_control;
>      unsigned int nr_resets;
> @@ -91,5 +93,5 @@ typedef struct PBMPCIBridge {
>  
>  APBState *pci_apb_init(hwaddr special_base,
>                         hwaddr mem_base,
> -                       qemu_irq *ivec_irqs, PCIBus **bus2, PCIBus **bus3);
> +                       PCIBus **bus2, PCIBus **bus3);
>  #endif
> diff --git a/include/hw/sparc/sparc64.h b/include/hw/sparc/sparc64.h
> index ca3bb4b..5af4344 100644
> --- a/include/hw/sparc/sparc64.h
> +++ b/include/hw/sparc/sparc64.h
> @@ -1,4 +1,6 @@
>  
> +#define IVEC_MAX             0x40
> +
>  SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, uint64_t prom_addr);
>  
>  void sparc64_cpu_set_ivec_irq(void *opaque, int irq, int level);
> 

[Qemu-devel] [PATCH 09/15] apb: move the two secondary PCI bridges objects into APBState

[Mark Cave-Ayland]

This enables us to remove these parameters from pci_apb_init().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |   14 +++++---------
 hw/sparc64/sun4u.c        |    5 ++++-
 include/hw/pci-host/apb.h |    5 +++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index c0b97e4..823661a 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -612,8 +612,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
 }
 
 APBState *pci_apb_init(hwaddr special_base,
-                       hwaddr mem_base,
-                       PCIBus **busA, PCIBus **busB)
+                       hwaddr mem_base)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -621,7 +620,6 @@ APBState *pci_apb_init(hwaddr special_base,
     APBState *d;
     IOMMUState *is;
     PCIDevice *pci_dev;
-    PCIBridge *br;
 
     /* Ultrasparc PBM main bus */
     dev = qdev_create(NULL, TYPE_APB);
@@ -659,18 +657,16 @@ APBState *pci_apb_init(hwaddr special_base,
     /* APB secondary busses */
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
                                    TYPE_PBM_PCI_BRIDGE);
-    br = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(br, "pciB", pci_pbm_map_irq);
+    d->bridgeB = PCI_BRIDGE(pci_dev);
+    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
     qdev_init_nofail(&pci_dev->qdev);
-    *busB = pci_bridge_get_sec_bus(br);
 
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
                                    TYPE_PBM_PCI_BRIDGE);
-    br = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(br, "pciA", pci_pbm_map_irq);
+    d->bridgeA = PCI_BRIDGE(pci_dev);
+    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
     qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
     qdev_init_nofail(&pci_dev->qdev);
-    *busA = pci_bridge_get_sec_bus(br);
 
     return d;
 }
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 2afd3f2..47952be 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -27,6 +27,7 @@
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/apb.h"
 #include "hw/i386/pc.h"
@@ -501,7 +502,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     prom_init(hwdef->prom_addr, bios_name);
 
-    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, &pci_busA, &pci_busB);
+    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
 
     /* Wire up PCI interrupts to CPU */
     for (i = 0; i < IVEC_MAX; i++) {
@@ -510,6 +511,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     }
 
     pci_bus = PCI_HOST_BRIDGE(apb)->bus;
+    pci_busA = pci_bridge_get_sec_bus(apb->bridgeA);
+    pci_busB = pci_bridge_get_sec_bus(apb->bridgeB);
 
     /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
        reserved (leaving no slots free after on-board devices) however slots
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index f7ead68..ae15d8c 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -68,6 +68,8 @@ typedef struct APBState {
     MemoryRegion pci_ioport;
     uint64_t pci_irq_in;
     IOMMUState iommu;
+    PCIBridge *bridgeA;
+    PCIBridge *bridgeB;
     uint32_t pci_control[16];
     uint32_t pci_irq_map[8];
     uint32_t pci_err_irq_map[4];
@@ -92,6 +94,5 @@ typedef struct PBMPCIBridge {
     OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
 
 APBState *pci_apb_init(hwaddr special_base,
-                       hwaddr mem_base,
-                       PCIBus **bus2, PCIBus **bus3);
+                       hwaddr mem_base);
 #endif
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 09/15] apb: move the two secondary PCI bridges objects into APBState

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This enables us to remove these parameters from pci_apb_init().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/pci-host/apb.c         |   14 +++++---------
>  hw/sparc64/sun4u.c        |    5 ++++-
>  include/hw/pci-host/apb.h |    5 +++--
>  3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index c0b97e4..823661a 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -612,8 +612,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>  }
>
>  APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base,
> -                       PCIBus **busA, PCIBus **busB)
> +                       hwaddr mem_base)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -621,7 +620,6 @@ APBState *pci_apb_init(hwaddr special_base,
>      APBState *d;
>      IOMMUState *is;
>      PCIDevice *pci_dev;
> -    PCIBridge *br;
>
>      /* Ultrasparc PBM main bus */
>      dev = qdev_create(NULL, TYPE_APB);
> @@ -659,18 +657,16 @@ APBState *pci_apb_init(hwaddr special_base,
>      /* APB secondary busses */
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    br = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(br, "pciB", pci_pbm_map_irq);
> +    d->bridgeB = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
> -    *busB = pci_bridge_get_sec_bus(br);
>
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    br = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(br, "pciA", pci_pbm_map_irq);
> +    d->bridgeA = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
>      qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
>      qdev_init_nofail(&pci_dev->qdev);
> -    *busA = pci_bridge_get_sec_bus(br);
>
>      return d;
>  }
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 2afd3f2..47952be 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -27,6 +27,7 @@
>  #include "cpu.h"
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/apb.h"
>  #include "hw/i386/pc.h"
> @@ -501,7 +502,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>
>      prom_init(hwdef->prom_addr, bios_name);
>
> -    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, &pci_busA, &pci_busB);
> +    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
>
>      /* Wire up PCI interrupts to CPU */
>      for (i = 0; i < IVEC_MAX; i++) {
> @@ -510,6 +511,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      }
>
>      pci_bus = PCI_HOST_BRIDGE(apb)->bus;
> +    pci_busA = pci_bridge_get_sec_bus(apb->bridgeA);
> +    pci_busB = pci_bridge_get_sec_bus(apb->bridgeB);
>
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
>         reserved (leaving no slots free after on-board devices) however slots
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index f7ead68..ae15d8c 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -68,6 +68,8 @@ typedef struct APBState {
>      MemoryRegion pci_ioport;
>      uint64_t pci_irq_in;
>      IOMMUState iommu;
> +    PCIBridge *bridgeA;
> +    PCIBridge *bridgeB;
>      uint32_t pci_control[16];
>      uint32_t pci_irq_map[8];
>      uint32_t pci_err_irq_map[4];
> @@ -92,6 +94,5 @@ typedef struct PBMPCIBridge {
>      OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>
>  APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base,
> -                       PCIBus **bus2, PCIBus **bus3);
> +                       hwaddr mem_base);
>  #endif
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 09/15] apb: move the two secondary PCI bridges objects into APBState

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This enables us to remove these parameters from pci_apb_init().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |   14 +++++---------
>  hw/sparc64/sun4u.c        |    5 ++++-
>  include/hw/pci-host/apb.h |    5 +++--
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index c0b97e4..823661a 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -612,8 +612,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>  }
>  
>  APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base,
> -                       PCIBus **busA, PCIBus **busB)
> +                       hwaddr mem_base)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -621,7 +620,6 @@ APBState *pci_apb_init(hwaddr special_base,
>      APBState *d;
>      IOMMUState *is;
>      PCIDevice *pci_dev;
> -    PCIBridge *br;
>  
>      /* Ultrasparc PBM main bus */
>      dev = qdev_create(NULL, TYPE_APB);
> @@ -659,18 +657,16 @@ APBState *pci_apb_init(hwaddr special_base,
>      /* APB secondary busses */
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    br = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(br, "pciB", pci_pbm_map_irq);
> +    d->bridgeB = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
> -    *busB = pci_bridge_get_sec_bus(br);
>  
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    br = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(br, "pciA", pci_pbm_map_irq);
> +    d->bridgeA = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
>      qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
>      qdev_init_nofail(&pci_dev->qdev);
> -    *busA = pci_bridge_get_sec_bus(br);
>  
>      return d;
>  }
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 2afd3f2..47952be 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -27,6 +27,7 @@
>  #include "cpu.h"
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/apb.h"
>  #include "hw/i386/pc.h"
> @@ -501,7 +502,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>  
>      prom_init(hwdef->prom_addr, bios_name);
>  
> -    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, &pci_busA, &pci_busB);
> +    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
>  
>      /* Wire up PCI interrupts to CPU */
>      for (i = 0; i < IVEC_MAX; i++) {
> @@ -510,6 +511,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      }
>  
>      pci_bus = PCI_HOST_BRIDGE(apb)->bus;
> +    pci_busA = pci_bridge_get_sec_bus(apb->bridgeA);
> +    pci_busB = pci_bridge_get_sec_bus(apb->bridgeB);

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  
>      /* Only in-built Simba PBMs can exist on the root bus, slot 0 on busA is
>         reserved (leaving no slots free after on-board devices) however slots
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index f7ead68..ae15d8c 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -68,6 +68,8 @@ typedef struct APBState {
>      MemoryRegion pci_ioport;
>      uint64_t pci_irq_in;
>      IOMMUState iommu;
> +    PCIBridge *bridgeA;
> +    PCIBridge *bridgeB;
>      uint32_t pci_control[16];
>      uint32_t pci_irq_map[8];
>      uint32_t pci_err_irq_map[4];
> @@ -92,6 +94,5 @@ typedef struct PBMPCIBridge {
>      OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>  
>  APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base,
> -                       PCIBus **bus2, PCIBus **bus3);
> +                       hwaddr mem_base);
>  #endif
> 

[Qemu-devel] [PATCH 10/15] apb: remove pci_apb_init() and instantiate APB device using qdev

[Mark Cave-Ayland]

By making the special_base and mem_base values qdev properties, we can move
the remaining parts of pci_apb_init() into the pbm init() and realize()
functions.

This finally allows us to instantiate the APB directly using standard qdev
create/init functions in sun4u.c.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |  123 ++++++++++++++++++++++-----------------------
 hw/sparc64/sun4u.c        |    6 ++-
 include/hw/pci-host/apb.h |    4 +-
 3 files changed, 68 insertions(+), 65 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 823661a..6c20285 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -611,41 +611,56 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
     pci_bridge_update_mappings(PCI_BRIDGE(br));
 }
 
-APBState *pci_apb_init(hwaddr special_base,
-                       hwaddr mem_base)
+static void pci_pbm_reset(DeviceState *d)
 {
-    DeviceState *dev;
-    SysBusDevice *s;
-    PCIHostState *phb;
-    APBState *d;
-    IOMMUState *is;
+    unsigned int i;
+    APBState *s = APB_DEVICE(d);
+
+    for (i = 0; i < 8; i++) {
+        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
+    }
+    for (i = 0; i < 32; i++) {
+        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
+    }
+
+    s->irq_request = NO_IRQ_REQUEST;
+    s->pci_irq_in = 0ULL;
+
+    if (s->nr_resets++ == 0) {
+        /* Power on reset */
+        s->reset_control = POR;
+    }
+}
+
+static const MemoryRegionOps pci_config_ops = {
+    .read = apb_pci_config_read,
+    .write = apb_pci_config_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void pci_pbm_realize(DeviceState *dev, Error **errp)
+{
+    APBState *s = APB_DEVICE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
     PCIDevice *pci_dev;
+    IOMMUState *is;
 
-    /* Ultrasparc PBM main bus */
-    dev = qdev_create(NULL, TYPE_APB);
-    d = APB_DEVICE(dev);
-    phb = PCI_HOST_BRIDGE(dev);
-    phb->bus = pci_register_bus(DEVICE(phb), "pci",
-                                pci_apb_set_irq, pci_apb_map_irq, d,
-                                &d->pci_mmio,
-                                &d->pci_ioport,
-                                0, 32, TYPE_PCI_BUS);
-    qdev_init_nofail(dev);
-    s = SYS_BUS_DEVICE(dev);
     /* apb_config */
-    sysbus_mmio_map(s, 0, special_base);
+    sysbus_mmio_map(sbd, 0, s->special_base);
     /* PCI configuration space */
-    sysbus_mmio_map(s, 1, special_base + 0x1000000ULL);
+    sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
     /* pci_ioport */
-    sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
+    sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
 
-    memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
-    memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
+    memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
+    memory_region_add_subregion(get_system_memory(), s->mem_base,
+                                &s->pci_mmio);
 
     pci_create_simple(phb->bus, 0, "pbm-pci");
 
     /* APB IOMMU */
-    is = &d->iommu;
+    is = &s->iommu;
     memset(is, 0, sizeof(IOMMUState));
 
     memory_region_init_iommu(&is->iommu, sizeof(is->iommu),
@@ -657,52 +672,30 @@ APBState *pci_apb_init(hwaddr special_base,
     /* APB secondary busses */
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
                                    TYPE_PBM_PCI_BRIDGE);
-    d->bridgeB = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
+    s->bridgeB = PCI_BRIDGE(pci_dev);
+    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbm_map_irq);
     qdev_init_nofail(&pci_dev->qdev);
 
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
                                    TYPE_PBM_PCI_BRIDGE);
-    d->bridgeA = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
+    s->bridgeA = PCI_BRIDGE(pci_dev);
+    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbm_map_irq);
     qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
     qdev_init_nofail(&pci_dev->qdev);
-
-    return d;
 }
 
-static void pci_pbm_reset(DeviceState *d)
+static void pci_pbm_init(Object *obj)
 {
+    APBState *s = APB_DEVICE(obj);
+    PCIHostState *phb = PCI_HOST_BRIDGE(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     unsigned int i;
-    APBState *s = APB_DEVICE(d);
-
-    for (i = 0; i < 8; i++) {
-        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
-    }
-    for (i = 0; i < 32; i++) {
-        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
-    }
-
-    s->irq_request = NO_IRQ_REQUEST;
-    s->pci_irq_in = 0ULL;
-
-    if (s->nr_resets++ == 0) {
-        /* Power on reset */
-        s->reset_control = POR;
-    }
-}
 
-static const MemoryRegionOps pci_config_ops = {
-    .read = apb_pci_config_read,
-    .write = apb_pci_config_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static int pci_pbm_init_device(DeviceState *dev)
-{
-    APBState *s = APB_DEVICE(dev);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
-    unsigned int i;
+    phb->bus = pci_register_bus(DEVICE(phb), "pci",
+                                pci_apb_set_irq, pci_apb_map_irq, s,
+                                &s->pci_mmio,
+                                &s->pci_ioport,
+                                0, 32, TYPE_PCI_BUS);
 
     for (i = 0; i < 8; i++) {
         s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
@@ -734,8 +727,6 @@ static int pci_pbm_init_device(DeviceState *dev)
 
     /* at region 2 */
     sysbus_init_mmio(sbd, &s->pci_ioport);
-
-    return 0;
 }
 
 static void pbm_pci_host_realize(PCIDevice *d, Error **errp)
@@ -774,12 +765,19 @@ static const TypeInfo pbm_pci_host_info = {
     },
 };
 
+static Property pbm_pci_host_properties[] = {
+    DEFINE_PROP_UINT64("special-base", APBState, special_base, 0),
+    DEFINE_PROP_UINT64("mem-base", APBState, mem_base, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pbm_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->init = pci_pbm_init_device;
+    dc->realize = pci_pbm_realize;
     dc->reset = pci_pbm_reset;
+    dc->props = pbm_pci_host_properties;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
@@ -787,6 +785,7 @@ static const TypeInfo pbm_host_info = {
     .name          = TYPE_APB,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(APBState),
+    .instance_init = pci_pbm_init,
     .class_init    = pbm_host_class_init,
 };
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 47952be..0a30fb8 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -502,7 +502,11 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     prom_init(hwdef->prom_addr, bios_name);
 
-    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
+    /* Init APB (PCI host bridge) */
+    apb = APB_DEVICE(qdev_create(NULL, TYPE_APB));
+    qdev_prop_set_uint64(DEVICE(apb), "special-base", APB_SPECIAL_BASE);
+    qdev_prop_set_uint64(DEVICE(apb), "mem-base", APB_MEM_BASE);
+    qdev_init_nofail(DEVICE(apb));
 
     /* Wire up PCI interrupts to CPU */
     for (i = 0; i < IVEC_MAX; i++) {
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index ae15d8c..f0074f7 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -62,6 +62,8 @@ typedef struct IOMMUState {
 typedef struct APBState {
     PCIHostState parent_obj;
 
+    hwaddr special_base;
+    hwaddr mem_base;
     MemoryRegion apb_config;
     MemoryRegion pci_config;
     MemoryRegion pci_mmio;
@@ -93,6 +95,4 @@ typedef struct PBMPCIBridge {
 #define PBM_PCI_BRIDGE(obj) \
     OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
 
-APBState *pci_apb_init(hwaddr special_base,
-                       hwaddr mem_base);
 #endif
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 10/15] apb: remove pci_apb_init() and instantiate APB device using qdev

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> By making the special_base and mem_base values qdev properties, we can move
> the remaining parts of pci_apb_init() into the pbm init() and realize()
> functions.
>
> This finally allows us to instantiate the APB directly using standard qdev
> create/init functions in sun4u.c.

Nice!
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/pci-host/apb.c         |  123 ++++++++++++++++++++++-----------------------
>  hw/sparc64/sun4u.c        |    6 ++-
>  include/hw/pci-host/apb.h |    4 +-
>  3 files changed, 68 insertions(+), 65 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 823661a..6c20285 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -611,41 +611,56 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>      pci_bridge_update_mappings(PCI_BRIDGE(br));
>  }
>
> -APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base)
> +static void pci_pbm_reset(DeviceState *d)
>  {
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -    PCIHostState *phb;
> -    APBState *d;
> -    IOMMUState *is;
> +    unsigned int i;
> +    APBState *s = APB_DEVICE(d);
> +
> +    for (i = 0; i < 8; i++) {
> +        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
> +    }
> +    for (i = 0; i < 32; i++) {
> +        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
> +    }
> +
> +    s->irq_request = NO_IRQ_REQUEST;
> +    s->pci_irq_in = 0ULL;
> +
> +    if (s->nr_resets++ == 0) {
> +        /* Power on reset */
> +        s->reset_control = POR;
> +    }
> +}
> +
> +static const MemoryRegionOps pci_config_ops = {
> +    .read = apb_pci_config_read,
> +    .write = apb_pci_config_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void pci_pbm_realize(DeviceState *dev, Error **errp)
> +{
> +    APBState *s = APB_DEVICE(dev);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>      PCIDevice *pci_dev;
> +    IOMMUState *is;
>
> -    /* Ultrasparc PBM main bus */
> -    dev = qdev_create(NULL, TYPE_APB);
> -    d = APB_DEVICE(dev);
> -    phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> -                                pci_apb_set_irq, pci_apb_map_irq, d,
> -                                &d->pci_mmio,
> -                                &d->pci_ioport,
> -                                0, 32, TYPE_PCI_BUS);
> -    qdev_init_nofail(dev);
> -    s = SYS_BUS_DEVICE(dev);
>      /* apb_config */
> -    sysbus_mmio_map(s, 0, special_base);
> +    sysbus_mmio_map(sbd, 0, s->special_base);
>      /* PCI configuration space */
> -    sysbus_mmio_map(s, 1, special_base + 0x1000000ULL);
> +    sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
>      /* pci_ioport */
> -    sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
> +    sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
>
> -    memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
> -    memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
> +    memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
> +    memory_region_add_subregion(get_system_memory(), s->mem_base,
> +                                &s->pci_mmio);
>
>      pci_create_simple(phb->bus, 0, "pbm-pci");
>
>      /* APB IOMMU */
> -    is = &d->iommu;
> +    is = &s->iommu;
>      memset(is, 0, sizeof(IOMMUState));
>
>      memory_region_init_iommu(&is->iommu, sizeof(is->iommu),
> @@ -657,52 +672,30 @@ APBState *pci_apb_init(hwaddr special_base,
>      /* APB secondary busses */
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    d->bridgeB = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
> +    s->bridgeB = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbm_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
>
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    d->bridgeA = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
> +    s->bridgeA = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbm_map_irq);
>      qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
>      qdev_init_nofail(&pci_dev->qdev);
> -
> -    return d;
>  }
>
> -static void pci_pbm_reset(DeviceState *d)
> +static void pci_pbm_init(Object *obj)
>  {
> +    APBState *s = APB_DEVICE(obj);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      unsigned int i;
> -    APBState *s = APB_DEVICE(d);
> -
> -    for (i = 0; i < 8; i++) {
> -        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
> -    }
> -    for (i = 0; i < 32; i++) {
> -        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
> -    }
> -
> -    s->irq_request = NO_IRQ_REQUEST;
> -    s->pci_irq_in = 0ULL;
> -
> -    if (s->nr_resets++ == 0) {
> -        /* Power on reset */
> -        s->reset_control = POR;
> -    }
> -}
>
> -static const MemoryRegionOps pci_config_ops = {
> -    .read = apb_pci_config_read,
> -    .write = apb_pci_config_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -static int pci_pbm_init_device(DeviceState *dev)
> -{
> -    APBState *s = APB_DEVICE(dev);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
> -    unsigned int i;
> +    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +                                pci_apb_set_irq, pci_apb_map_irq, s,
> +                                &s->pci_mmio,
> +                                &s->pci_ioport,
> +                                0, 32, TYPE_PCI_BUS);
>
>      for (i = 0; i < 8; i++) {
>          s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
> @@ -734,8 +727,6 @@ static int pci_pbm_init_device(DeviceState *dev)
>
>      /* at region 2 */
>      sysbus_init_mmio(sbd, &s->pci_ioport);
> -
> -    return 0;
>  }
>
>  static void pbm_pci_host_realize(PCIDevice *d, Error **errp)
> @@ -774,12 +765,19 @@ static const TypeInfo pbm_pci_host_info = {
>      },
>  };
>
> +static Property pbm_pci_host_properties[] = {
> +    DEFINE_PROP_UINT64("special-base", APBState, special_base, 0),
> +    DEFINE_PROP_UINT64("mem-base", APBState, mem_base, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pbm_host_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
> -    dc->init = pci_pbm_init_device;
> +    dc->realize = pci_pbm_realize;
>      dc->reset = pci_pbm_reset;
> +    dc->props = pbm_pci_host_properties;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>
> @@ -787,6 +785,7 @@ static const TypeInfo pbm_host_info = {
>      .name          = TYPE_APB,
>      .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(APBState),
> +    .instance_init = pci_pbm_init,
>      .class_init    = pbm_host_class_init,
>  };
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 47952be..0a30fb8 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -502,7 +502,11 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>
>      prom_init(hwdef->prom_addr, bios_name);
>
> -    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
> +    /* Init APB (PCI host bridge) */
> +    apb = APB_DEVICE(qdev_create(NULL, TYPE_APB));
> +    qdev_prop_set_uint64(DEVICE(apb), "special-base", APB_SPECIAL_BASE);
> +    qdev_prop_set_uint64(DEVICE(apb), "mem-base", APB_MEM_BASE);
> +    qdev_init_nofail(DEVICE(apb));
>
>      /* Wire up PCI interrupts to CPU */
>      for (i = 0; i < IVEC_MAX; i++) {
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index ae15d8c..f0074f7 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -62,6 +62,8 @@ typedef struct IOMMUState {
>  typedef struct APBState {
>      PCIHostState parent_obj;
>
> +    hwaddr special_base;
> +    hwaddr mem_base;
>      MemoryRegion apb_config;
>      MemoryRegion pci_config;
>      MemoryRegion pci_mmio;
> @@ -93,6 +95,4 @@ typedef struct PBMPCIBridge {
>  #define PBM_PCI_BRIDGE(obj) \
>      OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>
> -APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base);
>  #endif
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 10/15] apb: remove pci_apb_init() and instantiate APB device using qdev

[Philippe Mathieu-Daudé]

Hi Mark,

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> By making the special_base and mem_base values qdev properties, we can move
> the remaining parts of pci_apb_init() into the pbm init() and realize()
> functions.
> 
> This finally allows us to instantiate the APB directly using standard qdev
> create/init functions in sun4u.c.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |  123 ++++++++++++++++++++++-----------------------
>  hw/sparc64/sun4u.c        |    6 ++-
>  include/hw/pci-host/apb.h |    4 +-
>  3 files changed, 68 insertions(+), 65 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 823661a..6c20285 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -611,41 +611,56 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>      pci_bridge_update_mappings(PCI_BRIDGE(br));
>  }
>  
> -APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base)
> +static void pci_pbm_reset(DeviceState *d)
>  {
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -    PCIHostState *phb;
> -    APBState *d;
> -    IOMMUState *is;
> +    unsigned int i;
> +    APBState *s = APB_DEVICE(d);
> +
> +    for (i = 0; i < 8; i++) {
> +        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
> +    }
> +    for (i = 0; i < 32; i++) {
> +        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
> +    }
> +
> +    s->irq_request = NO_IRQ_REQUEST;
> +    s->pci_irq_in = 0ULL;
> +
> +    if (s->nr_resets++ == 0) {
> +        /* Power on reset */
> +        s->reset_control = POR;
> +    }
> +}
> +
> +static const MemoryRegionOps pci_config_ops = {
> +    .read = apb_pci_config_read,
> +    .write = apb_pci_config_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void pci_pbm_realize(DeviceState *dev, Error **errp)
> +{
> +    APBState *s = APB_DEVICE(dev);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>      PCIDevice *pci_dev;
> +    IOMMUState *is;
>  
> -    /* Ultrasparc PBM main bus */
> -    dev = qdev_create(NULL, TYPE_APB);
> -    d = APB_DEVICE(dev);
> -    phb = PCI_HOST_BRIDGE(dev);
> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> -                                pci_apb_set_irq, pci_apb_map_irq, d,
> -                                &d->pci_mmio,
> -                                &d->pci_ioport,
> -                                0, 32, TYPE_PCI_BUS);
> -    qdev_init_nofail(dev);
> -    s = SYS_BUS_DEVICE(dev);
>      /* apb_config */
> -    sysbus_mmio_map(s, 0, special_base);
> +    sysbus_mmio_map(sbd, 0, s->special_base);

You might add a safety check than special_base is correctly initialize
(compare to -1?)

>      /* PCI configuration space */
> -    sysbus_mmio_map(s, 1, special_base + 0x1000000ULL);
> +    sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
>      /* pci_ioport */
> -    sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
> +    sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
>  
> -    memory_region_init(&d->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
> -    memory_region_add_subregion(get_system_memory(), mem_base, &d->pci_mmio);
> +    memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
> +    memory_region_add_subregion(get_system_memory(), s->mem_base,

Ditto with mem_base

> +                                &s->pci_mmio);
>  
>      pci_create_simple(phb->bus, 0, "pbm-pci");
>  
>      /* APB IOMMU */
> -    is = &d->iommu;
> +    is = &s->iommu;
>      memset(is, 0, sizeof(IOMMUState));
>  
>      memory_region_init_iommu(&is->iommu, sizeof(is->iommu),
> @@ -657,52 +672,30 @@ APBState *pci_apb_init(hwaddr special_base,
>      /* APB secondary busses */
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    d->bridgeB = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(d->bridgeB, "pciB", pci_pbm_map_irq);
> +    s->bridgeB = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbm_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
>  
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     TYPE_PBM_PCI_BRIDGE);
> -    d->bridgeA = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(d->bridgeA, "pciA", pci_pbm_map_irq);
> +    s->bridgeA = PCI_BRIDGE(pci_dev);
> +    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbm_map_irq);
>      qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
>      qdev_init_nofail(&pci_dev->qdev);
> -
> -    return d;
>  }
>  
> -static void pci_pbm_reset(DeviceState *d)
> +static void pci_pbm_init(Object *obj)
>  {
> +    APBState *s = APB_DEVICE(obj);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      unsigned int i;
> -    APBState *s = APB_DEVICE(d);
> -
> -    for (i = 0; i < 8; i++) {
> -        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
> -    }
> -    for (i = 0; i < 32; i++) {
> -        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
> -    }
> -
> -    s->irq_request = NO_IRQ_REQUEST;
> -    s->pci_irq_in = 0ULL;
> -
> -    if (s->nr_resets++ == 0) {
> -        /* Power on reset */
> -        s->reset_control = POR;
> -    }
> -}
>  
> -static const MemoryRegionOps pci_config_ops = {
> -    .read = apb_pci_config_read,
> -    .write = apb_pci_config_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
> -static int pci_pbm_init_device(DeviceState *dev)
> -{
> -    APBState *s = APB_DEVICE(dev);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
> -    unsigned int i;
> +    phb->bus = pci_register_bus(DEVICE(phb), "pci",
> +                                pci_apb_set_irq, pci_apb_map_irq, s,
> +                                &s->pci_mmio,
> +                                &s->pci_ioport,
> +                                0, 32, TYPE_PCI_BUS);
>  
>      for (i = 0; i < 8; i++) {
>          s->pci_irq_map[i] = (0x1f << 6) | (i << 2);
> @@ -734,8 +727,6 @@ static int pci_pbm_init_device(DeviceState *dev)
>  
>      /* at region 2 */
>      sysbus_init_mmio(sbd, &s->pci_ioport);
> -
> -    return 0;
>  }
>  
>  static void pbm_pci_host_realize(PCIDevice *d, Error **errp)
> @@ -774,12 +765,19 @@ static const TypeInfo pbm_pci_host_info = {
>      },
>  };
>  
> +static Property pbm_pci_host_properties[] = {
> +    DEFINE_PROP_UINT64("special-base", APBState, special_base, 0),

UINT64_MAX for explicitly set correct base?

> +    DEFINE_PROP_UINT64("mem-base", APBState, mem_base, 0),

Ditto

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pbm_host_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->init = pci_pbm_init_device;
> +    dc->realize = pci_pbm_realize;
>      dc->reset = pci_pbm_reset;
> +    dc->props = pbm_pci_host_properties;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
> @@ -787,6 +785,7 @@ static const TypeInfo pbm_host_info = {
>      .name          = TYPE_APB,
>      .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(APBState),
> +    .instance_init = pci_pbm_init,
>      .class_init    = pbm_host_class_init,
>  };
>  
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 47952be..0a30fb8 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -502,7 +502,11 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>  
>      prom_init(hwdef->prom_addr, bios_name);
>  
> -    apb = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE);
> +    /* Init APB (PCI host bridge) */
> +    apb = APB_DEVICE(qdev_create(NULL, TYPE_APB));
> +    qdev_prop_set_uint64(DEVICE(apb), "special-base", APB_SPECIAL_BASE);
> +    qdev_prop_set_uint64(DEVICE(apb), "mem-base", APB_MEM_BASE);
> +    qdev_init_nofail(DEVICE(apb));

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  
>      /* Wire up PCI interrupts to CPU */
>      for (i = 0; i < IVEC_MAX; i++) {
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index ae15d8c..f0074f7 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -62,6 +62,8 @@ typedef struct IOMMUState {
>  typedef struct APBState {
>      PCIHostState parent_obj;
>  
> +    hwaddr special_base;
> +    hwaddr mem_base;
>      MemoryRegion apb_config;
>      MemoryRegion pci_config;
>      MemoryRegion pci_mmio;
> @@ -93,6 +95,4 @@ typedef struct PBMPCIBridge {
>  #define PBM_PCI_BRIDGE(obj) \
>      OBJECT_CHECK(PBMPCIBridge, (obj), TYPE_PBM_PCI_BRIDGE)
>  
> -APBState *pci_apb_init(hwaddr special_base,
> -                       hwaddr mem_base);
>  #endif
> 

Re: [Qemu-devel] [PATCH 10/15] apb: remove pci_apb_init() and instantiate APB device using qdev

[Mark Cave-Ayland]

On 20/11/17 17:51, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
>> By making the special_base and mem_base values qdev properties, we can move
>> the remaining parts of pci_apb_init() into the pbm init() and realize()
>> functions.
>>
>> This finally allows us to instantiate the APB directly using standard qdev
>> create/init functions in sun4u.c.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/pci-host/apb.c         |  123 ++++++++++++++++++++++-----------------------
>>  hw/sparc64/sun4u.c        |    6 ++-
>>  include/hw/pci-host/apb.h |    4 +-
>>  3 files changed, 68 insertions(+), 65 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 823661a..6c20285 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -611,41 +611,56 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>>      pci_bridge_update_mappings(PCI_BRIDGE(br));
>>  }
>>  
>> -APBState *pci_apb_init(hwaddr special_base,
>> -                       hwaddr mem_base)
>> +static void pci_pbm_reset(DeviceState *d)
>>  {
>> -    DeviceState *dev;
>> -    SysBusDevice *s;
>> -    PCIHostState *phb;
>> -    APBState *d;
>> -    IOMMUState *is;
>> +    unsigned int i;
>> +    APBState *s = APB_DEVICE(d);
>> +
>> +    for (i = 0; i < 8; i++) {
>> +        s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
>> +    }
>> +    for (i = 0; i < 32; i++) {
>> +        s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
>> +    }
>> +
>> +    s->irq_request = NO_IRQ_REQUEST;
>> +    s->pci_irq_in = 0ULL;
>> +
>> +    if (s->nr_resets++ == 0) {
>> +        /* Power on reset */
>> +        s->reset_control = POR;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps pci_config_ops = {
>> +    .read = apb_pci_config_read,
>> +    .write = apb_pci_config_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void pci_pbm_realize(DeviceState *dev, Error **errp)
>> +{
>> +    APBState *s = APB_DEVICE(dev);
>> +    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>>      PCIDevice *pci_dev;
>> +    IOMMUState *is;
>>  
>> -    /* Ultrasparc PBM main bus */
>> -    dev = qdev_create(NULL, TYPE_APB);
>> -    d = APB_DEVICE(dev);
>> -    phb = PCI_HOST_BRIDGE(dev);
>> -    phb->bus = pci_register_bus(DEVICE(phb), "pci",
>> -                                pci_apb_set_irq, pci_apb_map_irq, d,
>> -                                &d->pci_mmio,
>> -                                &d->pci_ioport,
>> -                                0, 32, TYPE_PCI_BUS);
>> -    qdev_init_nofail(dev);
>> -    s = SYS_BUS_DEVICE(dev);
>>      /* apb_config */
>> -    sysbus_mmio_map(s, 0, special_base);
>> +    sysbus_mmio_map(sbd, 0, s->special_base);
> 
> You might add a safety check than special_base is correctly initialize
> (compare to -1?)

I guess strictly speaking this would be good to have, however since the
same value is also hard-coded in OpenBIOS then you'll get a crash if you
change it to any other value anyhow. So for that reason I think it's
okay for the moment as it is.


ATB,

Mark.

[Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Mark Cave-Ayland]

After the previous refactoring it is now possible to use separate functions
to improve clarity of the interrupt paths. Similarly by checking the PCI
devnfn to identify busA during apb_pci_bridge_realize() it becomes possible
to completely remove the busA property from the PBMPCIBridge state.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |   54 ++++++++++++++++++---------------------------
 include/hw/pci-host/apb.h |    3 ---
 2 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 6c20285..268100e 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
     return irq_num;
 }
 
-static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
+static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
-                           PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
-
-    int bus_offset;
-    if (br->busA) {
-        bus_offset = 0x0;
+    /* The on-board devices have fixed (legacy) OBIO intnos */
+    switch (PCI_SLOT(pci_dev->devfn)) {
+    case 1:
+        /* Onboard NIC */
+        return 0x21;
+    case 3:
+        /* Onboard IDE */
+        return 0x20;
+    default:
+        /* Normal intno, fall through */
+        break;
+    }
 
-        /* The on-board devices have fixed (legacy) OBIO intnos */
-        switch (PCI_SLOT(pci_dev->devfn)) {
-        case 1:
-            /* Onboard NIC */
-            return 0x21;
-        case 3:
-            /* Onboard IDE */
-            return 0x20;
+    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
+}
 
-        default:
-            /* Normal intno, fall through */
-            break;
-        }
-    } else {
-        bus_offset = 0x10;
-    }
-    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
+static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
 }
 
 static void pci_apb_set_irq(void *opaque, int irq_num, int level)
@@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
 
     /* If initialising busA, ensure that we allow IO transactions so that
        we get the early serial console until OpenBIOS configures the bridge */
-    if (br->busA) {
+    if (dev->devfn == PCI_DEVFN(1, 1)) {
         cmd |= PCI_COMMAND_IO;
     }
 
@@ -673,14 +668,13 @@ static void pci_pbm_realize(DeviceState *dev, Error **errp)
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
                                    TYPE_PBM_PCI_BRIDGE);
     s->bridgeB = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbm_map_irq);
+    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbmB_map_irq);
     qdev_init_nofail(&pci_dev->qdev);
 
     pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
                                    TYPE_PBM_PCI_BRIDGE);
     s->bridgeA = PCI_BRIDGE(pci_dev);
-    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbm_map_irq);
-    qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
+    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbmA_map_irq);
     qdev_init_nofail(&pci_dev->qdev);
 }
 
@@ -789,11 +783,6 @@ static const TypeInfo pbm_host_info = {
     .class_init    = pbm_host_class_init,
 };
 
-static Property pbm_pci_properties[] = {
-    DEFINE_PROP_BOOL("busA", PBMPCIBridge, busA, false),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void pbm_pci_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -809,7 +798,6 @@ static void pbm_pci_bridge_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->reset = pci_bridge_reset;
     dc->vmsd = &vmstate_pci_device;
-    dc->props = pbm_pci_properties;
 }
 
 static const TypeInfo pbm_pci_bridge_info = {
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index f0074f7..dd49437 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -86,9 +86,6 @@ typedef struct APBState {
 typedef struct PBMPCIBridge {
     /*< private >*/
     PCIBridge parent_obj;
-
-    /* Is this busA with in-built devices (ebus)? */
-    bool busA;
 } PBMPCIBridge;
 
 #define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> After the previous refactoring it is now possible to use separate functions
> to improve clarity of the interrupt paths. Similarly by checking the PCI
> devnfn to identify busA during apb_pci_bridge_realize() it becomes possible
> to completely remove the busA property from the PBMPCIBridge state.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |   54 ++++++++++++++++++---------------------------
>  include/hw/pci-host/apb.h |    3 ---
>  2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 6c20285..268100e 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
>      return irq_num;
>  }
>
> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
> -                           PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
> -
> -    int bus_offset;
> -    if (br->busA) {
> -        bus_offset = 0x0;
> +    /* The on-board devices have fixed (legacy) OBIO intnos */
> +    switch (PCI_SLOT(pci_dev->devfn)) {
> +    case 1:
> +        /* Onboard NIC */
> +        return 0x21;
> +    case 3:
> +        /* Onboard IDE */
> +        return 0x20;
> +    default:
> +        /* Normal intno, fall through */
> +        break;
> +    }
>
> -        /* The on-board devices have fixed (legacy) OBIO intnos */
> -        switch (PCI_SLOT(pci_dev->devfn)) {
> -        case 1:
> -            /* Onboard NIC */
> -            return 0x21;
> -        case 3:
> -            /* Onboard IDE */
> -            return 0x20;
> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
> +}
>
> -        default:
> -            /* Normal intno, fall through */
> -            break;
> -        }
> -    } else {
> -        bus_offset = 0x10;
> -    }
> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
> +{
> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>  }
>
>  static void pci_apb_set_irq(void *opaque, int irq_num, int level)
> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>
>      /* If initialising busA, ensure that we allow IO transactions so that
>         we get the early serial console until OpenBIOS configures the bridge */
> -    if (br->busA) {
> +    if (dev->devfn == PCI_DEVFN(1, 1)) {

I think the previous syntax was more explicit here. A comment would be nice.


>          cmd |= PCI_COMMAND_IO;
>      }
>
> @@ -673,14 +668,13 @@ static void pci_pbm_realize(DeviceState *dev, Error **errp)
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 0), true,
>                                     TYPE_PBM_PCI_BRIDGE);
>      s->bridgeB = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbm_map_irq);
> +    pci_bridge_map_irq(s->bridgeB, "pciB", pci_pbmB_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
>
>      pci_dev = pci_create_multifunction(phb->bus, PCI_DEVFN(1, 1), true,
>                                     TYPE_PBM_PCI_BRIDGE);
>      s->bridgeA = PCI_BRIDGE(pci_dev);
> -    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbm_map_irq);
> -    qdev_prop_set_bit(DEVICE(pci_dev), "busA", true);
> +    pci_bridge_map_irq(s->bridgeA, "pciA", pci_pbmA_map_irq);
>      qdev_init_nofail(&pci_dev->qdev);
>  }
>
> @@ -789,11 +783,6 @@ static const TypeInfo pbm_host_info = {
>      .class_init    = pbm_host_class_init,
>  };
>
> -static Property pbm_pci_properties[] = {
> -    DEFINE_PROP_BOOL("busA", PBMPCIBridge, busA, false),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void pbm_pci_bridge_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -809,7 +798,6 @@ static void pbm_pci_bridge_class_init(ObjectClass *klass, void *data)
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>      dc->reset = pci_bridge_reset;
>      dc->vmsd = &vmstate_pci_device;
> -    dc->props = pbm_pci_properties;
>  }
>
>  static const TypeInfo pbm_pci_bridge_info = {
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index f0074f7..dd49437 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -86,9 +86,6 @@ typedef struct APBState {
>  typedef struct PBMPCIBridge {
>      /*< private >*/
>      PCIBridge parent_obj;
> -
> -    /* Is this busA with in-built devices (ebus)? */
> -    bool busA;
>  } PBMPCIBridge;
>
>  #define TYPE_PBM_PCI_BRIDGE "pbm-bridge"
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Mark Cave-Ayland]

On 17/11/17 14:33, Artyom Tarasenko wrote:

> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> After the previous refactoring it is now possible to use separate functions
>> to improve clarity of the interrupt paths. Similarly by checking the PCI
>> devnfn to identify busA during apb_pci_bridge_realize() it becomes possible
>> to completely remove the busA property from the PBMPCIBridge state.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/pci-host/apb.c         |   54 ++++++++++++++++++---------------------------
>>  include/hw/pci-host/apb.h |    3 ---
>>  2 files changed, 21 insertions(+), 36 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 6c20285..268100e 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
>>      return irq_num;
>>  }
>>
>> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
>> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>>  {
>> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
>> -                           PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
>> -
>> -    int bus_offset;
>> -    if (br->busA) {
>> -        bus_offset = 0x0;
>> +    /* The on-board devices have fixed (legacy) OBIO intnos */
>> +    switch (PCI_SLOT(pci_dev->devfn)) {
>> +    case 1:
>> +        /* Onboard NIC */
>> +        return 0x21;
>> +    case 3:
>> +        /* Onboard IDE */
>> +        return 0x20;
>> +    default:
>> +        /* Normal intno, fall through */
>> +        break;
>> +    }
>>
>> -        /* The on-board devices have fixed (legacy) OBIO intnos */
>> -        switch (PCI_SLOT(pci_dev->devfn)) {
>> -        case 1:
>> -            /* Onboard NIC */
>> -            return 0x21;
>> -        case 3:
>> -            /* Onboard IDE */
>> -            return 0x20;
>> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>> +}
>>
>> -        default:
>> -            /* Normal intno, fall through */
>> -            break;
>> -        }
>> -    } else {
>> -        bus_offset = 0x10;
>> -    }
>> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
>> +{
>> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>  }
>>
>>  static void pci_apb_set_irq(void *opaque, int irq_num, int level)
>> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>>
>>      /* If initialising busA, ensure that we allow IO transactions so that
>>         we get the early serial console until OpenBIOS configures the bridge */
>> -    if (br->busA) {
>> +    if (dev->devfn == PCI_DEVFN(1, 1)) {
> 
> I think the previous syntax was more explicit here. A comment would be nice.

Yes it's definitely something that isn't immediately obvious, which is
why I left the above comment in place explaining what the if() branch is
doing. Is there something in the comment that isn't particularly clear?

Note one of the reasons for wanting to remove the busA property is that
where possible I'd like to reduce the code in the IRQ path, and while
the existing code works I am still unsure of the additional overhead of
the 2 levels of QOM type checking that the current approach requires for
each IRQ.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Mark Cave-Ayland]

On 19/11/17 11:06, Mark Cave-Ayland wrote:

> On 17/11/17 14:33, Artyom Tarasenko wrote:
> 
>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> After the previous refactoring it is now possible to use separate functions
>>> to improve clarity of the interrupt paths. Similarly by checking the PCI
>>> devnfn to identify busA during apb_pci_bridge_realize() it becomes possible
>>> to completely remove the busA property from the PBMPCIBridge state.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>   hw/pci-host/apb.c         |   54 ++++++++++++++++++---------------------------
>>>   include/hw/pci-host/apb.h |    3 ---
>>>   2 files changed, 21 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>>> index 6c20285..268100e 100644
>>> --- a/hw/pci-host/apb.c
>>> +++ b/hw/pci-host/apb.c
>>> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
>>>       return irq_num;
>>>   }
>>>
>>> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
>>> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>>>   {
>>> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
>>> -                           PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
>>> -
>>> -    int bus_offset;
>>> -    if (br->busA) {
>>> -        bus_offset = 0x0;
>>> +    /* The on-board devices have fixed (legacy) OBIO intnos */
>>> +    switch (PCI_SLOT(pci_dev->devfn)) {
>>> +    case 1:
>>> +        /* Onboard NIC */
>>> +        return 0x21;
>>> +    case 3:
>>> +        /* Onboard IDE */
>>> +        return 0x20;
>>> +    default:
>>> +        /* Normal intno, fall through */
>>> +        break;
>>> +    }
>>>
>>> -        /* The on-board devices have fixed (legacy) OBIO intnos */
>>> -        switch (PCI_SLOT(pci_dev->devfn)) {
>>> -        case 1:
>>> -            /* Onboard NIC */
>>> -            return 0x21;
>>> -        case 3:
>>> -            /* Onboard IDE */
>>> -            return 0x20;
>>> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>> +}
>>>
>>> -        default:
>>> -            /* Normal intno, fall through */
>>> -            break;
>>> -        }
>>> -    } else {
>>> -        bus_offset = 0x10;
>>> -    }
>>> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
>>> +{
>>> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>   }
>>>
>>>   static void pci_apb_set_irq(void *opaque, int irq_num, int level)
>>> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error **errp)
>>>
>>>       /* If initialising busA, ensure that we allow IO transactions so that
>>>          we get the early serial console until OpenBIOS configures the bridge */
>>> -    if (br->busA) {
>>> +    if (dev->devfn == PCI_DEVFN(1, 1)) {
>>
>> I think the previous syntax was more explicit here. A comment would be nice.
> 
> Yes it's definitely something that isn't immediately obvious, which is
> why I left the above comment in place explaining what the if() branch is
> doing. Is there something in the comment that isn't particularly clear?
> 
> Note one of the reasons for wanting to remove the busA property is that
> where possible I'd like to reduce the code in the IRQ path, and while
> the existing code works I am still unsure of the additional overhead of
> the 2 levels of QOM type checking that the current approach requires for
> each IRQ.

Hi Artyom,

Thinking about this a bit more during freeze, this is actually doing the 
opposite of what we want, as it requires the device realise function to 
behave differently depending upon how it is related to the PCI bus.

How about swapping this out for a qdev bool property for APB named 
"enable-early-pci-io-access", setting it just for the PCI_DEVFN(1, 1) 
device containing the ebus and then alter the if() statement above to 
enable PCI IO access if the qdev property is set?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Artyom Tarasenko]

On Sun, Dec 17, 2017 at 12:09 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 19/11/17 11:06, Mark Cave-Ayland wrote:
>
>> On 17/11/17 14:33, Artyom Tarasenko wrote:
>>
>>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>>
>>>> After the previous refactoring it is now possible to use separate
>>>> functions
>>>> to improve clarity of the interrupt paths. Similarly by checking the PCI
>>>> devnfn to identify busA during apb_pci_bridge_realize() it becomes
>>>> possible
>>>> to completely remove the busA property from the PBMPCIBridge state.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> ---
>>>>   hw/pci-host/apb.c         |   54
>>>> ++++++++++++++++++---------------------------
>>>>   include/hw/pci-host/apb.h |    3 ---
>>>>   2 files changed, 21 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>>>> index 6c20285..268100e 100644
>>>> --- a/hw/pci-host/apb.c
>>>> +++ b/hw/pci-host/apb.c
>>>> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int
>>>> irq_num)
>>>>       return irq_num;
>>>>   }
>>>>
>>>> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
>>>> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>   {
>>>> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
>>>> -
>>>> PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
>>>> -
>>>> -    int bus_offset;
>>>> -    if (br->busA) {
>>>> -        bus_offset = 0x0;
>>>> +    /* The on-board devices have fixed (legacy) OBIO intnos */
>>>> +    switch (PCI_SLOT(pci_dev->devfn)) {
>>>> +    case 1:
>>>> +        /* Onboard NIC */
>>>> +        return 0x21;
>>>> +    case 3:
>>>> +        /* Onboard IDE */
>>>> +        return 0x20;
>>>> +    default:
>>>> +        /* Normal intno, fall through */
>>>> +        break;
>>>> +    }
>>>>
>>>> -        /* The on-board devices have fixed (legacy) OBIO intnos */
>>>> -        switch (PCI_SLOT(pci_dev->devfn)) {
>>>> -        case 1:
>>>> -            /* Onboard NIC */
>>>> -            return 0x21;
>>>> -        case 3:
>>>> -            /* Onboard IDE */
>>>> -            return 0x20;
>>>> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>> +}
>>>>
>>>> -        default:
>>>> -            /* Normal intno, fall through */
>>>> -            break;
>>>> -        }
>>>> -    } else {
>>>> -        bus_offset = 0x10;
>>>> -    }
>>>> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) &
>>>> 0x1f;
>>>> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
>>>> +{
>>>> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>>   }
>>>>
>>>>   static void pci_apb_set_irq(void *opaque, int irq_num, int level)
>>>> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev,
>>>> Error **errp)
>>>>
>>>>       /* If initialising busA, ensure that we allow IO transactions so
>>>> that
>>>>          we get the early serial console until OpenBIOS configures the
>>>> bridge */
>>>> -    if (br->busA) {
>>>> +    if (dev->devfn == PCI_DEVFN(1, 1)) {
>>>
>>>
>>> I think the previous syntax was more explicit here. A comment would be
>>> nice.
>>
>>
>> Yes it's definitely something that isn't immediately obvious, which is
>> why I left the above comment in place explaining what the if() branch is
>> doing. Is there something in the comment that isn't particularly clear?
>>
>> Note one of the reasons for wanting to remove the busA property is that
>> where possible I'd like to reduce the code in the IRQ path, and while
>> the existing code works I am still unsure of the additional overhead of
>> the 2 levels of QOM type checking that the current approach requires for
>> each IRQ.
>
>
> Hi Artyom,
>
> Thinking about this a bit more during freeze, this is actually doing the
> opposite of what we want, as it requires the device realise function to
> behave differently depending upon how it is related to the PCI bus.
>
> How about swapping this out for a qdev bool property for APB named
> "enable-early-pci-io-access", setting it just for the PCI_DEVFN(1, 1) device
> containing the ebus and then alter the if() statement above to enable PCI IO
> access if the qdev property is set?

This does sound reasonable. I wonder if this has to be a qdev property though.
Doesn't the physical bridge have a software visible bit/register for it?


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Mark Cave-Ayland]

On 19/12/17 07:56, Artyom Tarasenko wrote:
> On Sun, Dec 17, 2017 at 12:09 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> On 19/11/17 11:06, Mark Cave-Ayland wrote:
>>
>>> On 17/11/17 14:33, Artyom Tarasenko wrote:
>>>
>>>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>>>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>>>
>>>>> After the previous refactoring it is now possible to use separate
>>>>> functions
>>>>> to improve clarity of the interrupt paths. Similarly by checking the PCI
>>>>> devnfn to identify busA during apb_pci_bridge_realize() it becomes
>>>>> possible
>>>>> to completely remove the busA property from the PBMPCIBridge state.
>>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> ---
>>>>>    hw/pci-host/apb.c         |   54
>>>>> ++++++++++++++++++---------------------------
>>>>>    include/hw/pci-host/apb.h |    3 ---
>>>>>    2 files changed, 21 insertions(+), 36 deletions(-)
>>>>>
>>>>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>>>>> index 6c20285..268100e 100644
>>>>> --- a/hw/pci-host/apb.c
>>>>> +++ b/hw/pci-host/apb.c
>>>>> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int
>>>>> irq_num)
>>>>>        return irq_num;
>>>>>    }
>>>>>
>>>>> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>>    {
>>>>> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
>>>>> -
>>>>> PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
>>>>> -
>>>>> -    int bus_offset;
>>>>> -    if (br->busA) {
>>>>> -        bus_offset = 0x0;
>>>>> +    /* The on-board devices have fixed (legacy) OBIO intnos */
>>>>> +    switch (PCI_SLOT(pci_dev->devfn)) {
>>>>> +    case 1:
>>>>> +        /* Onboard NIC */
>>>>> +        return 0x21;
>>>>> +    case 3:
>>>>> +        /* Onboard IDE */
>>>>> +        return 0x20;
>>>>> +    default:
>>>>> +        /* Normal intno, fall through */
>>>>> +        break;
>>>>> +    }
>>>>>
>>>>> -        /* The on-board devices have fixed (legacy) OBIO intnos */
>>>>> -        switch (PCI_SLOT(pci_dev->devfn)) {
>>>>> -        case 1:
>>>>> -            /* Onboard NIC */
>>>>> -            return 0x21;
>>>>> -        case 3:
>>>>> -            /* Onboard IDE */
>>>>> -            return 0x20;
>>>>> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>>> +}
>>>>>
>>>>> -        default:
>>>>> -            /* Normal intno, fall through */
>>>>> -            break;
>>>>> -        }
>>>>> -    } else {
>>>>> -        bus_offset = 0x10;
>>>>> -    }
>>>>> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) &
>>>>> 0x1f;
>>>>> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>> +{
>>>>> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>>>    }
>>>>>
>>>>>    static void pci_apb_set_irq(void *opaque, int irq_num, int level)
>>>>> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev,
>>>>> Error **errp)
>>>>>
>>>>>        /* If initialising busA, ensure that we allow IO transactions so
>>>>> that
>>>>>           we get the early serial console until OpenBIOS configures the
>>>>> bridge */
>>>>> -    if (br->busA) {
>>>>> +    if (dev->devfn == PCI_DEVFN(1, 1)) {
>>>>
>>>>
>>>> I think the previous syntax was more explicit here. A comment would be
>>>> nice.
>>>
>>>
>>> Yes it's definitely something that isn't immediately obvious, which is
>>> why I left the above comment in place explaining what the if() branch is
>>> doing. Is there something in the comment that isn't particularly clear?
>>>
>>> Note one of the reasons for wanting to remove the busA property is that
>>> where possible I'd like to reduce the code in the IRQ path, and while
>>> the existing code works I am still unsure of the additional overhead of
>>> the 2 levels of QOM type checking that the current approach requires for
>>> each IRQ.
>>
>>
>> Hi Artyom,
>>
>> Thinking about this a bit more during freeze, this is actually doing the
>> opposite of what we want, as it requires the device realise function to
>> behave differently depending upon how it is related to the PCI bus.
>>
>> How about swapping this out for a qdev bool property for APB named
>> "enable-early-pci-io-access", setting it just for the PCI_DEVFN(1, 1) device
>> containing the ebus and then alter the if() statement above to enable PCI IO
>> access if the qdev property is set?
> 
> This does sound reasonable. I wonder if this has to be a qdev property though.
> Doesn't the physical bridge have a software visible bit/register for it?

Yes, we could enable the PCI IO transaction bit for that particular 
bridge after realize if that is acceptable?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 11/15] apb: split pci_pbm_map_irq() into separate functions for bus A and bus B

[Artyom Tarasenko]

On Tue, Dec 19, 2017 at 10:27 AM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 19/12/17 07:56, Artyom Tarasenko wrote:
>>
>> On Sun, Dec 17, 2017 at 12:09 PM, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>
>>> On 19/11/17 11:06, Mark Cave-Ayland wrote:
>>>
>>>> On 17/11/17 14:33, Artyom Tarasenko wrote:
>>>>
>>>>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>>>>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>>>>
>>>>>>
>>>>>> After the previous refactoring it is now possible to use separate
>>>>>> functions
>>>>>> to improve clarity of the interrupt paths. Similarly by checking the
>>>>>> PCI
>>>>>> devnfn to identify busA during apb_pci_bridge_realize() it becomes
>>>>>> possible
>>>>>> to completely remove the busA property from the PBMPCIBridge state.
>>>>>>
>>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>>> ---
>>>>>>    hw/pci-host/apb.c         |   54
>>>>>> ++++++++++++++++++---------------------------
>>>>>>    include/hw/pci-host/apb.h |    3 ---
>>>>>>    2 files changed, 21 insertions(+), 36 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>>>>>> index 6c20285..268100e 100644
>>>>>> --- a/hw/pci-host/apb.c
>>>>>> +++ b/hw/pci-host/apb.c
>>>>>> @@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev,
>>>>>> int
>>>>>> irq_num)
>>>>>>        return irq_num;
>>>>>>    }
>>>>>>
>>>>>> -static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>>> +static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>>>    {
>>>>>> -    PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
>>>>>> -
>>>>>> PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
>>>>>> -
>>>>>> -    int bus_offset;
>>>>>> -    if (br->busA) {
>>>>>> -        bus_offset = 0x0;
>>>>>> +    /* The on-board devices have fixed (legacy) OBIO intnos */
>>>>>> +    switch (PCI_SLOT(pci_dev->devfn)) {
>>>>>> +    case 1:
>>>>>> +        /* Onboard NIC */
>>>>>> +        return 0x21;
>>>>>> +    case 3:
>>>>>> +        /* Onboard IDE */
>>>>>> +        return 0x20;
>>>>>> +    default:
>>>>>> +        /* Normal intno, fall through */
>>>>>> +        break;
>>>>>> +    }
>>>>>>
>>>>>> -        /* The on-board devices have fixed (legacy) OBIO intnos */
>>>>>> -        switch (PCI_SLOT(pci_dev->devfn)) {
>>>>>> -        case 1:
>>>>>> -            /* Onboard NIC */
>>>>>> -            return 0x21;
>>>>>> -        case 3:
>>>>>> -            /* Onboard IDE */
>>>>>> -            return 0x20;
>>>>>> +    return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>>>> +}
>>>>>>
>>>>>> -        default:
>>>>>> -            /* Normal intno, fall through */
>>>>>> -            break;
>>>>>> -        }
>>>>>> -    } else {
>>>>>> -        bus_offset = 0x10;
>>>>>> -    }
>>>>>> -    return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) &
>>>>>> 0x1f;
>>>>>> +static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
>>>>>> +{
>>>>>> +    return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
>>>>>>    }
>>>>>>
>>>>>>    static void pci_apb_set_irq(void *opaque, int irq_num, int level)
>>>>>> @@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev,
>>>>>> Error **errp)
>>>>>>
>>>>>>        /* If initialising busA, ensure that we allow IO transactions
>>>>>> so
>>>>>> that
>>>>>>           we get the early serial console until OpenBIOS configures
>>>>>> the
>>>>>> bridge */
>>>>>> -    if (br->busA) {
>>>>>> +    if (dev->devfn == PCI_DEVFN(1, 1)) {
>>>>>
>>>>>
>>>>>
>>>>> I think the previous syntax was more explicit here. A comment would be
>>>>> nice.
>>>>
>>>>
>>>>
>>>> Yes it's definitely something that isn't immediately obvious, which is
>>>> why I left the above comment in place explaining what the if() branch is
>>>> doing. Is there something in the comment that isn't particularly clear?
>>>>
>>>> Note one of the reasons for wanting to remove the busA property is that
>>>> where possible I'd like to reduce the code in the IRQ path, and while
>>>> the existing code works I am still unsure of the additional overhead of
>>>> the 2 levels of QOM type checking that the current approach requires for
>>>> each IRQ.
>>>
>>>
>>>
>>> Hi Artyom,
>>>
>>> Thinking about this a bit more during freeze, this is actually doing the
>>> opposite of what we want, as it requires the device realise function to
>>> behave differently depending upon how it is related to the PCI bus.
>>>
>>> How about swapping this out for a qdev bool property for APB named
>>> "enable-early-pci-io-access", setting it just for the PCI_DEVFN(1, 1)
>>> device
>>> containing the ebus and then alter the if() statement above to enable PCI
>>> IO
>>> access if the qdev property is set?
>>
>>
>> This does sound reasonable. I wonder if this has to be a qdev property
>> though.
>> Doesn't the physical bridge have a software visible bit/register for it?
>
>
> Yes, we could enable the PCI IO transaction bit for that particular bridge
> after realize if that is acceptable?

Yes, please. I think it would be pretty close to what the real hw does.

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

[Qemu-devel] [PATCH 12/15] ebus: wire up OBIO interrupts to APB pbm via qdev GPIOs

[Mark Cave-Ayland]

This enables us to remove the static array mapping in the ISA IRQ
handler (and the embedded reference to the APB device) by formalising
the interrupt wiring via the qdev GPIO API.

For more clarity we replace the APB OBIO interrupt numbers with constants
designating the interrupt source, and rename isa_irq_handler() to
ebus_isa_irq_handler().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |    2 +-
 hw/sparc64/sun4u.c        |   48 ++++++++++++++++++++++-----------------------
 include/hw/pci-host/apb.h |    8 +++++++-
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 268100e..f092780c 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -700,7 +700,7 @@ static void pci_pbm_init(Object *obj)
     for (i = 0; i < 32; i++) {
         s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
     }
-    s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
+    qdev_init_gpio_in_named(DEVICE(s), pci_apb_set_irq, "pbm-irq", MAX_IVEC);
     qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
     s->irq_request = NO_IRQ_REQUEST;
     s->pci_irq_in = 0ULL;
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 0a30fb8..da386d3 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -86,6 +86,7 @@ typedef struct EbusState {
     PCIDevice parent_obj;
 
     ISABus *isa_bus;
+    qemu_irq isa_bus_irqs[16];
     uint64_t console_serial_base;
     MemoryRegion bar0;
     MemoryRegion bar1;
@@ -211,23 +212,15 @@ typedef struct ResetData {
     uint64_t prom_addr;
 } ResetData;
 
-static void isa_irq_handler(void *opaque, int n, int level)
+static void ebus_isa_irq_handler(void *opaque, int n, int level)
 {
-    static const int isa_irq_to_ivec[16] = {
-        [1] = 0x29, /* keyboard */
-        [4] = 0x2b, /* serial */
-        [6] = 0x27, /* floppy */
-        [7] = 0x22, /* parallel */
-        [12] = 0x2a, /* mouse */
-    };
-    qemu_irq *irqs = opaque;
-    int ivec;
-
-    assert(n < ARRAY_SIZE(isa_irq_to_ivec));
-    ivec = isa_irq_to_ivec[n];
-    EBUS_DPRINTF("Set ISA IRQ %d level %d -> ivec 0x%x\n", n, level, ivec);
-    if (ivec) {
-        qemu_set_irq(irqs[ivec], level);
+    EbusState *s = EBUS(opaque);
+    qemu_irq irq = s->isa_bus_irqs[n];
+
+    /* Pass ISA bus IRQs onto their gpio equivalent */
+    EBUS_DPRINTF("Set ISA IRQ %d level %d\n", n, level);
+    if (irq) {
+        qemu_set_irq(irq, level);
     }
 }
 
@@ -235,7 +228,6 @@ static void isa_irq_handler(void *opaque, int n, int level)
 static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
     EbusState *s = EBUS(pci_dev);
-    APBState *apb;
     DeviceState *dev;
     qemu_irq *isa_irq;
     DriveInfo *fd[MAX_FD];
@@ -248,14 +240,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));
-    if (!apb) {
-        error_setg(errp, "unable to locate APB PCI host bridge");
-        return;
-    }
-
-    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
+    /* ISA bus */
+    isa_irq = qemu_allocate_irqs(ebus_isa_irq_handler, s, 16);
     isa_bus_irqs(s->isa_bus, isa_irq);
+    qdev_init_gpio_out_named(DEVICE(s), s->isa_bus_irqs, "isa-irq", 16);
 
     /* Serial ports */
     i = 0;
@@ -530,6 +518,18 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                          hwdef->console_serial_base);
     qdev_init_nofail(DEVICE(ebus));
 
+    /* Wire up "well-known" ISA IRQs to APB legacy obio IRQs */
+    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 7,
+        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_LPT_IRQ));
+    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 6,
+        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_FDD_IRQ));
+    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 1,
+        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_KBD_IRQ));
+    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
+        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_MSE_IRQ));
+    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 4,
+        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_SER_IRQ));
+
     pci_dev = pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
 
     memset(&macaddr, 0, sizeof(MACAddr));
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index dd49437..09ebd53 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -52,6 +52,13 @@ typedef struct IOMMUState {
 
 #define MAX_IVEC 0x40
 
+/* OBIO IVEC IRQs */
+#define OBIO_LPT_IRQ         0x22
+#define OBIO_FDD_IRQ         0x27
+#define OBIO_KBD_IRQ         0x29
+#define OBIO_MSE_IRQ         0x2a
+#define OBIO_SER_IRQ         0x2b
+
 #define TYPE_APB "pbm"
 
 #define APB_DEVICE(obj) \
@@ -76,7 +83,6 @@ typedef struct APBState {
     uint32_t pci_irq_map[8];
     uint32_t pci_err_irq_map[4];
     uint32_t obio_irq_map[32];
-    qemu_irq *pbm_irqs;
     qemu_irq ivec_irqs[MAX_IVEC];
     unsigned int irq_request;
     uint32_t reset_control;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 12/15] ebus: wire up OBIO interrupts to APB pbm via qdev GPIOs

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This enables us to remove the static array mapping in the ISA IRQ
> handler (and the embedded reference to the APB device) by formalising
> the interrupt wiring via the qdev GPIO API.
>
> For more clarity we replace the APB OBIO interrupt numbers with constants
> designating the interrupt source, and rename isa_irq_handler() to
> ebus_isa_irq_handler().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/pci-host/apb.c         |    2 +-
>  hw/sparc64/sun4u.c        |   48 ++++++++++++++++++++++-----------------------
>  include/hw/pci-host/apb.h |    8 +++++++-
>  3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 268100e..f092780c 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -700,7 +700,7 @@ static void pci_pbm_init(Object *obj)
>      for (i = 0; i < 32; i++) {
>          s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
>      }
> -    s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
> +    qdev_init_gpio_in_named(DEVICE(s), pci_apb_set_irq, "pbm-irq", MAX_IVEC);
>      qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
>      s->irq_request = NO_IRQ_REQUEST;
>      s->pci_irq_in = 0ULL;
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 0a30fb8..da386d3 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -86,6 +86,7 @@ typedef struct EbusState {
>      PCIDevice parent_obj;
>
>      ISABus *isa_bus;
> +    qemu_irq isa_bus_irqs[16];
>      uint64_t console_serial_base;
>      MemoryRegion bar0;
>      MemoryRegion bar1;
> @@ -211,23 +212,15 @@ typedef struct ResetData {
>      uint64_t prom_addr;
>  } ResetData;
>
> -static void isa_irq_handler(void *opaque, int n, int level)
> +static void ebus_isa_irq_handler(void *opaque, int n, int level)
>  {
> -    static const int isa_irq_to_ivec[16] = {
> -        [1] = 0x29, /* keyboard */
> -        [4] = 0x2b, /* serial */
> -        [6] = 0x27, /* floppy */
> -        [7] = 0x22, /* parallel */
> -        [12] = 0x2a, /* mouse */
> -    };
> -    qemu_irq *irqs = opaque;
> -    int ivec;
> -
> -    assert(n < ARRAY_SIZE(isa_irq_to_ivec));
> -    ivec = isa_irq_to_ivec[n];
> -    EBUS_DPRINTF("Set ISA IRQ %d level %d -> ivec 0x%x\n", n, level, ivec);
> -    if (ivec) {
> -        qemu_set_irq(irqs[ivec], level);
> +    EbusState *s = EBUS(opaque);
> +    qemu_irq irq = s->isa_bus_irqs[n];
> +
> +    /* Pass ISA bus IRQs onto their gpio equivalent */
> +    EBUS_DPRINTF("Set ISA IRQ %d level %d\n", n, level);
> +    if (irq) {
> +        qemu_set_irq(irq, level);
>      }
>  }
>
> @@ -235,7 +228,6 @@ static void isa_irq_handler(void *opaque, int n, int level)
>  static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
> -    APBState *apb;
>      DeviceState *dev;
>      qemu_irq *isa_irq;
>      DriveInfo *fd[MAX_FD];
> @@ -248,14 +240,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>          return;
>      }
>
> -    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));
> -    if (!apb) {
> -        error_setg(errp, "unable to locate APB PCI host bridge");
> -        return;
> -    }
> -
> -    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
> +    /* ISA bus */
> +    isa_irq = qemu_allocate_irqs(ebus_isa_irq_handler, s, 16);
>      isa_bus_irqs(s->isa_bus, isa_irq);
> +    qdev_init_gpio_out_named(DEVICE(s), s->isa_bus_irqs, "isa-irq", 16);
>
>      /* Serial ports */
>      i = 0;
> @@ -530,6 +518,18 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                           hwdef->console_serial_base);
>      qdev_init_nofail(DEVICE(ebus));
>
> +    /* Wire up "well-known" ISA IRQs to APB legacy obio IRQs */
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 7,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_LPT_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 6,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_FDD_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 1,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_KBD_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_MSE_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 4,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_SER_IRQ));
> +
>      pci_dev = pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
>
>      memset(&macaddr, 0, sizeof(MACAddr));
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index dd49437..09ebd53 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -52,6 +52,13 @@ typedef struct IOMMUState {
>
>  #define MAX_IVEC 0x40
>
> +/* OBIO IVEC IRQs */
> +#define OBIO_LPT_IRQ         0x22
> +#define OBIO_FDD_IRQ         0x27
> +#define OBIO_KBD_IRQ         0x29
> +#define OBIO_MSE_IRQ         0x2a
> +#define OBIO_SER_IRQ         0x2b
> +
>  #define TYPE_APB "pbm"
>
>  #define APB_DEVICE(obj) \
> @@ -76,7 +83,6 @@ typedef struct APBState {
>      uint32_t pci_irq_map[8];
>      uint32_t pci_err_irq_map[4];
>      uint32_t obio_irq_map[32];
> -    qemu_irq *pbm_irqs;
>      qemu_irq ivec_irqs[MAX_IVEC];
>      unsigned int irq_request;
>      uint32_t reset_control;
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 12/15] ebus: wire up OBIO interrupts to APB pbm via qdev GPIOs

[Philippe Mathieu-Daudé]

Hi Mark,

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This enables us to remove the static array mapping in the ISA IRQ
> handler (and the embedded reference to the APB device) by formalising
> the interrupt wiring via the qdev GPIO API.
> 
> For more clarity we replace the APB OBIO interrupt numbers with constants
> designating the interrupt source, and rename isa_irq_handler() to
> ebus_isa_irq_handler().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c         |    2 +-
>  hw/sparc64/sun4u.c        |   48 ++++++++++++++++++++++-----------------------
>  include/hw/pci-host/apb.h |    8 +++++++-
>  3 files changed, 32 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 268100e..f092780c 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -700,7 +700,7 @@ static void pci_pbm_init(Object *obj)
>      for (i = 0; i < 32; i++) {
>          s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
>      }
> -    s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
> +    qdev_init_gpio_in_named(DEVICE(s), pci_apb_set_irq, "pbm-irq", MAX_IVEC);
>      qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
>      s->irq_request = NO_IRQ_REQUEST;
>      s->pci_irq_in = 0ULL;
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 0a30fb8..da386d3 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -86,6 +86,7 @@ typedef struct EbusState {
>      PCIDevice parent_obj;
>  
>      ISABus *isa_bus;
> +    qemu_irq isa_bus_irqs[16];

Can you use ISA_NUM_IRQS here?

>      uint64_t console_serial_base;
>      MemoryRegion bar0;
>      MemoryRegion bar1;
> @@ -211,23 +212,15 @@ typedef struct ResetData {
>      uint64_t prom_addr;
>  } ResetData;
>  
> -static void isa_irq_handler(void *opaque, int n, int level)
> +static void ebus_isa_irq_handler(void *opaque, int n, int level)
>  {
> -    static const int isa_irq_to_ivec[16] = {
> -        [1] = 0x29, /* keyboard */
> -        [4] = 0x2b, /* serial */
> -        [6] = 0x27, /* floppy */
> -        [7] = 0x22, /* parallel */
> -        [12] = 0x2a, /* mouse */
> -    };
> -    qemu_irq *irqs = opaque;
> -    int ivec;
> -
> -    assert(n < ARRAY_SIZE(isa_irq_to_ivec));
> -    ivec = isa_irq_to_ivec[n];
> -    EBUS_DPRINTF("Set ISA IRQ %d level %d -> ivec 0x%x\n", n, level, ivec);
> -    if (ivec) {
> -        qemu_set_irq(irqs[ivec], level);
> +    EbusState *s = EBUS(opaque);
> +    qemu_irq irq = s->isa_bus_irqs[n];
> +
> +    /* Pass ISA bus IRQs onto their gpio equivalent */
> +    EBUS_DPRINTF("Set ISA IRQ %d level %d\n", n, level);
> +    if (irq) {
> +        qemu_set_irq(irq, level);
>      }
>  }
>  
> @@ -235,7 +228,6 @@ static void isa_irq_handler(void *opaque, int n, int level)
>  static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      EbusState *s = EBUS(pci_dev);
> -    APBState *apb;
>      DeviceState *dev;
>      qemu_irq *isa_irq;
>      DriveInfo *fd[MAX_FD];
> @@ -248,14 +240,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>          return;
>      }
>  
> -    apb = APB_DEVICE(object_resolve_path_type("", TYPE_APB, NULL));
> -    if (!apb) {
> -        error_setg(errp, "unable to locate APB PCI host bridge");
> -        return;
> -    }
> -
> -    isa_irq = qemu_allocate_irqs(isa_irq_handler, apb->pbm_irqs, 16);
> +    /* ISA bus */
> +    isa_irq = qemu_allocate_irqs(ebus_isa_irq_handler, s, 16);

Here,

>      isa_bus_irqs(s->isa_bus, isa_irq);
> +    qdev_init_gpio_out_named(DEVICE(s), s->isa_bus_irqs, "isa-irq", 16);

Here too.

>  
>      /* Serial ports */
>      i = 0;
> @@ -530,6 +518,18 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                           hwdef->console_serial_base);
>      qdev_init_nofail(DEVICE(ebus));
>  
> +    /* Wire up "well-known" ISA IRQs to APB legacy obio IRQs */
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 7,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_LPT_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 6,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_FDD_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 1,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_KBD_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_MSE_IRQ));
> +    qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 4,
> +        qdev_get_gpio_in_named(DEVICE(apb), "pbm-irq", OBIO_SER_IRQ));
> +
>      pci_dev = pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
>  
>      memset(&macaddr, 0, sizeof(MACAddr));
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index dd49437..09ebd53 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -52,6 +52,13 @@ typedef struct IOMMUState {
>  
>  #define MAX_IVEC 0x40
>  
> +/* OBIO IVEC IRQs */
> +#define OBIO_LPT_IRQ         0x22
> +#define OBIO_FDD_IRQ         0x27
> +#define OBIO_KBD_IRQ         0x29
> +#define OBIO_MSE_IRQ         0x2a
> +#define OBIO_SER_IRQ         0x2b
> +
>  #define TYPE_APB "pbm"
>  
>  #define APB_DEVICE(obj) \
> @@ -76,7 +83,6 @@ typedef struct APBState {
>      uint32_t pci_irq_map[8];
>      uint32_t pci_err_irq_map[4];
>      uint32_t obio_irq_map[32];
> -    qemu_irq *pbm_irqs;
>      qemu_irq ivec_irqs[MAX_IVEC];
>      unsigned int irq_request;
>      uint32_t reset_control;

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [Qemu-devel] [PATCH 12/15] ebus: wire up OBIO interrupts to APB pbm via qdev GPIOs

[Mark Cave-Ayland]

On 20/11/17 01:02, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
>> This enables us to remove the static array mapping in the ISA IRQ
>> handler (and the embedded reference to the APB device) by formalising
>> the interrupt wiring via the qdev GPIO API.
>>
>> For more clarity we replace the APB OBIO interrupt numbers with constants
>> designating the interrupt source, and rename isa_irq_handler() to
>> ebus_isa_irq_handler().
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/pci-host/apb.c         |    2 +-
>>  hw/sparc64/sun4u.c        |   48 ++++++++++++++++++++++-----------------------
>>  include/hw/pci-host/apb.h |    8 +++++++-
>>  3 files changed, 32 insertions(+), 26 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 268100e..f092780c 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -700,7 +700,7 @@ static void pci_pbm_init(Object *obj)
>>      for (i = 0; i < 32; i++) {
>>          s->obio_irq_map[i] = ((0x1f << 6) | 0x20) + i;
>>      }
>> -    s->pbm_irqs = qemu_allocate_irqs(pci_apb_set_irq, s, MAX_IVEC);
>> +    qdev_init_gpio_in_named(DEVICE(s), pci_apb_set_irq, "pbm-irq", MAX_IVEC);
>>      qdev_init_gpio_out_named(DEVICE(s), s->ivec_irqs, "ivec-irq", MAX_IVEC);
>>      s->irq_request = NO_IRQ_REQUEST;
>>      s->pci_irq_in = 0ULL;
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 0a30fb8..da386d3 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -86,6 +86,7 @@ typedef struct EbusState {
>>      PCIDevice parent_obj;
>>  
>>      ISABus *isa_bus;
>> +    qemu_irq isa_bus_irqs[16];
> 
> Can you use ISA_NUM_IRQS here?

Ah yes, good spot. I've updated this (and the other occurrences you
found) to ISA_NUM_IRQS accordingly.


ATB,

Mark.

[Qemu-devel] [PATCH 13/15] apb: replace OBIO interrupt numbers in pci_pbmA_map_irq() with constants

[Mark Cave-Ayland]

Following on from the previous commit, we can also do the same with
with legacy OBIO interrupts in pci_pbmA_map_irq().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c         |    4 ++--
 include/hw/pci-host/apb.h |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index f092780c..31eb675 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -523,10 +523,10 @@ static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
     switch (PCI_SLOT(pci_dev->devfn)) {
     case 1:
         /* Onboard NIC */
-        return 0x21;
+        return OBIO_NIC_IRQ;
     case 3:
         /* Onboard IDE */
-        return 0x20;
+        return OBIO_HDD_IRQ;
     default:
         /* Normal intno, fall through */
         break;
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index 09ebd53..6194c8c 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -53,6 +53,8 @@ typedef struct IOMMUState {
 #define MAX_IVEC 0x40
 
 /* OBIO IVEC IRQs */
+#define OBIO_HDD_IRQ         0x20
+#define OBIO_NIC_IRQ         0x21
 #define OBIO_LPT_IRQ         0x22
 #define OBIO_FDD_IRQ         0x27
 #define OBIO_KBD_IRQ         0x29
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 13/15] apb: replace OBIO interrupt numbers in pci_pbmA_map_irq() with constants

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Following on from the previous commit, we can also do the same with
> with legacy OBIO interrupts in pci_pbmA_map_irq().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/pci-host/apb.c         |    4 ++--
>  include/hw/pci-host/apb.h |    2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index f092780c..31eb675 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -523,10 +523,10 @@ static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>      switch (PCI_SLOT(pci_dev->devfn)) {
>      case 1:
>          /* Onboard NIC */
> -        return 0x21;
> +        return OBIO_NIC_IRQ;
>      case 3:
>          /* Onboard IDE */
> -        return 0x20;
> +        return OBIO_HDD_IRQ;
>      default:
>          /* Normal intno, fall through */
>          break;
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index 09ebd53..6194c8c 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -53,6 +53,8 @@ typedef struct IOMMUState {
>  #define MAX_IVEC 0x40
>
>  /* OBIO IVEC IRQs */
> +#define OBIO_HDD_IRQ         0x20
> +#define OBIO_NIC_IRQ         0x21
>  #define OBIO_LPT_IRQ         0x22
>  #define OBIO_FDD_IRQ         0x27
>  #define OBIO_KBD_IRQ         0x29
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 13/15] apb: replace OBIO interrupt numbers in pci_pbmA_map_irq() with constants

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Following on from the previous commit, we can also do the same with
> with legacy OBIO interrupts in pci_pbmA_map_irq().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/pci-host/apb.c         |    4 ++--
>  include/hw/pci-host/apb.h |    2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index f092780c..31eb675 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -523,10 +523,10 @@ static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
>      switch (PCI_SLOT(pci_dev->devfn)) {
>      case 1:
>          /* Onboard NIC */
> -        return 0x21;
> +        return OBIO_NIC_IRQ;
>      case 3:
>          /* Onboard IDE */
> -        return 0x20;
> +        return OBIO_HDD_IRQ;
>      default:
>          /* Normal intno, fall through */
>          break;
> diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
> index 09ebd53..6194c8c 100644
> --- a/include/hw/pci-host/apb.h
> +++ b/include/hw/pci-host/apb.h
> @@ -53,6 +53,8 @@ typedef struct IOMMUState {
>  #define MAX_IVEC 0x40
>  
>  /* OBIO IVEC IRQs */
> +#define OBIO_HDD_IRQ         0x20
> +#define OBIO_NIC_IRQ         0x21
>  #define OBIO_LPT_IRQ         0x22
>  #define OBIO_FDD_IRQ         0x27
>  #define OBIO_KBD_IRQ         0x29
> 

[Qemu-devel] [PATCH 14/15] sparc64: introduce trace-events for hw/sparc64

[Mark Cave-Ayland]

This is in preparation for switching code in hw/sparc64 from DPRINTF over to
trace events.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 Makefile.objs           |    1 +
 hw/sparc64/trace-events |    1 +
 2 files changed, 2 insertions(+)
 create mode 100644 hw/sparc64/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 285c6f3..c8b1bba 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -140,6 +140,7 @@ trace-events-subdirs += hw/input
 trace-events-subdirs += hw/timer
 trace-events-subdirs += hw/dma
 trace-events-subdirs += hw/sparc
+trace-events-subdirs += hw/sparc64
 trace-events-subdirs += hw/sd
 trace-events-subdirs += hw/isa
 trace-events-subdirs += hw/mem
diff --git a/hw/sparc64/trace-events b/hw/sparc64/trace-events
new file mode 100644
index 0000000..9284b1f
--- /dev/null
+++ b/hw/sparc64/trace-events
@@ -0,0 +1 @@
+# See docs/devel/tracing.txt for syntax documentation.
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 14/15] sparc64: introduce trace-events for hw/sparc64

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This is in preparation for switching code in hw/sparc64 from DPRINTF over to
> trace events.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  Makefile.objs           |    1 +
>  hw/sparc64/trace-events |    1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 hw/sparc64/trace-events
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 285c6f3..c8b1bba 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -140,6 +140,7 @@ trace-events-subdirs += hw/input
>  trace-events-subdirs += hw/timer
>  trace-events-subdirs += hw/dma
>  trace-events-subdirs += hw/sparc
> +trace-events-subdirs += hw/sparc64
>  trace-events-subdirs += hw/sd
>  trace-events-subdirs += hw/isa
>  trace-events-subdirs += hw/mem
> diff --git a/hw/sparc64/trace-events b/hw/sparc64/trace-events
> new file mode 100644
> index 0000000..9284b1f
> --- /dev/null
> +++ b/hw/sparc64/trace-events
> @@ -0,0 +1 @@
> +# See docs/devel/tracing.txt for syntax documentation.
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 14/15] sparc64: introduce trace-events for hw/sparc64

[Philippe Mathieu-Daudé]

Hi Mark,

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> This is in preparation for switching code in hw/sparc64 from DPRINTF over to
> trace events.

This could be squashed with next commit,

Either way:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  Makefile.objs           |    1 +
>  hw/sparc64/trace-events |    1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 hw/sparc64/trace-events
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 285c6f3..c8b1bba 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -140,6 +140,7 @@ trace-events-subdirs += hw/input
>  trace-events-subdirs += hw/timer
>  trace-events-subdirs += hw/dma
>  trace-events-subdirs += hw/sparc
> +trace-events-subdirs += hw/sparc64
>  trace-events-subdirs += hw/sd
>  trace-events-subdirs += hw/isa
>  trace-events-subdirs += hw/mem
> diff --git a/hw/sparc64/trace-events b/hw/sparc64/trace-events
> new file mode 100644
> index 0000000..9284b1f
> --- /dev/null
> +++ b/hw/sparc64/trace-events
> @@ -0,0 +1 @@
> +# See docs/devel/tracing.txt for syntax documentation.
> 

Re: [Qemu-devel] [PATCH 14/15] sparc64: introduce trace-events for hw/sparc64

[Mark Cave-Ayland]

On 19/11/17 15:14, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
>> This is in preparation for switching code in hw/sparc64 from DPRINTF over to
>> trace events.
> 
> This could be squashed with next commit,
> 
> Either way:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks for the review! I'd prefer to keep this separate as it touches an
area which is outside SPARC just in case anyone wants to handle this
separately.


ATB,

Mark.

[Qemu-devel] [PATCH 15/15] sun4u: switch from EBUS_DPRINTF() macro to trace-events

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc64/sun4u.c      |   12 ++----------
 hw/sparc64/trace-events |    3 +++
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index da386d3..50ccf75 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -47,17 +47,9 @@
 #include "hw/ide/pci.h"
 #include "hw/loader.h"
 #include "elf.h"
+#include "trace.h"
 #include "qemu/cutils.h"
 
-//#define DEBUG_EBUS
-
-#ifdef DEBUG_EBUS
-#define EBUS_DPRINTF(fmt, ...)                                  \
-    do { printf("EBUS: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define EBUS_DPRINTF(fmt, ...)
-#endif
-
 #define KERNEL_LOAD_ADDR     0x00404000
 #define CMDLINE_ADDR         0x003ff000
 #define PROM_SIZE_MAX        (4 * 1024 * 1024)
@@ -218,7 +210,7 @@ static void ebus_isa_irq_handler(void *opaque, int n, int level)
     qemu_irq irq = s->isa_bus_irqs[n];
 
     /* Pass ISA bus IRQs onto their gpio equivalent */
-    EBUS_DPRINTF("Set ISA IRQ %d level %d\n", n, level);
+    trace_ebus_isa_irq_handler(n, level);
     if (irq) {
         qemu_set_irq(irq, level);
     }
diff --git a/hw/sparc64/trace-events b/hw/sparc64/trace-events
index 9284b1f..04d80b7 100644
--- a/hw/sparc64/trace-events
+++ b/hw/sparc64/trace-events
@@ -1 +1,4 @@
 # See docs/devel/tracing.txt for syntax documentation.
+
+# hw/sparc64/sun4u.c
+ebus_isa_irq_handler(int n, int level) "Set ISA IRQ %d level %d"
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 15/15] sun4u: switch from EBUS_DPRINTF() macro to trace-events

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/sparc64/sun4u.c      |   12 ++----------
>  hw/sparc64/trace-events |    3 +++
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index da386d3..50ccf75 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -47,17 +47,9 @@
>  #include "hw/ide/pci.h"
>  #include "hw/loader.h"
>  #include "elf.h"
> +#include "trace.h"
>  #include "qemu/cutils.h"
>  
> -//#define DEBUG_EBUS
> -
> -#ifdef DEBUG_EBUS
> -#define EBUS_DPRINTF(fmt, ...)                                  \
> -    do { printf("EBUS: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define EBUS_DPRINTF(fmt, ...)
> -#endif
> -
>  #define KERNEL_LOAD_ADDR     0x00404000
>  #define CMDLINE_ADDR         0x003ff000
>  #define PROM_SIZE_MAX        (4 * 1024 * 1024)
> @@ -218,7 +210,7 @@ static void ebus_isa_irq_handler(void *opaque, int n, int level)
>      qemu_irq irq = s->isa_bus_irqs[n];
>  
>      /* Pass ISA bus IRQs onto their gpio equivalent */
> -    EBUS_DPRINTF("Set ISA IRQ %d level %d\n", n, level);
> +    trace_ebus_isa_irq_handler(n, level);
>      if (irq) {
>          qemu_set_irq(irq, level);
>      }
> diff --git a/hw/sparc64/trace-events b/hw/sparc64/trace-events
> index 9284b1f..04d80b7 100644
> --- a/hw/sparc64/trace-events
> +++ b/hw/sparc64/trace-events
> @@ -1 +1,4 @@
>  # See docs/devel/tracing.txt for syntax documentation.
> +
> +# hw/sparc64/sun4u.c
> +ebus_isa_irq_handler(int n, int level) "Set ISA IRQ %d level %d"
>