[PATCH 00/18] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified

[PATCH 00/18] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified

[Philippe Mathieu-Daudé]

Patches I wrote while reviewing/testing Jiaxun's v3 [1] trying
to understand the problem with "Fixup pci.lomem mapping" [2].

The issue should be fixed by patch #8 of this series:
'Remap PCI "lo" regions when PCIMAP reg is modified'.
The rest are cleanups patches.

Happy new year,

Phil.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769286.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769294.html

Philippe Mathieu-Daudé (18):
  hw/pci-host/bonito: Remove unused definitions
  hw/pci-host/bonito: Display hexadecimal value with '0x' prefix
  hw/pci-host/bonito: Use PCI_DEVFN() macro
  hw/pci-host/bonito: Use pci_config_set_interrupt_pin()
  hw/pci-host/bonito: Simplify soft reset using FIELD_EX32()
  hw/pci-host/bonito: Do not allow big-endian targets
  hw/pci-host/bonito: Make BONPONCFG register read-only
  hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified
  hw/pci-host/bonito: Rename north bridge helpers
  hw/pci-host/bonito: Rename PCI host helpers
  hw/pci-host/bonito: Rename PCI function helpers
  hw/pci-host/bonito: Rename PCIBonitoState -> BonitoPciState
  hw/pci-host/bonito: Create PCI regions in bonito_host_realize()
  hw/pci-host/bonito: Simplify using pci_host_conf_le_ops
    MemoryRegionOps
  hw/pci-host/bonito: Map north bridge region in bonito_host_realize()
  hw/pci-host/bonito: Create TYPE_PCI_BONITO in bonito_host_realize()
  hw/pci-host/bonito: Declare TYPE_BONITO_PCI_HOST_BRIDGE in include/
  hw/mips/fuloong2e: Inline bonito_init()

 include/hw/mips/mips.h       |   3 -
 include/hw/pci-host/bonito.h |  34 ++++
 hw/mips/fuloong2e.c          |   7 +-
 hw/pci-host/bonito.c         | 357 +++++++++++++++--------------------
 MAINTAINERS                  |   1 +
 5 files changed, 197 insertions(+), 205 deletions(-)
 create mode 100644 include/hw/pci-host/bonito.h

-- 
2.26.2

[PATCH 01/18] hw/pci-host/bonito: Remove unused definitions

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a99eced0657..0634f3289c4 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -189,12 +189,6 @@ FIELD(BONGENCFG, PCIQUEUE,      12, 1)
 #define BONITO_PCICONF_REG_MASK        0xFC
 #define BONITO_PCICONF_REG_OFFSET      0
 
-
-/* idsel BIT = pci slot number +12 */
-#define PCI_SLOT_BASE              12
-#define PCI_IDSEL_VIA686B_BIT      (17)
-#define PCI_IDSEL_VIA686B          (1 << PCI_IDSEL_VIA686B_BIT)
-
 #define PCI_ADDR(busno , devno , funno , regno)  \
     ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \
     (((funno) << 8) & 0x700) + (regno))
-- 
2.26.2

Re: [PATCH 01/18] hw/pci-host/bonito: Remove unused definitions

[Huacai Chen]

Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

On Fri, Jan 1, 2021 at 6:49 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/bonito.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index a99eced0657..0634f3289c4 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -189,12 +189,6 @@ FIELD(BONGENCFG, PCIQUEUE,      12, 1)
>  #define BONITO_PCICONF_REG_MASK        0xFC
>  #define BONITO_PCICONF_REG_OFFSET      0
>
> -
> -/* idsel BIT = pci slot number +12 */
> -#define PCI_SLOT_BASE              12
> -#define PCI_IDSEL_VIA686B_BIT      (17)
> -#define PCI_IDSEL_VIA686B          (1 << PCI_IDSEL_VIA686B_BIT)
> -
>  #define PCI_ADDR(busno , devno , funno , regno)  \
>      ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \
>      (((funno) << 8) & 0x700) + (regno))
> --
> 2.26.2
>

[PATCH 02/18] hw/pci-host/bonito: Display hexadecimal value with '0x' prefix

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 0634f3289c4..efeba29011f 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -463,8 +463,8 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr addr)
     regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >> BONITO_PCICONF_REG_OFFSET;
 
     if (idsel == 0) {
-        error_report("error in bonito pci config address " TARGET_FMT_plx
-                     ",pcimap_cfg=%x", addr, s->regs[BONITO_PCIMAP_CFG]);
+        error_report("error in bonito pci config address 0x" TARGET_FMT_plx
+                     ",pcimap_cfg=0x%x", addr, s->regs[BONITO_PCIMAP_CFG]);
         exit(1);
     }
     pciaddr = PCI_ADDR(pci_bus_num(phb->bus), devno, funno, regno);
-- 
2.26.2

Re: [PATCH 02/18] hw/pci-host/bonito: Display hexadecimal value with '0x' prefix

[Huacai Chen]

Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

On Fri, Jan 1, 2021 at 6:49 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/bonito.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 0634f3289c4..efeba29011f 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -463,8 +463,8 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr addr)
>      regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >> BONITO_PCICONF_REG_OFFSET;
>
>      if (idsel == 0) {
> -        error_report("error in bonito pci config address " TARGET_FMT_plx
> -                     ",pcimap_cfg=%x", addr, s->regs[BONITO_PCIMAP_CFG]);
> +        error_report("error in bonito pci config address 0x" TARGET_FMT_plx
> +                     ",pcimap_cfg=0x%x", addr, s->regs[BONITO_PCIMAP_CFG]);
>          exit(1);
>      }
>      pciaddr = PCI_ADDR(pci_bus_num(phb->bus), devno, funno, regno);
> --
> 2.26.2
>

[PATCH 03/18] hw/pci-host/bonito: Use PCI_DEVFN() macro

[Philippe Mathieu-Daudé]

Use the PCI_DEVFN() macro to replace the '0x28' magic value,
this way it is clearer we access PCI function #0 of slot #5.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index efeba29011f..4c903d4f682 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -626,7 +626,7 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
     phb->bus = pci_register_root_bus(dev, "pci",
                                      pci_bonito_set_irq, pci_bonito_map_irq,
                                      dev, &bs->pci_mem, get_system_io(),
-                                     0x28, 32, TYPE_PCI_BUS);
+                                     PCI_DEVFN(5, 0), 32, TYPE_PCI_BUS);
 
     for (size_t i = 0; i < 3; i++) {
         char *name = g_strdup_printf("pci.lomem%zu", i);
-- 
2.26.2

Re: [PATCH 03/18] hw/pci-host/bonito: Use PCI_DEVFN() macro

[Huacai Chen]

Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

On Fri, Jan 1, 2021 at 6:49 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Use the PCI_DEVFN() macro to replace the '0x28' magic value,
> this way it is clearer we access PCI function #0 of slot #5.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/bonito.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index efeba29011f..4c903d4f682 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -626,7 +626,7 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>      phb->bus = pci_register_root_bus(dev, "pci",
>                                       pci_bonito_set_irq, pci_bonito_map_irq,
>                                       dev, &bs->pci_mem, get_system_io(),
> -                                     0x28, 32, TYPE_PCI_BUS);
> +                                     PCI_DEVFN(5, 0), 32, TYPE_PCI_BUS);
>
>      for (size_t i = 0; i < 3; i++) {
>          char *name = g_strdup_printf("pci.lomem%zu", i);
> --
> 2.26.2
>

[PATCH 04/18] hw/pci-host/bonito: Use pci_config_set_interrupt_pin()

[Philippe Mathieu-Daudé]

Replace pci_set_byte(PCI_INTERRUPT_PIN) by
pci_config_set_interrupt_pin().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 4c903d4f682..fe94e6740b5 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -723,7 +723,8 @@ static void bonito_realize(PCIDevice *dev, Error **errp)
     pci_set_word(dev->config + PCI_SUBSYSTEM_ID, 0x0000);
 
     pci_set_byte(dev->config + PCI_INTERRUPT_LINE, 0x00);
-    pci_set_byte(dev->config + PCI_INTERRUPT_PIN, 0x01);
+    pci_config_set_interrupt_pin(dev->config, 0x01); /* interrupt pin A */
+
     pci_set_byte(dev->config + PCI_MIN_GNT, 0x3c);
     pci_set_byte(dev->config + PCI_MAX_LAT, 0x00);
 
-- 
2.26.2

Re: [PATCH 04/18] hw/pci-host/bonito: Use pci_config_set_interrupt_pin()

[Huacai Chen]

Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

On Fri, Jan 1, 2021 at 6:49 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Replace pci_set_byte(PCI_INTERRUPT_PIN) by
> pci_config_set_interrupt_pin().
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/bonito.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 4c903d4f682..fe94e6740b5 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -723,7 +723,8 @@ static void bonito_realize(PCIDevice *dev, Error **errp)
>      pci_set_word(dev->config + PCI_SUBSYSTEM_ID, 0x0000);
>
>      pci_set_byte(dev->config + PCI_INTERRUPT_LINE, 0x00);
> -    pci_set_byte(dev->config + PCI_INTERRUPT_PIN, 0x01);
> +    pci_config_set_interrupt_pin(dev->config, 0x01); /* interrupt pin A */
> +
>      pci_set_byte(dev->config + PCI_MIN_GNT, 0x3c);
>      pci_set_byte(dev->config + PCI_MAX_LAT, 0x00);
>
> --
> 2.26.2
>

[PATCH 05/18] hw/pci-host/bonito: Simplify soft reset using FIELD_EX32()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index fe94e6740b5..104c58331d0 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -244,7 +244,6 @@ static void bonito_writel(void *opaque, hwaddr addr,
 {
     PCIBonitoState *s = opaque;
     uint32_t saddr;
-    int reset = 0;
 
     saddr = addr >> 2;
 
@@ -277,13 +276,12 @@ static void bonito_writel(void *opaque, hwaddr addr,
         s->regs[saddr] = val;
         break;
     case BONITO_BONGENCFG:
-        if (!(s->regs[saddr] & 0x04) && (val & 0x04)) {
-            reset = 1; /* bit 2 jump from 0 to 1 cause reset */
-        }
-        s->regs[saddr] = val;
-        if (reset) {
+        if (!FIELD_EX32(s->regs[saddr], BONGENCFG, CPUSELFRESET)
+                && FIELD_EX32(val, BONGENCFG, CPUSELFRESET)) {
+            /* bit 2 jump from 0 to 1 cause reset */
             qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
+        s->regs[saddr] = val;
         break;
     case BONITO_INTENSET:
         s->regs[BONITO_INTENSET] = val;
-- 
2.26.2

[RFC PATCH 06/18] hw/pci-host/bonito: Do not allow big-endian targets

[Philippe Mathieu-Daudé]

This model is not complete for big-endian targets,
do not allow its use.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Yes I must improve that, I know =) However enough for now
to post the following patches.
---
 hw/pci-host/bonito.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 104c58331d0..93820f69e49 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -53,6 +53,7 @@
 #include "hw/misc/unimp.h"
 #include "hw/registerfields.h"
 #include "qom/object.h"
+#include "hw/core/cpu.h"
 
 /* #define DEBUG_BONITO */
 
@@ -354,7 +355,7 @@ static uint64_t bonito_pciconf_readl(void *opaque, hwaddr addr,
 static const MemoryRegionOps bonito_pciconf_ops = {
     .read = bonito_pciconf_readl,
     .write = bonito_pciconf_writel,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
@@ -648,6 +649,8 @@ static void bonito_realize(PCIDevice *dev, Error **errp)
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
     MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
 
+    assert(!target_words_bigendian()); /* FIXME not supported */
+
     /*
      * Bonito North Bridge, built on FPGA,
      * VENDOR_ID/DEVICE_ID are "undefined"
-- 
2.26.2

[PATCH 07/18] hw/pci-host/bonito: Make BONPONCFG register read-only

[Philippe Mathieu-Daudé]

Per the datasheet (Chapter 5.2. "Power-on settable configuration
register - bonponcfg"), the bonponcfg can only be modified using
"pull-up on the corresponding IOD0-15 signal". Do not allow
update of this register by the CPU.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 93820f69e49..29c0294d289 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -251,7 +251,6 @@ static void bonito_writel(void *opaque, hwaddr addr,
     DPRINTF("bonito_writel "TARGET_FMT_plx" val %lx saddr %x\n",
             addr, val, saddr);
     switch (saddr) {
-    case BONITO_BONPONCFG:
     case BONITO_IODEVCFG:
     case BONITO_SDCFG:
     case BONITO_PCIMAP:
@@ -292,6 +291,7 @@ static void bonito_writel(void *opaque, hwaddr addr,
         s->regs[BONITO_INTENCLR] = val;
         s->regs[BONITO_INTEN] &= ~val;
         break;
+    case BONITO_BONPONCFG:
     case BONITO_INTEN:
     case BONITO_INTISR:
         DPRINTF("write to readonly bonito register %x\n", saddr);
-- 
2.26.2

[RFC PATCH 08/18] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified

[Philippe Mathieu-Daudé]

Per the datasheet (Chapter 5.7.1. "PCI address regions"),
the PCIMAP register:

  Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
  each of which can be assigned to any 64 Mbyte-aligned region of
  PCI memory. The address appearing on the PCI bus consists of the
  low 26 bits of the CPU physical address, with the high 6 bits
  coming from the appropriate base6 field. Each of the three regions
  is an independent window onto PCI memory, and can be positioned on
  any 64Mbyte boundary in PCI space.

Remap the 3 regions on reset and when PCIMAP is updated.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC: memory_region_is_mapped() is probably not the best call to check
     if this is the first call.

 hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 29c0294d289..a091ef15d27 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -138,6 +138,10 @@ FIELD(BONGENCFG, PCIQUEUE,      12, 1)
 
 /* 4. PCI address map control */
 #define BONITO_PCIMAP           (0x10 >> 2)      /* 0x110 */
+FIELD(PCIMAP, LO0,               0, 6)
+FIELD(PCIMAP, LO1,               6, 6)
+FIELD(PCIMAP, LO2,              12, 6)
+FIELD(PCIMAP, 2G,               18, 1)
 #define BONITO_PCIMEMBASECFG    (0x14 >> 2)      /* 0x114 */
 #define BONITO_PCIMAP_CFG       (0x18 >> 2)      /* 0x118 */
 
@@ -232,6 +236,7 @@ struct BonitoState {
     qemu_irq *pic;
     PCIBonitoState *pci_dev;
     MemoryRegion pci_mem;
+    MemoryRegion pcimem_lo_alias[3];
 };
 
 #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
@@ -240,6 +245,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
 #define TYPE_PCI_BONITO "Bonito"
 OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
 
+static void bonito_remap(PCIBonitoState *s)
+{
+    static const char *const region_name[3] = {
+        "pci.lomem0", "pci.lomem1", "pci.lomem2"
+    };
+    BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
+
+    for (size_t i = 0; i < 3; i++) {
+        uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
+
+        if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
+            memory_region_del_subregion(get_system_memory(),
+                                        &bs->pcimem_lo_alias[i]);
+            object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
+        }
+
+        memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
+                                 region_name[i], &bs->pci_mem,
+                                 offset, 64 * MiB);
+        memory_region_add_subregion(get_system_memory(),
+                                    BONITO_PCILO_BASE + i * 64 * MiB,
+                                    &bs->pcimem_lo_alias[i]);
+    }
+}
+
 static void bonito_writel(void *opaque, hwaddr addr,
                           uint64_t val, unsigned size)
 {
@@ -253,7 +283,6 @@ static void bonito_writel(void *opaque, hwaddr addr,
     switch (saddr) {
     case BONITO_IODEVCFG:
     case BONITO_SDCFG:
-    case BONITO_PCIMAP:
     case BONITO_PCIMEMBASECFG:
     case BONITO_PCIMAP_CFG:
     case BONITO_GPIODATA:
@@ -275,6 +304,10 @@ static void bonito_writel(void *opaque, hwaddr addr,
     case BONITO_MEMSIZE:
         s->regs[saddr] = val;
         break;
+    case BONITO_PCIMAP:
+        s->regs[saddr] = val;
+        bonito_remap(s);
+        break;
     case BONITO_BONGENCFG:
         if (!FIELD_EX32(s->regs[saddr], BONGENCFG, CPUSELFRESET)
                 && FIELD_EX32(val, BONGENCFG, CPUSELFRESET)) {
@@ -603,6 +636,8 @@ static void bonito_reset(void *opaque)
     s->regs[BONITO_DQCFG] = 0x8;
     s->regs[BONITO_MEMSIZE] = 0x10000000;
     s->regs[BONITO_PCIMAP] = 0x6140;
+
+    bonito_remap(s);
 }
 
 static const VMStateDescription vmstate_bonito = {
@@ -619,7 +654,6 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
-    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
 
     memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
     phb->bus = pci_register_root_bus(dev, "pci",
@@ -627,17 +661,6 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
                                      dev, &bs->pci_mem, get_system_io(),
                                      PCI_DEVFN(5, 0), 32, TYPE_PCI_BUS);
 
-    for (size_t i = 0; i < 3; i++) {
-        char *name = g_strdup_printf("pci.lomem%zu", i);
-
-        memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
-                                 &bs->pci_mem, i * 64 * MiB, 64 * MiB);
-        memory_region_add_subregion(get_system_memory(),
-                                    BONITO_PCILO_BASE + i * 64 * MiB,
-                                    &pcimem_lo_alias[i]);
-        g_free(name);
-    }
-
     create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
 }
 
-- 
2.26.2

[PATCH 09/18] hw/pci-host/bonito: Rename north bridge helpers

[Philippe Mathieu-Daudé]

Rename north bridge related helpers as 'bonito_northbridge_*'.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a091ef15d27..d91ed82c99e 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -270,7 +270,7 @@ static void bonito_remap(PCIBonitoState *s)
     }
 }
 
-static void bonito_writel(void *opaque, hwaddr addr,
+static void bonito_northbridge_write(void *opaque, hwaddr addr,
                           uint64_t val, unsigned size)
 {
     PCIBonitoState *s = opaque;
@@ -335,7 +335,7 @@ static void bonito_writel(void *opaque, hwaddr addr,
     }
 }
 
-static uint64_t bonito_readl(void *opaque, hwaddr addr,
+static uint64_t bonito_northbridge_read(void *opaque, hwaddr addr,
                              unsigned size)
 {
     PCIBonitoState *s = opaque;
@@ -352,9 +352,9 @@ static uint64_t bonito_readl(void *opaque, hwaddr addr,
     }
 }
 
-static const MemoryRegionOps bonito_ops = {
-    .read = bonito_readl,
-    .write = bonito_writel,
+static const MemoryRegionOps bonito_northbridge_ops = {
+    .read = bonito_northbridge_read,
+    .write = bonito_northbridge_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 4,
@@ -681,7 +681,7 @@ static void bonito_realize(PCIDevice *dev, Error **errp)
     pci_config_set_prog_interface(dev->config, 0x00);
 
     /* set the north bridge register mapping */
-    memory_region_init_io(&s->iomem, OBJECT(s), &bonito_ops, s,
+    memory_region_init_io(&s->iomem, OBJECT(s), &bonito_northbridge_ops, s,
                           "north-bridge-register", BONITO_INTERNAL_REG_SIZE);
     sysbus_init_mmio(sysbus, &s->iomem);
     sysbus_mmio_map(sysbus, 0, BONITO_INTERNAL_REG_BASE);
-- 
2.26.2

[PATCH 10/18] hw/pci-host/bonito: Rename PCI host helpers

[Philippe Mathieu-Daudé]

Rename PCI host side related helpers as 'bonito_host_*'.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index d91ed82c99e..45f31512b25 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -574,7 +574,7 @@ static const MemoryRegionOps bonito_spciconf_ops = {
 
 #define BONITO_IRQ_BASE 32
 
-static void pci_bonito_set_irq(void *opaque, int irq_num, int level)
+static void bonito_host_set_irq(void *opaque, int irq_num, int level)
 {
     BonitoState *s = opaque;
     qemu_irq *pic = s->pic;
@@ -593,7 +593,7 @@ static void pci_bonito_set_irq(void *opaque, int irq_num, int level)
 }
 
 /* map the original irq (0~3) to bonito irq (16~47, but 16~31 are unused) */
-static int pci_bonito_map_irq(PCIDevice *pci_dev, int irq_num)
+static int bonito_host_map_irq(PCIDevice *pci_dev, int irq_num)
 {
     int slot;
 
@@ -650,14 +650,14 @@ static const VMStateDescription vmstate_bonito = {
     }
 };
 
-static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
+static void bonito_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
 
     memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
     phb->bus = pci_register_root_bus(dev, "pci",
-                                     pci_bonito_set_irq, pci_bonito_map_irq,
+                                     bonito_host_set_irq, bonito_host_map_irq,
                                      dev, &bs->pci_mem, get_system_io(),
                                      PCI_DEVFN(5, 0), 32, TYPE_PCI_BUS);
 
@@ -808,23 +808,23 @@ static const TypeInfo bonito_info = {
     },
 };
 
-static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
+static void bonito_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->realize = bonito_pcihost_realize;
+    dc->realize = bonito_host_realize;
 }
 
-static const TypeInfo bonito_pcihost_info = {
+static const TypeInfo bonito_host_info = {
     .name          = TYPE_BONITO_PCI_HOST_BRIDGE,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(BonitoState),
-    .class_init    = bonito_pcihost_class_init,
+    .class_init    = bonito_host_class_init,
 };
 
 static void bonito_register_types(void)
 {
-    type_register_static(&bonito_pcihost_info);
+    type_register_static(&bonito_host_info);
     type_register_static(&bonito_info);
 }
 
-- 
2.26.2

[PATCH 11/18] hw/pci-host/bonito: Rename PCI function helpers

[Philippe Mathieu-Daudé]

Rename PCI function related helpers as 'bonito_pci_*'.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 45f31512b25..06f5e683c81 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -664,7 +664,7 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
 }
 
-static void bonito_realize(PCIDevice *dev, Error **errp)
+static void bonito_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIBonitoState *s = PCI_BONITO(dev);
     SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
@@ -778,12 +778,12 @@ PCIBus *bonito_init(qemu_irq *pic)
     return phb->bus;
 }
 
-static void bonito_class_init(ObjectClass *klass, void *data)
+static void bonito_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->realize = bonito_realize;
+    k->realize = bonito_pci_realize;
     k->vendor_id = 0xdf53;
     k->device_id = 0x00d5;
     k->revision = 0x01;
@@ -797,11 +797,11 @@ static void bonito_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static const TypeInfo bonito_info = {
+static const TypeInfo bonito_pci_info = {
     .name          = TYPE_PCI_BONITO,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIBonitoState),
-    .class_init    = bonito_class_init,
+    .class_init    = bonito_pci_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },
@@ -825,7 +825,7 @@ static const TypeInfo bonito_host_info = {
 static void bonito_register_types(void)
 {
     type_register_static(&bonito_host_info);
-    type_register_static(&bonito_info);
+    type_register_static(&bonito_pci_info);
 }
 
 type_init(bonito_register_types)
-- 
2.26.2

[PATCH 12/18] hw/pci-host/bonito: Rename PCIBonitoState -> BonitoPciState

[Philippe Mathieu-Daudé]

As we use BonitoState for the whole device, rename
the PCI function device as BonitoPciState.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 06f5e683c81..1c88ccc6af8 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -200,7 +200,7 @@ FIELD(PCIMAP, 2G,               18, 1)
 
 typedef struct BonitoState BonitoState;
 
-struct PCIBonitoState {
+struct BonitoPciState {
     PCIDevice dev;
 
     BonitoState *pcihost;
@@ -229,12 +229,12 @@ struct PCIBonitoState {
     MemoryRegion bonito_localio;
 
 };
-typedef struct PCIBonitoState PCIBonitoState;
+typedef struct BonitoPciState BonitoPciState;
 
 struct BonitoState {
     PCIHostState parent_obj;
     qemu_irq *pic;
-    PCIBonitoState *pci_dev;
+    BonitoPciState *pci_dev;
     MemoryRegion pci_mem;
     MemoryRegion pcimem_lo_alias[3];
 };
@@ -243,9 +243,9 @@ struct BonitoState {
 OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
 
 #define TYPE_PCI_BONITO "Bonito"
-OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
+OBJECT_DECLARE_SIMPLE_TYPE(BonitoPciState, PCI_BONITO)
 
-static void bonito_remap(PCIBonitoState *s)
+static void bonito_remap(BonitoPciState *s)
 {
     static const char *const region_name[3] = {
         "pci.lomem0", "pci.lomem1", "pci.lomem2"
@@ -273,7 +273,7 @@ static void bonito_remap(PCIBonitoState *s)
 static void bonito_northbridge_write(void *opaque, hwaddr addr,
                           uint64_t val, unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     uint32_t saddr;
 
     saddr = addr >> 2;
@@ -338,7 +338,7 @@ static void bonito_northbridge_write(void *opaque, hwaddr addr,
 static uint64_t bonito_northbridge_read(void *opaque, hwaddr addr,
                              unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     uint32_t saddr;
 
     saddr = addr >> 2;
@@ -365,7 +365,7 @@ static const MemoryRegionOps bonito_northbridge_ops = {
 static void bonito_pciconf_writel(void *opaque, hwaddr addr,
                                   uint64_t val, unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     PCIDevice *d = PCI_DEVICE(s);
 
     DPRINTF("bonito_pciconf_writel "TARGET_FMT_plx" val %lx\n", addr, val);
@@ -376,7 +376,7 @@ static uint64_t bonito_pciconf_readl(void *opaque, hwaddr addr,
                                      unsigned size)
 {
 
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     PCIDevice *d = PCI_DEVICE(s);
 
     DPRINTF("bonito_pciconf_readl "TARGET_FMT_plx"\n", addr);
@@ -399,7 +399,7 @@ static uint64_t bonito_ldma_readl(void *opaque, hwaddr addr,
                                   unsigned size)
 {
     uint32_t val;
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
 
     if (addr >= sizeof(s->bonldma)) {
         return 0;
@@ -413,7 +413,7 @@ static uint64_t bonito_ldma_readl(void *opaque, hwaddr addr,
 static void bonito_ldma_writel(void *opaque, hwaddr addr,
                                uint64_t val, unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
 
     if (addr >= sizeof(s->bonldma)) {
         return;
@@ -436,7 +436,7 @@ static uint64_t bonito_cop_readl(void *opaque, hwaddr addr,
                                  unsigned size)
 {
     uint32_t val;
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
 
     if (addr >= sizeof(s->boncop)) {
         return 0;
@@ -450,7 +450,7 @@ static uint64_t bonito_cop_readl(void *opaque, hwaddr addr,
 static void bonito_cop_writel(void *opaque, hwaddr addr,
                               uint64_t val, unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
 
     if (addr >= sizeof(s->boncop)) {
         return;
@@ -471,7 +471,7 @@ static const MemoryRegionOps bonito_cop_ops = {
 
 static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr addr)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
     uint32_t cfgaddr;
     uint32_t idsel;
@@ -509,7 +509,7 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr addr)
 static void bonito_spciconf_write(void *opaque, hwaddr addr, uint64_t val,
                                   unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     PCIDevice *d = PCI_DEVICE(s);
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
     uint32_t pciaddr;
@@ -536,7 +536,7 @@ static void bonito_spciconf_write(void *opaque, hwaddr addr, uint64_t val,
 
 static uint64_t bonito_spciconf_read(void *opaque, hwaddr addr, unsigned size)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     PCIDevice *d = PCI_DEVICE(s);
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
     uint32_t pciaddr;
@@ -578,7 +578,7 @@ static void bonito_host_set_irq(void *opaque, int irq_num, int level)
 {
     BonitoState *s = opaque;
     qemu_irq *pic = s->pic;
-    PCIBonitoState *bonito_state = s->pci_dev;
+    BonitoPciState *bonito_state = s->pci_dev;
     int internal_irq = irq_num - BONITO_IRQ_BASE;
 
     if (bonito_state->regs[BONITO_INTEDGE] & (1 << internal_irq)) {
@@ -615,7 +615,7 @@ static int bonito_host_map_irq(PCIDevice *pci_dev, int irq_num)
 
 static void bonito_reset(void *opaque)
 {
-    PCIBonitoState *s = opaque;
+    BonitoPciState *s = opaque;
     uint32_t val = 0;
 
     /* set the default value of north bridge registers */
@@ -645,7 +645,7 @@ static const VMStateDescription vmstate_bonito = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PCIBonitoState),
+        VMSTATE_PCI_DEVICE(dev, BonitoPciState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -666,7 +666,7 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
 
 static void bonito_pci_realize(PCIDevice *dev, Error **errp)
 {
-    PCIBonitoState *s = PCI_BONITO(dev);
+    BonitoPciState *s = PCI_BONITO(dev);
     SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
@@ -760,7 +760,7 @@ PCIBus *bonito_init(qemu_irq *pic)
     DeviceState *dev;
     BonitoState *pcihost;
     PCIHostState *phb;
-    PCIBonitoState *s;
+    BonitoPciState *s;
     PCIDevice *d;
 
     dev = qdev_new(TYPE_BONITO_PCI_HOST_BRIDGE);
@@ -800,7 +800,7 @@ static void bonito_pci_class_init(ObjectClass *klass, void *data)
 static const TypeInfo bonito_pci_info = {
     .name          = TYPE_PCI_BONITO,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIBonitoState),
+    .instance_size = sizeof(BonitoPciState),
     .class_init    = bonito_pci_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-- 
2.26.2

[PATCH 13/18] hw/pci-host/bonito: Create PCI regions in bonito_host_realize()

[Philippe Mathieu-Daudé]

The PCI regions belong to the 'host' device, not the PCI function.
Move the PCI regions creation there.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 1c88ccc6af8..4dcaa2fa8bb 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -654,6 +654,7 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
+    MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
 
     memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
     phb->bus = pci_register_root_bus(dev, "pci",
@@ -662,6 +663,14 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
                                      PCI_DEVFN(5, 0), 32, TYPE_PCI_BUS);
 
     create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
+
+    memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
+                             &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
+    memory_region_add_subregion(get_system_memory(),
+                                BONITO_PCIHI_BASE, pcimem_alias);
+    create_unimplemented_device("PCI_2",
+                                (hwaddr)BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE,
+                                2 * GiB);
 }
 
 static void bonito_pci_realize(PCIDevice *dev, Error **errp)
@@ -669,8 +678,6 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     BonitoPciState *s = PCI_BONITO(dev);
     SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
-    BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
-    MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
 
     assert(!target_words_bigendian()); /* FIXME not supported */
 
@@ -732,14 +739,6 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     create_unimplemented_device("IOCS[3]", BONITO_DEV_BASE + 3 * 256 * KiB,
                                 256 * KiB);
 
-    memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
-                             &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
-    memory_region_add_subregion(get_system_memory(),
-                                BONITO_PCIHI_BASE, pcimem_alias);
-    create_unimplemented_device("PCI_2",
-                                (hwaddr)BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE,
-                                2 * GiB);
-
     /* set the default value of north bridge pci config */
     pci_set_word(dev->config + PCI_COMMAND, 0x0000);
     pci_set_word(dev->config + PCI_STATUS, 0x0000);
-- 
2.26.2

[RFC PATCH 14/18] hw/pci-host/bonito: Simplify using pci_host_conf_le_ops MemoryRegionOps

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 35 +----------------------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 4dcaa2fa8bb..c09d54cca82 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -362,39 +362,6 @@ static const MemoryRegionOps bonito_northbridge_ops = {
     },
 };
 
-static void bonito_pciconf_writel(void *opaque, hwaddr addr,
-                                  uint64_t val, unsigned size)
-{
-    BonitoPciState *s = opaque;
-    PCIDevice *d = PCI_DEVICE(s);
-
-    DPRINTF("bonito_pciconf_writel "TARGET_FMT_plx" val %lx\n", addr, val);
-    d->config_write(d, addr, val, 4);
-}
-
-static uint64_t bonito_pciconf_readl(void *opaque, hwaddr addr,
-                                     unsigned size)
-{
-
-    BonitoPciState *s = opaque;
-    PCIDevice *d = PCI_DEVICE(s);
-
-    DPRINTF("bonito_pciconf_readl "TARGET_FMT_plx"\n", addr);
-    return d->config_read(d, addr, 4);
-}
-
-/* north bridge PCI configure space. 0x1fe0 0000 - 0x1fe0 00ff */
-
-static const MemoryRegionOps bonito_pciconf_ops = {
-    .read = bonito_pciconf_readl,
-    .write = bonito_pciconf_writel,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-};
-
 static uint64_t bonito_ldma_readl(void *opaque, hwaddr addr,
                                   unsigned size)
 {
@@ -694,7 +661,7 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     sysbus_mmio_map(sysbus, 0, BONITO_INTERNAL_REG_BASE);
 
     /* set the north bridge pci configure  mapping */
-    memory_region_init_io(&phb->conf_mem, OBJECT(s), &bonito_pciconf_ops, s,
+    memory_region_init_io(&phb->conf_mem, OBJECT(s), &pci_host_conf_le_ops, s,
                           "north-bridge-pci-config", BONITO_PCICONFIG_SIZE);
     sysbus_init_mmio(sysbus, &phb->conf_mem);
     sysbus_mmio_map(sysbus, 1, BONITO_PCICONFIG_BASE);
-- 
2.26.2

[PATCH 15/18] hw/pci-host/bonito: Map north bridge region in bonito_host_realize()

[Philippe Mathieu-Daudé]

The 'north bridge' is not part of the PCI function,
so create and map it in bonito_host_realize().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index c09d54cca82..099bb737c77 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -638,6 +638,13 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("PCI_2",
                                 (hwaddr)BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE,
                                 2 * GiB);
+
+    /* set the north bridge pci configure mapping */
+    /* north bridge PCI configure space. 0x1fe0 0000 - 0x1fe0 00ff */
+    memory_region_init_io(&phb->conf_mem, OBJECT(phb), &pci_host_conf_le_ops, bs,
+                          "north-bridge-pci-config", BONITO_PCICONFIG_SIZE);
+    memory_region_add_subregion(get_system_memory(), BONITO_PCICONFIG_BASE,
+                                &phb->conf_mem);
 }
 
 static void bonito_pci_realize(PCIDevice *dev, Error **errp)
@@ -660,30 +667,24 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     sysbus_init_mmio(sysbus, &s->iomem);
     sysbus_mmio_map(sysbus, 0, BONITO_INTERNAL_REG_BASE);
 
-    /* set the north bridge pci configure  mapping */
-    memory_region_init_io(&phb->conf_mem, OBJECT(s), &pci_host_conf_le_ops, s,
-                          "north-bridge-pci-config", BONITO_PCICONFIG_SIZE);
-    sysbus_init_mmio(sysbus, &phb->conf_mem);
-    sysbus_mmio_map(sysbus, 1, BONITO_PCICONFIG_BASE);
-
     /* set the south bridge pci configure  mapping */
     memory_region_init_io(&phb->data_mem, OBJECT(s), &bonito_spciconf_ops, s,
                           "south-bridge-pci-config", BONITO_SPCICONFIG_SIZE);
     sysbus_init_mmio(sysbus, &phb->data_mem);
-    sysbus_mmio_map(sysbus, 2, BONITO_SPCICONFIG_BASE);
+    sysbus_mmio_map(sysbus, 1, BONITO_SPCICONFIG_BASE);
 
     create_unimplemented_device("bonito", BONITO_REG_BASE, BONITO_REG_SIZE);
 
     memory_region_init_io(&s->iomem_ldma, OBJECT(s), &bonito_ldma_ops, s,
                           "ldma", 0x100);
     sysbus_init_mmio(sysbus, &s->iomem_ldma);
-    sysbus_mmio_map(sysbus, 3, 0x1fe00200);
+    sysbus_mmio_map(sysbus, 2, 0x1fe00200);
 
     /* PCI copier */
     memory_region_init_io(&s->iomem_cop, OBJECT(s), &bonito_cop_ops, s,
                           "cop", 0x100);
     sysbus_init_mmio(sysbus, &s->iomem_cop);
-    sysbus_mmio_map(sysbus, 4, 0x1fe00300);
+    sysbus_mmio_map(sysbus, 3, 0x1fe00300);
 
     create_unimplemented_device("ROMCS", BONITO_FLASH_BASE, 60 * MiB);
 
@@ -691,14 +692,14 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     memory_region_init_alias(&s->bonito_pciio, OBJECT(s), "isa_mmio",
                              get_system_io(), 0, BONITO_PCIIO_SIZE);
     sysbus_init_mmio(sysbus, &s->bonito_pciio);
-    sysbus_mmio_map(sysbus, 5, BONITO_PCIIO_BASE);
+    sysbus_mmio_map(sysbus, 4, BONITO_PCIIO_BASE);
 
     /* add pci local io mapping */
 
     memory_region_init_alias(&s->bonito_localio, OBJECT(s), "IOCS[0]",
                              get_system_io(), 0, 256 * KiB);
     sysbus_init_mmio(sysbus, &s->bonito_localio);
-    sysbus_mmio_map(sysbus, 6, BONITO_DEV_BASE);
+    sysbus_mmio_map(sysbus, 5, BONITO_DEV_BASE);
     create_unimplemented_device("IOCS[1]", BONITO_DEV_BASE + 1 * 256 * KiB,
                                 256 * KiB);
     create_unimplemented_device("IOCS[2]", BONITO_DEV_BASE + 2 * 256 * KiB,
-- 
2.26.2

[PATCH 16/18] hw/pci-host/bonito: Create TYPE_PCI_BONITO in bonito_host_realize()

[Philippe Mathieu-Daudé]

As it doesn't make sens to create the TYPE_BONITO_PCI_HOST_BRIDGE
PCI function device without its host side, create it in the host
realize(). This will allow to inline bonito_init() in few commits.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 099bb737c77..5a8a85ae443 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -622,6 +622,7 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
     MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
+    PCIDevice *pci_dev;
 
     memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
     phb->bus = pci_register_root_bus(dev, "pci",
@@ -645,6 +646,11 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
                           "north-bridge-pci-config", BONITO_PCICONFIG_SIZE);
     memory_region_add_subregion(get_system_memory(), BONITO_PCICONFIG_BASE,
                                 &phb->conf_mem);
+
+    pci_dev = pci_new(PCI_DEVFN(0, 0), TYPE_PCI_BONITO);
+    PCI_BONITO(pci_dev)->pcihost = bs;
+    bs->pci_dev = PCI_BONITO(pci_dev);
+    pci_realize_and_unref(pci_dev, phb->bus, &error_fatal);
 }
 
 static void bonito_pci_realize(PCIDevice *dev, Error **errp)
@@ -727,8 +733,6 @@ PCIBus *bonito_init(qemu_irq *pic)
     DeviceState *dev;
     BonitoState *pcihost;
     PCIHostState *phb;
-    BonitoPciState *s;
-    PCIDevice *d;
 
     dev = qdev_new(TYPE_BONITO_PCI_HOST_BRIDGE);
     phb = PCI_HOST_BRIDGE(dev);
@@ -736,12 +740,6 @@ PCIBus *bonito_init(qemu_irq *pic)
     pcihost->pic = pic;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-    d = pci_new(PCI_DEVFN(0, 0), TYPE_PCI_BONITO);
-    s = PCI_BONITO(d);
-    s->pcihost = pcihost;
-    pcihost->pci_dev = s;
-    pci_realize_and_unref(d, phb->bus, &error_fatal);
-
     return phb->bus;
 }
 
-- 
2.26.2

[PATCH 17/18] hw/pci-host/bonito: Declare TYPE_BONITO_PCI_HOST_BRIDGE in include/

[Philippe Mathieu-Daudé]

Declare TYPE_BONITO_PCI_HOST_BRIDGE in the new "hw/pci-host/bonito.h"
header, so we can inline the bonito_init() call in the next commit.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/pci-host/bonito.h | 34 ++++++++++++++++++++++++++++++++++
 hw/pci-host/bonito.c         | 16 +---------------
 MAINTAINERS                  |  1 +
 3 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 include/hw/pci-host/bonito.h

diff --git a/include/hw/pci-host/bonito.h b/include/hw/pci-host/bonito.h
new file mode 100644
index 00000000000..d0264f34783
--- /dev/null
+++ b/include/hw/pci-host/bonito.h
@@ -0,0 +1,34 @@
+/*
+ * Algorithmics Bonito64 'north bridge' controller
+ *
+ * Copyright (c) 2008 yajin (yajin@vm-kernel.org)
+ * Copyright (c) 2010 Huacai Chen (zltjiangshi@gmail.com)
+ * Copyright (c) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef HW_PCI_HOST_BONITO_H
+#define HW_PCI_HOST_BONITO_H
+
+#include "exec/memory.h"
+#include "hw/pci/pci_host.h"
+#include "qom/object.h"
+
+typedef struct BonitoPciState BonitoPciState;
+
+#define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
+OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
+
+typedef struct BonitoState BonitoState;
+
+struct BonitoState {
+    /*< private >*/
+    PCIHostState parent_obj;
+    /*< public >*/
+    qemu_irq *pic;
+    BonitoPciState *pci_dev;
+    MemoryRegion pci_mem;
+    MemoryRegion pcimem_lo_alias[3];
+};
+
+#endif
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 5a8a85ae443..b959b353854 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -45,6 +45,7 @@
 #include "hw/pci/pci.h"
 #include "hw/irq.h"
 #include "hw/mips/mips.h"
+#include "hw/pci-host/bonito.h"
 #include "hw/pci/pci_host.h"
 #include "migration/vmstate.h"
 #include "sysemu/reset.h"
@@ -198,8 +199,6 @@ FIELD(PCIMAP, 2G,               18, 1)
     ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \
     (((funno) << 8) & 0x700) + (regno))
 
-typedef struct BonitoState BonitoState;
-
 struct BonitoPciState {
     PCIDevice dev;
 
@@ -227,20 +226,7 @@ struct BonitoPciState {
     MemoryRegion iomem_cop;
     MemoryRegion bonito_pciio;
     MemoryRegion bonito_localio;
-
 };
-typedef struct BonitoPciState BonitoPciState;
-
-struct BonitoState {
-    PCIHostState parent_obj;
-    qemu_irq *pic;
-    BonitoPciState *pci_dev;
-    MemoryRegion pci_mem;
-    MemoryRegion pcimem_lo_alias[3];
-};
-
-#define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
-OBJECT_DECLARE_SIMPLE_TYPE(BonitoState, BONITO_PCI_HOST_BRIDGE)
 
 #define TYPE_PCI_BONITO "Bonito"
 OBJECT_DECLARE_SIMPLE_TYPE(BonitoPciState, PCI_BONITO)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1e7c8f04885..2e22830efba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1153,6 +1153,7 @@ F: hw/mips/fuloong2e.c
 F: hw/isa/vt82c686.c
 F: hw/pci-host/bonito.c
 F: include/hw/isa/vt82c686.h
+F: include/hw/pci-host/bonito.h
 
 Loongson-3 virtual platforms
 M: Huacai Chen <chenhuacai@kernel.org>
-- 
2.26.2

[RFC PATCH 18/18] hw/mips/fuloong2e: Inline bonito_init()

[Philippe Mathieu-Daudé]

Remove bonito_init() by inlining it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
TBC next year.
---
 include/hw/mips/mips.h |  3 ---
 hw/mips/fuloong2e.c    |  7 ++++++-
 hw/pci-host/bonito.c   | 15 ---------------
 3 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 6c9c8805f3f..609a33881f6 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -12,9 +12,6 @@
 /* gt64xxx.c */
 PCIBus *gt64120_register(qemu_irq *pic);
 
-/* bonito.c */
-PCIBus *bonito_init(qemu_irq *pic);
-
 /* rc4030.c */
 typedef struct rc4030DMAState *rc4030_dma;
 void rc4030_dma_read(void *dma, uint8_t *buf, int len);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe5..19b3788e816 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "cpu.h"
 #include "hw/clock.h"
+#include "hw/pci-host/bonito.h"
 #include "hw/intc/i8259.h"
 #include "hw/dma/i8257.h"
 #include "hw/isa/superio.h"
@@ -303,6 +304,7 @@ static void mips_fuloong2e_init(MachineState *machine)
     MIPSCPU *cpu;
     CPUMIPSState *env;
     DeviceState *dev;
+    BonitoState *bonito;
 
     cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
     clock_set_hz(cpuclk, 533080000); /* ~533 MHz */
@@ -360,7 +362,10 @@ static void mips_fuloong2e_init(MachineState *machine)
     cpu_mips_clock_init(cpu);
 
     /* North bridge, Bonito --> IP2 */
-    pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
+    bonito = BONITO_PCI_HOST_BRIDGE(qdev_new(TYPE_BONITO_PCI_HOST_BRIDGE));
+    bonito->pic = (qemu_irq *)&(env->irq[2]); /* TODO link/property */
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(bonito), &error_fatal);
+    pci_bus = PCI_HOST_BRIDGE(bonito)->bus;
 
     /* South bridge -> IP5 */
     vt82c686b_southbridge_init(pci_bus, FULOONG2E_VIA_SLOT, env->irq[5],
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index b959b353854..28d385ff363 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,21 +714,6 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
     qemu_register_reset(bonito_reset, s);
 }
 
-PCIBus *bonito_init(qemu_irq *pic)
-{
-    DeviceState *dev;
-    BonitoState *pcihost;
-    PCIHostState *phb;
-
-    dev = qdev_new(TYPE_BONITO_PCI_HOST_BRIDGE);
-    phb = PCI_HOST_BRIDGE(dev);
-    pcihost = BONITO_PCI_HOST_BRIDGE(dev);
-    pcihost->pic = pic;
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-
-    return phb->bus;
-}
-
 static void bonito_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-- 
2.26.2

Re: [PATCH 00/18] hw/pci-host/bonito: Remap PCI "lo" regions when PCIMAP reg is modified

[Philippe Mathieu-Daudé]

On 12/31/20 11:48 PM, Philippe Mathieu-Daudé wrote:
> Patches I wrote while reviewing/testing Jiaxun's v3 [1] trying
> to understand the problem with "Fixup pci.lomem mapping" [2].
> 
> The issue should be fixed by patch #8 of this series:
> 'Remap PCI "lo" regions when PCIMAP reg is modified'.
> The rest are cleanups patches.
> 
> Happy new year,
> 
> Phil.
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769286.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg769294.html
> 
> Philippe Mathieu-Daudé (18):
>   hw/pci-host/bonito: Remove unused definitions
>   hw/pci-host/bonito: Display hexadecimal value with '0x' prefix
>   hw/pci-host/bonito: Use PCI_DEVFN() macro
>   hw/pci-host/bonito: Use pci_config_set_interrupt_pin()

Patches 2-4 applied to mips-next.