qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes
@ 2021-03-09 14:26 Philippe Mathieu-Daudé
  2021-03-09 14:26 ` [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize() Philippe Mathieu-Daudé
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

Trivial fixes extracted from another series which became too big,
so I prefer to send them in a previous step.

(This is a resend for Zoltan).

Philippe Mathieu-Daudé (6):
  hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize()
  hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers
  hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats
  hw/mips/gt64xxx: Rename trace events related to interrupt registers
  hw/mips/gt64xxx: Trace accesses to ISD registers
  hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole

 hw/mips/gt64xxx_pci.c | 67 +++++++++++++++++++++++++++----------------
 hw/mips/malta.c       |  7 -----
 hw/mips/trace-events  |  6 ++--
 3 files changed, 47 insertions(+), 33 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize()
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:36   ` BALATON Zoltan
  2021-03-09 14:26 ` [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

The ISD I/O region belongs to the TYPE_GT64120_PCI_HOST_BRIDGE,
so initialize it before it is realized, not after.
Rename the region as 'gt64120-isd' so it is clearer to realize
it belongs to the GT64120 in the memory tree view.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 588e6f99301..6eb73e77057 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1196,6 +1196,14 @@ static void gt64120_reset(DeviceState *dev)
     gt64120_pci_mapping(s);
 }
 
+static void gt64120_realize(DeviceState *dev, Error **errp)
+{
+    GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
+
+    memory_region_init_io(&s->ISD_mem, OBJECT(dev), &isd_mem_ops, s,
+                          "gt64120-isd", 0x1000);
+}
+
 PCIBus *gt64120_register(qemu_irq *pic)
 {
     GT64120State *d;
@@ -1214,8 +1222,6 @@ PCIBus *gt64120_register(qemu_irq *pic)
                                      get_system_io(),
                                      PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d,
-                          "isd-mem", 0x1000);
 
     pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
     return phb->bus;
@@ -1270,6 +1276,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->realize = gt64120_realize;
     dc->reset = gt64120_reset;
     dc->vmsd = &vmstate_gt64120;
 }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
  2021-03-09 14:26 ` [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize() Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:50   ` BALATON Zoltan
  2021-03-09 14:26 ` [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

The ISD MemoryRegion is implemented for 32-bit accesses.
Simplify it by setting the MemoryRegionOps::impl min/max
access size fields.

Since the region is registered with a size of 0x1000 bytes,
we can remove the hwaddr mask.

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

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 6eb73e77057..99b1690af19 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -385,13 +385,12 @@ static void gt64120_writel(void *opaque, hwaddr addr,
 {
     GT64120State *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    uint32_t saddr;
+    uint32_t saddr = addr >> 2;
 
     if (!(s->regs[GT_CPU] & 0x00001000)) {
         val = bswap32(val);
     }
 
-    saddr = (addr & 0xfff) >> 2;
     switch (saddr) {
 
     /* CPU Configuration */
@@ -695,9 +694,8 @@ static uint64_t gt64120_readl(void *opaque,
     GT64120State *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
     uint32_t val;
-    uint32_t saddr;
+    uint32_t saddr = addr >> 2;
 
-    saddr = (addr & 0xfff) >> 2;
     switch (saddr) {
 
     /* CPU Configuration */
@@ -976,6 +974,10 @@ static const MemoryRegionOps isd_mem_ops = {
     .read = gt64120_readl,
     .write = gt64120_writel,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
  2021-03-09 14:26 ` [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize() Philippe Mathieu-Daudé
  2021-03-09 14:26 ` [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:39   ` BALATON Zoltan
  2021-03-09 14:26 ` [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

Fix the following typos:
- GT_PCI1_CFGDATA is not a timer register but a PCI one,
- zero-padding flag is out of the format

Fixes: 641ca2bfcd5 ("hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 99b1690af19..8ff31380d74 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -463,7 +463,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Read-only registers, do nothing */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gt64120: Read-only register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -473,7 +473,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Read-only registers, do nothing */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gt64120: Read-only register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -515,7 +515,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Not implemented */
         qemu_log_mask(LOG_UNIMP,
                       "gt64120: Unimplemented device register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -528,7 +528,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Read-only registers, do nothing */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gt64120: Read-only register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -565,7 +565,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Not implemented */
         qemu_log_mask(LOG_UNIMP,
                       "gt64120: Unimplemented DMA register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -578,7 +578,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* Not implemented */
         qemu_log_mask(LOG_UNIMP,
                       "gt64120: Unimplemented timer register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
 
@@ -621,8 +621,8 @@ static void gt64120_writel(void *opaque, hwaddr addr,
     case GT_PCI1_CFGDATA:
         /* not implemented */
         qemu_log_mask(LOG_UNIMP,
-                      "gt64120: Unimplemented timer register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "gt64120: Unimplemented PCI register write "
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
     case GT_PCI0_CFGADDR:
@@ -682,7 +682,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gt64120: Illegal register write "
-                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
                       saddr << 2, size, size << 1, val);
         break;
     }
@@ -958,7 +958,7 @@ static uint64_t gt64120_readl(void *opaque,
         val = s->regs[saddr];
         qemu_log_mask(LOG_GUEST_ERROR,
                       "gt64120: Illegal register read "
-                      "reg:0x03%x size:%u value:0x%0*x\n",
+                      "reg:0x%03x size:%u value:0x%0*x\n",
                       saddr << 2, size, size << 1, val);
         break;
     }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-03-09 14:26 ` [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:41   ` BALATON Zoltan
  2021-03-09 14:26 ` [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

We want to trace all register accesses. First rename the current
gt64120_read / gt64120_write events with '_intreg' suffix, as they
are restricted to interrupt registers.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 16 ++++++++--------
 hw/mips/trace-events  |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 8ff31380d74..9a12d00d1e1 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -642,19 +642,19 @@ static void gt64120_writel(void *opaque, hwaddr addr,
         /* not really implemented */
         s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffffffe));
         s->regs[saddr] |= !!(s->regs[saddr] & 0xfffffffe);
-        trace_gt64120_write("INTRCAUSE", size, val);
+        trace_gt64120_write_intreg("INTRCAUSE", size, val);
         break;
     case GT_INTRMASK:
         s->regs[saddr] = val & 0x3c3ffffe;
-        trace_gt64120_write("INTRMASK", size, val);
+        trace_gt64120_write_intreg("INTRMASK", size, val);
         break;
     case GT_PCI0_ICMASK:
         s->regs[saddr] = val & 0x03fffffe;
-        trace_gt64120_write("ICMASK", size, val);
+        trace_gt64120_write_intreg("ICMASK", size, val);
         break;
     case GT_PCI0_SERR0MASK:
         s->regs[saddr] = val & 0x0000003f;
-        trace_gt64120_write("SERR0MASK", size, val);
+        trace_gt64120_write_intreg("SERR0MASK", size, val);
         break;
 
     /* Reserved when only PCI_0 is configured. */
@@ -929,19 +929,19 @@ static uint64_t gt64120_readl(void *opaque,
     /* Interrupts */
     case GT_INTRCAUSE:
         val = s->regs[saddr];
-        trace_gt64120_read("INTRCAUSE", size, val);
+        trace_gt64120_read_intreg("INTRCAUSE", size, val);
         break;
     case GT_INTRMASK:
         val = s->regs[saddr];
-        trace_gt64120_read("INTRMASK", size, val);
+        trace_gt64120_read_intreg("INTRMASK", size, val);
         break;
     case GT_PCI0_ICMASK:
         val = s->regs[saddr];
-        trace_gt64120_read("ICMASK", size, val);
+        trace_gt64120_read_intreg("ICMASK", size, val);
         break;
     case GT_PCI0_SERR0MASK:
         val = s->regs[saddr];
-        trace_gt64120_read("SERR0MASK", size, val);
+        trace_gt64120_read_intreg("SERR0MASK", size, val);
         break;
 
     /* Reserved when only PCI_0 is configured. */
diff --git a/hw/mips/trace-events b/hw/mips/trace-events
index 915139d9811..b7e934c3933 100644
--- a/hw/mips/trace-events
+++ b/hw/mips/trace-events
@@ -1,4 +1,4 @@
 # gt64xxx_pci.c
-gt64120_read(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
-gt64120_write(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
+gt64120_read_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
+gt64120_write_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
 gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-03-09 14:26 ` [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:42   ` BALATON Zoltan
  2021-03-09 15:47   ` BALATON Zoltan
  2021-03-09 14:26 ` [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole Philippe Mathieu-Daudé
  2021-03-11 23:24 ` [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
  6 siblings, 2 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

Trace all accesses to Internal Space Decode (ISD) registers.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 2 ++
 hw/mips/trace-events  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 9a12d00d1e1..43349d6837d 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -387,6 +387,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
     uint32_t saddr = addr >> 2;
 
+    trace_gt64120_write(addr, val);
     if (!(s->regs[GT_CPU] & 0x00001000)) {
         val = bswap32(val);
     }
@@ -966,6 +967,7 @@ static uint64_t gt64120_readl(void *opaque,
     if (!(s->regs[GT_CPU] & 0x00001000)) {
         val = bswap32(val);
     }
+    trace_gt64120_read(addr, val);
 
     return val;
 }
diff --git a/hw/mips/trace-events b/hw/mips/trace-events
index b7e934c3933..13ee731a488 100644
--- a/hw/mips/trace-events
+++ b/hw/mips/trace-events
@@ -1,4 +1,6 @@
 # gt64xxx_pci.c
+gt64120_read(uint64_t addr, uint64_t value) "gt64120 read 0x%03"PRIx64" value:0x%08" PRIx64
+gt64120_write(uint64_t addr, uint64_t value) "gt64120 write 0x%03"PRIx64" value:0x%08" PRIx64
 gt64120_read_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
 gt64120_write_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
 gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2021-03-09 14:26 ` [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers Philippe Mathieu-Daudé
@ 2021-03-09 14:26 ` Philippe Mathieu-Daudé
  2021-03-09 15:52   ` BALATON Zoltan
  2021-03-11 23:24 ` [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
  6 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 14:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Philippe Mathieu-Daudé, Aurelien Jarno

Per the comment in the Malta board, the [0x0000.0000-0x2000.0000]
range is decoded by the GT64120, so move the "empty_slot" there.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 8 ++++++++
 hw/mips/malta.c       | 7 -------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 43349d6837d..a3926e5cb8a 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -29,6 +29,7 @@
 #include "hw/mips/mips.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
+#include "hw/misc/empty_slot.h"
 #include "hw/southbridge/piix.h"
 #include "migration/vmstate.h"
 #include "hw/intc/i8259.h"
@@ -1206,6 +1207,13 @@ static void gt64120_realize(DeviceState *dev, Error **errp)
 
     memory_region_init_io(&s->ISD_mem, OBJECT(dev), &isd_mem_ops, s,
                           "gt64120-isd", 0x1000);
+
+    /*
+     * The whole address space decoded by the GT-64120A doesn't generate
+     * exception when accessing invalid memory. Create an empty slot to
+     * emulate this feature.
+     */
+    empty_slot_init("GT64120", 0, 0x20000000);
 }
 
 PCIBus *gt64120_register(qemu_irq *pic)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9afc0b427bf..b2469f8ee78 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -56,7 +56,6 @@
 #include "sysemu/runstate.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "hw/misc/empty_slot.h"
 #include "sysemu/kvm.h"
 #include "hw/semihosting/semihost.h"
 #include "hw/mips/cps.h"
@@ -1396,12 +1395,6 @@ void mips_malta_init(MachineState *machine)
 
     /* Northbridge */
     pci_bus = gt64120_register(s->i8259);
-    /*
-     * The whole address space decoded by the GT-64120A doesn't generate
-     * exception when accessing invalid memory. Create an empty slot to
-     * emulate this feature.
-     */
-    empty_slot_init("GT64120", 0, 0x20000000);
 
     /* Southbridge */
     dev = piix4_create(pci_bus, &isa_bus, &smbus);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize()
  2021-03-09 14:26 ` [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize() Philippe Mathieu-Daudé
@ 2021-03-09 15:36   ` BALATON Zoltan
  0 siblings, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 1870 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> The ISD I/O region belongs to the TYPE_GT64120_PCI_HOST_BRIDGE,
> so initialize it before it is realized, not after.
> Rename the region as 'gt64120-isd' so it is clearer to realize
> it belongs to the GT64120 in the memory tree view.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/mips/gt64xxx_pci.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 588e6f99301..6eb73e77057 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1196,6 +1196,14 @@ static void gt64120_reset(DeviceState *dev)
>     gt64120_pci_mapping(s);
> }
>
> +static void gt64120_realize(DeviceState *dev, Error **errp)
> +{
> +    GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
> +
> +    memory_region_init_io(&s->ISD_mem, OBJECT(dev), &isd_mem_ops, s,
> +                          "gt64120-isd", 0x1000);
> +}
> +
> PCIBus *gt64120_register(qemu_irq *pic)
> {
>     GT64120State *d;
> @@ -1214,8 +1222,6 @@ PCIBus *gt64120_register(qemu_irq *pic)
>                                      get_system_io(),
>                                      PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
>     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> -    memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d,
> -                          "isd-mem", 0x1000);
>
>     pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
>     return phb->bus;
> @@ -1270,6 +1276,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
>     DeviceClass *dc = DEVICE_CLASS(klass);
>
>     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->realize = gt64120_realize;
>     dc->reset = gt64120_reset;
>     dc->vmsd = &vmstate_gt64120;
> }
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats
  2021-03-09 14:26 ` [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats Philippe Mathieu-Daudé
@ 2021-03-09 15:39   ` BALATON Zoltan
  0 siblings, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:39 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]



On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:

> Fix the following typos:
> - GT_PCI1_CFGDATA is not a timer register but a PCI one,
> - zero-padding flag is out of the format
>
> Fixes: 641ca2bfcd5 ("hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/mips/gt64xxx_pci.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 99b1690af19..8ff31380d74 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -463,7 +463,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Read-only registers, do nothing */
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "gt64120: Read-only register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -473,7 +473,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Read-only registers, do nothing */
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "gt64120: Read-only register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -515,7 +515,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Not implemented */
>         qemu_log_mask(LOG_UNIMP,
>                       "gt64120: Unimplemented device register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -528,7 +528,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Read-only registers, do nothing */
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "gt64120: Read-only register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -565,7 +565,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Not implemented */
>         qemu_log_mask(LOG_UNIMP,
>                       "gt64120: Unimplemented DMA register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -578,7 +578,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* Not implemented */
>         qemu_log_mask(LOG_UNIMP,
>                       "gt64120: Unimplemented timer register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>
> @@ -621,8 +621,8 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>     case GT_PCI1_CFGDATA:
>         /* not implemented */
>         qemu_log_mask(LOG_UNIMP,
> -                      "gt64120: Unimplemented timer register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "gt64120: Unimplemented PCI register write "
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>     case GT_PCI0_CFGADDR:
> @@ -682,7 +682,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>     default:
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "gt64120: Illegal register write "
> -                      "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> +                      "reg:0x%03x size:%u value:0x%0*" PRIx64 "\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>     }
> @@ -958,7 +958,7 @@ static uint64_t gt64120_readl(void *opaque,
>         val = s->regs[saddr];
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "gt64120: Illegal register read "
> -                      "reg:0x03%x size:%u value:0x%0*x\n",
> +                      "reg:0x%03x size:%u value:0x%0*x\n",
>                       saddr << 2, size, size << 1, val);
>         break;
>     }
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers
  2021-03-09 14:26 ` [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers Philippe Mathieu-Daudé
@ 2021-03-09 15:41   ` BALATON Zoltan
  0 siblings, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 3439 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> We want to trace all register accesses. First rename the current
> gt64120_read / gt64120_write events with '_intreg' suffix, as they
> are restricted to interrupt registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/mips/gt64xxx_pci.c | 16 ++++++++--------
> hw/mips/trace-events  |  4 ++--
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 8ff31380d74..9a12d00d1e1 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -642,19 +642,19 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>         /* not really implemented */
>         s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffffffe));
>         s->regs[saddr] |= !!(s->regs[saddr] & 0xfffffffe);
> -        trace_gt64120_write("INTRCAUSE", size, val);
> +        trace_gt64120_write_intreg("INTRCAUSE", size, val);
>         break;
>     case GT_INTRMASK:
>         s->regs[saddr] = val & 0x3c3ffffe;
> -        trace_gt64120_write("INTRMASK", size, val);
> +        trace_gt64120_write_intreg("INTRMASK", size, val);
>         break;
>     case GT_PCI0_ICMASK:
>         s->regs[saddr] = val & 0x03fffffe;
> -        trace_gt64120_write("ICMASK", size, val);
> +        trace_gt64120_write_intreg("ICMASK", size, val);
>         break;
>     case GT_PCI0_SERR0MASK:
>         s->regs[saddr] = val & 0x0000003f;
> -        trace_gt64120_write("SERR0MASK", size, val);
> +        trace_gt64120_write_intreg("SERR0MASK", size, val);
>         break;
>
>     /* Reserved when only PCI_0 is configured. */
> @@ -929,19 +929,19 @@ static uint64_t gt64120_readl(void *opaque,
>     /* Interrupts */
>     case GT_INTRCAUSE:
>         val = s->regs[saddr];
> -        trace_gt64120_read("INTRCAUSE", size, val);
> +        trace_gt64120_read_intreg("INTRCAUSE", size, val);
>         break;
>     case GT_INTRMASK:
>         val = s->regs[saddr];
> -        trace_gt64120_read("INTRMASK", size, val);
> +        trace_gt64120_read_intreg("INTRMASK", size, val);
>         break;
>     case GT_PCI0_ICMASK:
>         val = s->regs[saddr];
> -        trace_gt64120_read("ICMASK", size, val);
> +        trace_gt64120_read_intreg("ICMASK", size, val);
>         break;
>     case GT_PCI0_SERR0MASK:
>         val = s->regs[saddr];
> -        trace_gt64120_read("SERR0MASK", size, val);
> +        trace_gt64120_read_intreg("SERR0MASK", size, val);
>         break;
>
>     /* Reserved when only PCI_0 is configured. */
> diff --git a/hw/mips/trace-events b/hw/mips/trace-events
> index 915139d9811..b7e934c3933 100644
> --- a/hw/mips/trace-events
> +++ b/hw/mips/trace-events
> @@ -1,4 +1,4 @@
> # gt64xxx_pci.c
> -gt64120_read(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
> -gt64120_write(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
> +gt64120_read_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
> +gt64120_write_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
> gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers
  2021-03-09 14:26 ` [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers Philippe Mathieu-Daudé
@ 2021-03-09 15:42   ` BALATON Zoltan
  2021-03-09 15:47   ` BALATON Zoltan
  1 sibling, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> Trace all accesses to Internal Space Decode (ISD) registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/mips/gt64xxx_pci.c | 2 ++
> hw/mips/trace-events  | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 9a12d00d1e1..43349d6837d 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -387,6 +387,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>     PCIHostState *phb = PCI_HOST_BRIDGE(s);
>     uint32_t saddr = addr >> 2;
>
> +    trace_gt64120_write(addr, val);
>     if (!(s->regs[GT_CPU] & 0x00001000)) {
>         val = bswap32(val);
>     }
> @@ -966,6 +967,7 @@ static uint64_t gt64120_readl(void *opaque,
>     if (!(s->regs[GT_CPU] & 0x00001000)) {
>         val = bswap32(val);
>     }
> +    trace_gt64120_read(addr, val);
>
>     return val;
> }
> diff --git a/hw/mips/trace-events b/hw/mips/trace-events
> index b7e934c3933..13ee731a488 100644
> --- a/hw/mips/trace-events
> +++ b/hw/mips/trace-events
> @@ -1,4 +1,6 @@
> # gt64xxx_pci.c
> +gt64120_read(uint64_t addr, uint64_t value) "gt64120 read 0x%03"PRIx64" value:0x%08" PRIx64
> +gt64120_write(uint64_t addr, uint64_t value) "gt64120 write 0x%03"PRIx64" value:0x%08" PRIx64
> gt64120_read_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
> gt64120_write_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
> gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers
  2021-03-09 14:26 ` [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers Philippe Mathieu-Daudé
  2021-03-09 15:42   ` BALATON Zoltan
@ 2021-03-09 15:47   ` BALATON Zoltan
  1 sibling, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> Trace all accesses to Internal Space Decode (ISD) registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/mips/gt64xxx_pci.c | 2 ++
> hw/mips/trace-events  | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 9a12d00d1e1..43349d6837d 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -387,6 +387,7 @@ static void gt64120_writel(void *opaque, hwaddr addr,
>     PCIHostState *phb = PCI_HOST_BRIDGE(s);
>     uint32_t saddr = addr >> 2;
>
> +    trace_gt64120_write(addr, val);
>     if (!(s->regs[GT_CPU] & 0x00001000)) {
>         val = bswap32(val);
>     }
> @@ -966,6 +967,7 @@ static uint64_t gt64120_readl(void *opaque,
>     if (!(s->regs[GT_CPU] & 0x00001000)) {
>         val = bswap32(val);
>     }
> +    trace_gt64120_read(addr, val);

There are a few other places where logs use saddr << 2 instead of addr so 
for consistency you might consider either changing those too or use saddr 
<< 2 here as well. (Reviewed-by still stands either way.)

Regards,
BALATON Zoltan

>
>     return val;
> }
> diff --git a/hw/mips/trace-events b/hw/mips/trace-events
> index b7e934c3933..13ee731a488 100644
> --- a/hw/mips/trace-events
> +++ b/hw/mips/trace-events
> @@ -1,4 +1,6 @@
> # gt64xxx_pci.c
> +gt64120_read(uint64_t addr, uint64_t value) "gt64120 read 0x%03"PRIx64" value:0x%08" PRIx64
> +gt64120_write(uint64_t addr, uint64_t value) "gt64120 write 0x%03"PRIx64" value:0x%08" PRIx64
> gt64120_read_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 read %s size:%u value:0x%08" PRIx64
> gt64120_write_intreg(const char *regname, unsigned size, uint64_t value) "gt64120 write %s size:%u value:0x%08" PRIx64
> gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers
  2021-03-09 14:26 ` [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers Philippe Mathieu-Daudé
@ 2021-03-09 15:50   ` BALATON Zoltan
  0 siblings, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> The ISD MemoryRegion is implemented for 32-bit accesses.
> Simplify it by setting the MemoryRegionOps::impl min/max
> access size fields.
>
> Since the region is registered with a size of 0x1000 bytes,
> we can remove the hwaddr mask.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/mips/gt64xxx_pci.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 6eb73e77057..99b1690af19 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -385,13 +385,12 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> {
>     GT64120State *s = opaque;
>     PCIHostState *phb = PCI_HOST_BRIDGE(s);
> -    uint32_t saddr;
> +    uint32_t saddr = addr >> 2;
>
>     if (!(s->regs[GT_CPU] & 0x00001000)) {
>         val = bswap32(val);
>     }
>
> -    saddr = (addr & 0xfff) >> 2;
>     switch (saddr) {
>
>     /* CPU Configuration */
> @@ -695,9 +694,8 @@ static uint64_t gt64120_readl(void *opaque,
>     GT64120State *s = opaque;
>     PCIHostState *phb = PCI_HOST_BRIDGE(s);
>     uint32_t val;
> -    uint32_t saddr;
> +    uint32_t saddr = addr >> 2;
>
> -    saddr = (addr & 0xfff) >> 2;
>     switch (saddr) {
>
>     /* CPU Configuration */
> @@ -976,6 +974,10 @@ static const MemoryRegionOps isd_mem_ops = {
>     .read = gt64120_readl,
>     .write = gt64120_writel,
>     .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> };
>
> static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole
  2021-03-09 14:26 ` [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole Philippe Mathieu-Daudé
@ 2021-03-09 15:52   ` BALATON Zoltan
  2021-03-09 17:14     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 16+ messages in thread
From: BALATON Zoltan @ 2021-03-09 15:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> Per the comment in the Malta board, the [0x0000.0000-0x2000.0000]
> range is decoded by the GT64120, so move the "empty_slot" there.

I don't know anything about it to be able to review but is this a feature 
of the GT64120 chip (in which case the change is correct) or the Malta 
board (in which case this might make the GT64120 model board specific that 
may not matter much as there's nothing else using it now).

Regards,
BALATON Zoltan

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/mips/gt64xxx_pci.c | 8 ++++++++
> hw/mips/malta.c       | 7 -------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 43349d6837d..a3926e5cb8a 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -29,6 +29,7 @@
> #include "hw/mips/mips.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_host.h"
> +#include "hw/misc/empty_slot.h"
> #include "hw/southbridge/piix.h"
> #include "migration/vmstate.h"
> #include "hw/intc/i8259.h"
> @@ -1206,6 +1207,13 @@ static void gt64120_realize(DeviceState *dev, Error **errp)
>
>     memory_region_init_io(&s->ISD_mem, OBJECT(dev), &isd_mem_ops, s,
>                           "gt64120-isd", 0x1000);
> +
> +    /*
> +     * The whole address space decoded by the GT-64120A doesn't generate
> +     * exception when accessing invalid memory. Create an empty slot to
> +     * emulate this feature.
> +     */
> +    empty_slot_init("GT64120", 0, 0x20000000);
> }
>
> PCIBus *gt64120_register(qemu_irq *pic)
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index 9afc0b427bf..b2469f8ee78 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -56,7 +56,6 @@
> #include "sysemu/runstate.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> -#include "hw/misc/empty_slot.h"
> #include "sysemu/kvm.h"
> #include "hw/semihosting/semihost.h"
> #include "hw/mips/cps.h"
> @@ -1396,12 +1395,6 @@ void mips_malta_init(MachineState *machine)
>
>     /* Northbridge */
>     pci_bus = gt64120_register(s->i8259);
> -    /*
> -     * The whole address space decoded by the GT-64120A doesn't generate
> -     * exception when accessing invalid memory. Create an empty slot to
> -     * emulate this feature.
> -     */
> -    empty_slot_init("GT64120", 0, 0x20000000);
>
>     /* Southbridge */
>     dev = piix4_create(pci_bus, &isa_bus, &smbus);
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole
  2021-03-09 15:52   ` BALATON Zoltan
@ 2021-03-09 17:14     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-09 17:14 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: Aleksandar Rikalo, qemu-devel, Aurelien Jarno

On 3/9/21 4:52 PM, BALATON Zoltan wrote:
> On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
>> Per the comment in the Malta board, the [0x0000.0000-0x2000.0000]
>> range is decoded by the GT64120, so move the "empty_slot" there.
> 
> I don't know anything about it to be able to review but is this a
> feature of the GT64120 chip (in which case the change is correct) or the
> Malta board (in which case this might make the GT64120 model board
> specific that may not matter much as there's nothing else using it now).

As this is board-specific, I'll improve the description. Let's ignore
this patch for now.

Thank you for the previous reviews :)

Phil.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes
  2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2021-03-09 14:26 ` [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole Philippe Mathieu-Daudé
@ 2021-03-11 23:24 ` Philippe Mathieu-Daudé
  6 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-11 23:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aleksandar Rikalo, Aurelien Jarno

On 3/9/21 3:26 PM, Philippe Mathieu-Daudé wrote:
> Trivial fixes extracted from another series which became too big,
> so I prefer to send them in a previous step.
> 
> (This is a resend for Zoltan).
> 
> Philippe Mathieu-Daudé (6):
>   hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize()
>   hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers
>   hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats
>   hw/mips/gt64xxx: Rename trace events related to interrupt registers
>   hw/mips/gt64xxx: Trace accesses to ISD registers
>   hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole
Thanks, patches 1-5 applied to mips-next.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-03-11 23:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09 14:26 [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé
2021-03-09 14:26 ` [PATCH RESEND 1/6] hw/mips/gt64xxx: Initialize ISD I/O memory region in DeviceRealize() Philippe Mathieu-Daudé
2021-03-09 15:36   ` BALATON Zoltan
2021-03-09 14:26 ` [PATCH RESEND 2/6] hw/mips/gt64xxx: Simplify ISD MemoryRegion read/write handlers Philippe Mathieu-Daudé
2021-03-09 15:50   ` BALATON Zoltan
2021-03-09 14:26 ` [PATCH RESEND 3/6] hw/mips/gt64xxx: Fix typos in qemu_log_mask() formats Philippe Mathieu-Daudé
2021-03-09 15:39   ` BALATON Zoltan
2021-03-09 14:26 ` [PATCH RESEND 4/6] hw/mips/gt64xxx: Rename trace events related to interrupt registers Philippe Mathieu-Daudé
2021-03-09 15:41   ` BALATON Zoltan
2021-03-09 14:26 ` [PATCH RESEND 5/6] hw/mips/gt64xxx: Trace accesses to ISD registers Philippe Mathieu-Daudé
2021-03-09 15:42   ` BALATON Zoltan
2021-03-09 15:47   ` BALATON Zoltan
2021-03-09 14:26 ` [PATCH RESEND 6/6] hw/mips/gt64xxx: Let the GT64120 manage the lower 512MiB hole Philippe Mathieu-Daudé
2021-03-09 15:52   ` BALATON Zoltan
2021-03-09 17:14     ` Philippe Mathieu-Daudé
2021-03-11 23:24 ` [PATCH RESEND 0/6] hw/mips/gt64120: Minor fixes Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).