All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements
@ 2021-09-23  9:12 Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
                   ` (19 more replies)
  0 siblings, 20 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

This patchset is the next set of changes required to boot MacOS on the q800 machine. The
main aim of these patches is to improve the Nubus support so that devices can be plugged
into the Nubus from the command line i.e.

    -device nubus-macfb[,slot=num][,romfile=decl.rom]

At the moment the only device that can be plugged into the Nubus is the macfb framebuffer
however with these changes it is possible to take a ROM from a real Nubus card and
attempt to use it in QEMU, and also allow for future interfaces such as virtio.

Patches 1 to 6 move the logic which manages bus addresses from the NubusDevice into
the NubusBus itself, including the introduction of a bitmap to manage available
slots on the bus.

Patches 7 and 8 change the handling for unassigned (empty) slots to generate a bus
fault and add trace events to allow logging of empty slot accesses during Nubus
enumeration.

Patches 9 to 11 remove the existing stubs for generating the format block (the epilogue
of the Nubus device embedded ROM consisting of metadata and a checksum) and replace them
with a romfile device property to allow the entire Nubus ROM to be loaded from a file
into the ROM area, similar to a PCI option ROM.

Patch 12 moves the Nubus into its own separate address space whilst patches 13 to 17
update the NubusBridge (and MacNubusBridge) devices to allow machines to map the
required slots from the Nubus address space using sysbus_mmio_map().

Finally patches 18 to 20 add support for Nubus IRQs and wire them up appropriately for
the q800 machine through VIA2, which is required for the next set of macfb updates.

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


v5:
- Rebase onto master
- Add R-B tags from Laurent
- Introduce NUBUS_FIRST_SLOT/NUBUS_LAST_SLOT and MAC_NUBUS_FIRST_SLOT/MAC_NUBUS_LAST_SLOT
  and fix up NUBUS_SUPER_SLOT_NB/NUBUS_SLOT_NB in patch 4
- Fix super slot offset calculation in patch 4
- Squash original patch 3 ("nubus-device: add device slot parameter") into patch 4
  ("nubus: use bitmap to manage available slots")
- Add new patch 1 ("nubus: add comment indicating reference documents") containing
  documentation references
- Drop "nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);" from nubus_init() in patch 17
  
v4:
- Rebase onto master
- Pass &error_abort to memory_region_init_rom() in patch 11
- Change warn_error() to error_setg() and tweak message in patch 11

v3:
- Rebase onto master
- Add Phil's R-B for patch 7
- Move NUBUS_FIRST_SLOT/NUBUS_LAST_SLOT check to end of nubus_device_realize() in patch 4
- Use BIT() macro in patches 4 and 20

v2:
- Rebase onto master
- Tweak the cover letter by adding the optional slot parameter in the -device example
- Add R-B tags from Phil
- Document the increase in max_access_size in patch 7
- Change the maximum declaration ROM size to 128KiB using (128 * KiB) in patch 11
- use MAKE_64BIT_MASK() in patches 4 and 16


Mark Cave-Ayland (20):
  nubus: add comment indicating reference documents
  nubus-device: rename slot_nb variable to slot
  nubus-device: expose separate super slot memory region
  nubus: use bitmap to manage available slots
  nubus: move slot bitmap checks from NubusDevice realize() to BusClass
    check_address()
  nubus: implement BusClass get_dev_path()
  nubus: add trace-events for empty slot accesses
  nubus: generate bus error when attempting to access empty slots
  macfb: don't register declaration ROM
  nubus-device: remove nubus_register_rom() and
    nubus_register_format_block()
  nubus-device: add romfile property for loading declaration ROMs
  nubus: move nubus to its own 32-bit address space
  nubus-bridge: introduce separate NubusBridge structure
  mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  nubus: move NubusBus from mac-nubus-bridge to nubus-bridge
  nubus-bridge: embed the NubusBus object directly within nubus-bridge
  nubus-bridge: make slot_available_mask a qdev property
  nubus: add support for slot IRQs
  q800: wire up nubus IRQs
  q800: configure nubus available slots for Quadra 800

 hw/display/macfb.c                  |   6 -
 hw/m68k/q800.c                      |  26 +++-
 hw/nubus/mac-nubus-bridge.c         |  34 ++++-
 hw/nubus/nubus-bridge.c             |  23 ++-
 hw/nubus/nubus-bus.c                | 121 ++++++++++++---
 hw/nubus/nubus-device.c             | 227 ++++++++--------------------
 hw/nubus/trace-events               |   7 +
 hw/nubus/trace.h                    |   1 +
 include/hw/nubus/mac-nubus-bridge.h |  13 +-
 include/hw/nubus/nubus.h            |  49 +++---
 meson.build                         |   1 +
 11 files changed, 279 insertions(+), 229 deletions(-)
 create mode 100644 hw/nubus/trace-events
 create mode 100644 hw/nubus/trace.h

-- 
2.20.1



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

* [PATCH v5 01/20] nubus: add comment indicating reference documents
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:35   ` Philippe Mathieu-Daudé
  2021-09-23 10:01   ` Laurent Vivier
  2021-09-23  9:12 ` [PATCH v5 02/20] nubus-device: rename slot_nb variable to slot Mark Cave-Ayland
                   ` (18 subsequent siblings)
  19 siblings, 2 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/nubus/nubus-bus.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index 5c13452308..f4410803ff 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -8,6 +8,14 @@
  *
  */
 
+/*
+ * References:
+ *   Nubus Specification (TI)
+ *     http://www.bitsavers.org/pdf/ti/nubus/2242825-0001_NuBus_Spec1983.pdf
+ *
+ *   Designing Cards and Drivers for the Macintosh Family (Apple)
+ */
+
 #include "qemu/osdep.h"
 #include "hw/nubus/nubus.h"
 #include "qapi/error.h"
-- 
2.20.1



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

* [PATCH v5 02/20] nubus-device: rename slot_nb variable to slot
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 03/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

This is in preparation for creating a qdev property of the same name.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-device.c  | 14 +++++++-------
 include/hw/nubus/nubus.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index ffe78a8823..be01269563 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -87,7 +87,7 @@ static void nubus_register_format_block(NubusDevice *dev)
     char *fblock_name;
 
     fblock_name = g_strdup_printf("nubus-slot-%d-format-block",
-                                  dev->slot_nb);
+                                  dev->slot);
 
     hwaddr fblock_offset = memory_region_size(&dev->slot_mem) - FBLOCK_SIZE;
     memory_region_init_io(&dev->fblock_io, NULL, &nubus_format_block_ops,
@@ -142,7 +142,7 @@ void nubus_register_rom(NubusDevice *dev, const uint8_t *rom, uint32_t size,
     /* ROM */
 
     dev->rom = rom;
-    rom_name = g_strdup_printf("nubus-slot-%d-rom", dev->slot_nb);
+    rom_name = g_strdup_printf("nubus-slot-%d-rom", dev->slot);
     memory_region_init_io(&dev->rom_io, NULL, &mac_nubus_rom_ops,
                           dev, rom_name, size);
     memory_region_set_readonly(&dev->rom_io, true);
@@ -167,12 +167,12 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    nd->slot_nb = nubus->current_slot++;
-    name = g_strdup_printf("nubus-slot-%d", nd->slot_nb);
+    nd->slot = nubus->current_slot++;
+    name = g_strdup_printf("nubus-slot-%d", nd->slot);
 
-    if (nd->slot_nb < NUBUS_FIRST_SLOT) {
+    if (nd->slot < NUBUS_FIRST_SLOT) {
         /* Super */
-        slot_offset = (nd->slot_nb - 6) * NUBUS_SUPER_SLOT_SIZE;
+        slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
 
         memory_region_init(&nd->slot_mem, OBJECT(dev), name,
                            NUBUS_SUPER_SLOT_SIZE);
@@ -180,7 +180,7 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
                                     &nd->slot_mem);
     } else {
         /* Normal */
-        slot_offset = nd->slot_nb * NUBUS_SLOT_SIZE;
+        slot_offset = nd->slot * NUBUS_SLOT_SIZE;
 
         memory_region_init(&nd->slot_mem, OBJECT(dev), name, NUBUS_SLOT_SIZE);
         memory_region_add_subregion(&nubus->slot_io, slot_offset,
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index e2b5cf260b..424309dd73 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -42,7 +42,7 @@ struct NubusBus {
 struct NubusDevice {
     DeviceState qdev;
 
-    int slot_nb;
+    int slot;
     MemoryRegion slot_mem;
 
     /* Format Block */
-- 
2.20.1



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

* [PATCH v5 03/20] nubus-device: expose separate super slot memory region
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 02/20] nubus-device: rename slot_nb variable to slot Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23 10:04   ` Laurent Vivier
  2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

According to "Designing Cards and Drivers for the Macintosh Family" each physical
nubus slot can access 2 separate address ranges: a super slot memory region which
is 256MB and a standard slot memory region which is 16MB.

Currently a Nubus device uses the physical slot number to determine whether it is
using a standard slot memory region or a super slot memory region rather than
exposing both memory regions for use as required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/nubus/nubus-device.c  | 36 ++++++++++++++++++------------------
 include/hw/nubus/nubus.h |  1 +
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index be01269563..4e23df1280 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -168,26 +168,26 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     }
 
     nd->slot = nubus->current_slot++;
-    name = g_strdup_printf("nubus-slot-%d", nd->slot);
-
-    if (nd->slot < NUBUS_FIRST_SLOT) {
-        /* Super */
-        slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
-
-        memory_region_init(&nd->slot_mem, OBJECT(dev), name,
-                           NUBUS_SUPER_SLOT_SIZE);
-        memory_region_add_subregion(&nubus->super_slot_io, slot_offset,
-                                    &nd->slot_mem);
-    } else {
-        /* Normal */
-        slot_offset = nd->slot * NUBUS_SLOT_SIZE;
-
-        memory_region_init(&nd->slot_mem, OBJECT(dev), name, NUBUS_SLOT_SIZE);
-        memory_region_add_subregion(&nubus->slot_io, slot_offset,
-                                    &nd->slot_mem);
-    }
 
+    /* Super */
+    slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
+
+    name = g_strdup_printf("nubus-super-slot-%x", nd->slot);
+    memory_region_init(&nd->super_slot_mem, OBJECT(dev), name,
+                       NUBUS_SUPER_SLOT_SIZE);
+    memory_region_add_subregion(&nubus->super_slot_io, slot_offset,
+                                &nd->super_slot_mem);
+    g_free(name);
+
+    /* Normal */
+    slot_offset = nd->slot * NUBUS_SLOT_SIZE;
+
+    name = g_strdup_printf("nubus-slot-%x", nd->slot);
+    memory_region_init(&nd->slot_mem, OBJECT(dev), name, NUBUS_SLOT_SIZE);
+    memory_region_add_subregion(&nubus->slot_io, slot_offset,
+                                &nd->slot_mem);
     g_free(name);
+
     nubus_register_format_block(nd);
 }
 
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 424309dd73..89b0976aaa 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -43,6 +43,7 @@ struct NubusDevice {
     DeviceState qdev;
 
     int slot;
+    MemoryRegion super_slot_mem;
     MemoryRegion slot_mem;
 
     /* Format Block */
-- 
2.20.1



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

* [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 03/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:42   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2021-09-23  9:12 ` [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
                   ` (15 subsequent siblings)
  19 siblings, 3 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Convert nubus_device_realize() to use a bitmap to manage available slots to allow
for future Nubus devices to be plugged into arbitrary slots from the command line
using a new qdev "slot" parameter for nubus devices.

Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
machines as documented in "Desigining Cards and Drivers for the Macintosh Family".

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/nubus/mac-nubus-bridge.c         |  4 ++++
 hw/nubus/nubus-bus.c                |  5 +++--
 hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
 include/hw/nubus/mac-nubus-bridge.h |  4 ++++
 include/hw/nubus/nubus.h            | 13 ++++++------
 5 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 7c329300b8..3f075789e9 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -18,6 +18,10 @@ static void mac_nubus_bridge_init(Object *obj)
 
     s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
 
+    /* Macintosh only has slots 0x9 to 0xe available */
+    s->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
+                                                  MAC_NUBUS_SLOT_NB);
+
     sysbus_init_mmio(sbd, &s->bus->super_slot_io);
     sysbus_init_mmio(sbd, &s->bus->slot_io);
 }
diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index f4410803ff..3cd7534864 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -86,13 +86,14 @@ static void nubus_init(Object *obj)
 
     memory_region_init_io(&nubus->super_slot_io, obj, &nubus_super_slot_ops,
                           nubus, "nubus-super-slots",
-                          NUBUS_SUPER_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
+                          (NUBUS_SUPER_SLOT_NB + 1) * NUBUS_SUPER_SLOT_SIZE);
 
     memory_region_init_io(&nubus->slot_io, obj, &nubus_slot_ops,
                           nubus, "nubus-slots",
                           NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
 
-    nubus->current_slot = NUBUS_FIRST_SLOT;
+    nubus->slot_available_mask = MAKE_64BIT_MASK(NUBUS_FIRST_SLOT,
+                                                 NUBUS_SLOT_NB);
 }
 
 static void nubus_class_init(ObjectClass *oc, void *data)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 4e23df1280..562650a05b 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -160,14 +160,28 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     NubusDevice *nd = NUBUS_DEVICE(dev);
     char *name;
     hwaddr slot_offset;
-
-    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
-            nubus->current_slot > NUBUS_LAST_SLOT) {
-        error_setg(errp, "Cannot register nubus card, not enough slots");
-        return;
+    uint16_t s;
+
+    if (nd->slot == -1) {
+        /* No slot specified, find first available free slot */
+        s = ctz32(nubus->slot_available_mask);
+        if (s != 32) {
+            nd->slot = s;
+        } else {
+            error_setg(errp, "Cannot register nubus card, no free slot "
+                             "available");
+            return;
+        }
+    } else {
+        /* Slot specified, make sure the slot is available */
+        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
+            error_setg(errp, "Cannot register nubus card, slot %d is "
+                             "unavailable or already occupied", nd->slot);
+            return;
+        }
     }
 
-    nd->slot = nubus->current_slot++;
+    nubus->slot_available_mask &= ~BIT(nd->slot);
 
     /* Super */
     slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
@@ -191,12 +205,18 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     nubus_register_format_block(nd);
 }
 
+static Property nubus_device_properties[] = {
+    DEFINE_PROP_INT32("slot", NubusDevice, slot, -1),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void nubus_device_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = nubus_device_realize;
     dc->bus_type = TYPE_NUBUS_BUS;
+    device_class_set_props(dc, nubus_device_properties);
 }
 
 static const TypeInfo nubus_device_type_info = {
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index 36aa098dd4..118d67267d 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -12,6 +12,10 @@
 #include "hw/nubus/nubus.h"
 #include "qom/object.h"
 
+#define MAC_NUBUS_FIRST_SLOT 0x9
+#define MAC_NUBUS_LAST_SLOT  0xe
+#define MAC_NUBUS_SLOT_NB    (MAC_NUBUS_LAST_SLOT - MAC_NUBUS_FIRST_SLOT + 1)
+
 #define TYPE_MAC_NUBUS_BRIDGE "mac-nubus-bridge"
 OBJECT_DECLARE_SIMPLE_TYPE(MacNubusState, MAC_NUBUS_BRIDGE)
 
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 89b0976aaa..988e4a2361 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -14,13 +14,12 @@
 #include "qom/object.h"
 
 #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
-#define NUBUS_SUPER_SLOT_NB   0x9
+#define NUBUS_SUPER_SLOT_NB   0xe
 
 #define NUBUS_SLOT_SIZE       0x01000000
-#define NUBUS_SLOT_NB         0xF
-
-#define NUBUS_FIRST_SLOT      0x9
-#define NUBUS_LAST_SLOT       0xF
+#define NUBUS_FIRST_SLOT      0x0
+#define NUBUS_LAST_SLOT       0xf
+#define NUBUS_SLOT_NB         (NUBUS_LAST_SLOT - NUBUS_FIRST_SLOT + 1)
 
 #define TYPE_NUBUS_DEVICE "nubus-device"
 OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
@@ -36,13 +35,13 @@ struct NubusBus {
     MemoryRegion super_slot_io;
     MemoryRegion slot_io;
 
-    int current_slot;
+    uint32_t slot_available_mask;
 };
 
 struct NubusDevice {
     DeviceState qdev;
 
-    int slot;
+    int32_t slot;
     MemoryRegion super_slot_mem;
     MemoryRegion slot_mem;
 
-- 
2.20.1



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

* [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:45   ` Philippe Mathieu-Daudé
  2021-09-23 10:30   ` Laurent Vivier
  2021-09-23  9:12 ` [PATCH v5 06/20] nubus: implement BusClass get_dev_path() Mark Cave-Ayland
                   ` (14 subsequent siblings)
  19 siblings, 2 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Allow Nubus to manage the slot allocations itself using the BusClass check_address()
virtual function rather than managing this during NubusDevice realize().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bus.c    | 30 ++++++++++++++++++++++++++++++
 hw/nubus/nubus-device.c | 22 ----------------------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index 3cd7534864..d4daaa36f2 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -96,11 +96,41 @@ static void nubus_init(Object *obj)
                                                  NUBUS_SLOT_NB);
 }
 
+static bool nubus_check_address(BusState *bus, DeviceState *dev, Error **errp)
+{
+    NubusDevice *nd = NUBUS_DEVICE(dev);
+    NubusBus *nubus = NUBUS_BUS(bus);
+    uint16_t s;
+
+    if (nd->slot == -1) {
+        /* No slot specified, find first available free slot */
+        s = ctz32(nubus->slot_available_mask);
+        if (s != 32) {
+            nd->slot = s;
+        } else {
+            error_setg(errp, "Cannot register nubus card, no free slot "
+                             "available");
+            return false;
+        }
+    } else {
+        /* Slot specified, make sure the slot is available */
+        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
+            error_setg(errp, "Cannot register nubus card, slot %d is "
+                             "unavailable or already occupied", nd->slot);
+            return false;
+        }
+    }
+
+    nubus->slot_available_mask &= ~BIT(nd->slot);
+    return true;
+}
+
 static void nubus_class_init(ObjectClass *oc, void *data)
 {
     BusClass *bc = BUS_CLASS(oc);
 
     bc->realize = nubus_realize;
+    bc->check_address = nubus_check_address;
 }
 
 static const TypeInfo nubus_bus_info = {
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 562650a05b..516b13d2d5 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -160,28 +160,6 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     NubusDevice *nd = NUBUS_DEVICE(dev);
     char *name;
     hwaddr slot_offset;
-    uint16_t s;
-
-    if (nd->slot == -1) {
-        /* No slot specified, find first available free slot */
-        s = ctz32(nubus->slot_available_mask);
-        if (s != 32) {
-            nd->slot = s;
-        } else {
-            error_setg(errp, "Cannot register nubus card, no free slot "
-                             "available");
-            return;
-        }
-    } else {
-        /* Slot specified, make sure the slot is available */
-        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
-            error_setg(errp, "Cannot register nubus card, slot %d is "
-                             "unavailable or already occupied", nd->slot);
-            return;
-        }
-    }
-
-    nubus->slot_available_mask &= ~BIT(nd->slot);
 
     /* Super */
     slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
-- 
2.20.1



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

* [PATCH v5 06/20] nubus: implement BusClass get_dev_path()
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 07/20] nubus: add trace-events for empty slot accesses Mark Cave-Ayland
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bus.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index d4daaa36f2..df93a07dfe 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -96,6 +96,21 @@ static void nubus_init(Object *obj)
                                                  NUBUS_SLOT_NB);
 }
 
+static char *nubus_get_dev_path(DeviceState *dev)
+{
+    NubusDevice *nd = NUBUS_DEVICE(dev);
+    BusState *bus = qdev_get_parent_bus(dev);
+    char *p = qdev_get_dev_path(bus->parent);
+
+    if (p) {
+        char *ret = g_strdup_printf("%s/%s/%02x", p, bus->name, nd->slot);
+        g_free(p);
+        return ret;
+    } else {
+        return g_strdup_printf("%s/%02x", bus->name, nd->slot);
+    }
+}
+
 static bool nubus_check_address(BusState *bus, DeviceState *dev, Error **errp)
 {
     NubusDevice *nd = NUBUS_DEVICE(dev);
@@ -131,6 +146,7 @@ static void nubus_class_init(ObjectClass *oc, void *data)
 
     bc->realize = nubus_realize;
     bc->check_address = nubus_check_address;
+    bc->get_dev_path = nubus_get_dev_path;
 }
 
 static const TypeInfo nubus_bus_info = {
-- 
2.20.1



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

* [PATCH v5 07/20] nubus: add trace-events for empty slot accesses
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 06/20] nubus: implement BusClass get_dev_path() Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 08/20] nubus: generate bus error when attempting to access empty slots Mark Cave-Ayland
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Increase the max_access_size to 4 bytes for empty Nubus slot and super slot
accesses to allow tracing of the Nubus enumeration process by the guest OS.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bus.c  | 10 +++++++---
 hw/nubus/trace-events |  7 +++++++
 hw/nubus/trace.h      |  1 +
 meson.build           |  1 +
 4 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 hw/nubus/trace-events
 create mode 100644 hw/nubus/trace.h

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index df93a07dfe..b398423c99 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -19,6 +19,7 @@
 #include "qemu/osdep.h"
 #include "hw/nubus/nubus.h"
 #include "qapi/error.h"
+#include "trace.h"
 
 
 static NubusBus *nubus_find(void)
@@ -31,12 +32,13 @@ static void nubus_slot_write(void *opaque, hwaddr addr, uint64_t val,
                              unsigned int size)
 {
     /* read only */
+    trace_nubus_slot_write(addr, val, size);
 }
 
-
 static uint64_t nubus_slot_read(void *opaque, hwaddr addr,
                                 unsigned int size)
 {
+    trace_nubus_slot_read(addr, size);
     return 0;
 }
 
@@ -46,7 +48,7 @@ static const MemoryRegionOps nubus_slot_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
-        .max_access_size = 1,
+        .max_access_size = 4,
     },
 };
 
@@ -54,11 +56,13 @@ static void nubus_super_slot_write(void *opaque, hwaddr addr, uint64_t val,
                                    unsigned int size)
 {
     /* read only */
+    trace_nubus_super_slot_write(addr, val, size);
 }
 
 static uint64_t nubus_super_slot_read(void *opaque, hwaddr addr,
                                       unsigned int size)
 {
+    trace_nubus_super_slot_read(addr, size);
     return 0;
 }
 
@@ -68,7 +72,7 @@ static const MemoryRegionOps nubus_super_slot_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
-        .max_access_size = 1,
+        .max_access_size = 4,
     },
 };
 
diff --git a/hw/nubus/trace-events b/hw/nubus/trace-events
new file mode 100644
index 0000000000..e31833d694
--- /dev/null
+++ b/hw/nubus/trace-events
@@ -0,0 +1,7 @@
+# See docs/devel/tracing.txt for syntax documentation.
+
+# nubus-bus.c
+nubus_slot_read(uint64_t addr, int size) "reading unassigned addr 0x%"PRIx64 " size %d"
+nubus_slot_write(uint64_t addr, uint64_t val, int size) "writing unassigned addr 0x%"PRIx64 " value 0x%"PRIx64 " size %d"
+nubus_super_slot_read(uint64_t addr, int size) "reading unassigned addr 0x%"PRIx64 " size %d"
+nubus_super_slot_write(uint64_t addr, uint64_t val, int size) "writing unassigned addr 0x%"PRIx64 " value 0x%"PRIx64 " size %d"
diff --git a/hw/nubus/trace.h b/hw/nubus/trace.h
new file mode 100644
index 0000000000..3749420da1
--- /dev/null
+++ b/hw/nubus/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-hw_nubus.h"
diff --git a/meson.build b/meson.build
index 15ef4d3c41..7bdbbbdf02 100644
--- a/meson.build
+++ b/meson.build
@@ -2142,6 +2142,7 @@ if have_system
     'hw/misc/macio',
     'hw/net',
     'hw/net/can',
+    'hw/nubus',
     'hw/nvme',
     'hw/nvram',
     'hw/pci',
-- 
2.20.1



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

* [PATCH v5 08/20] nubus: generate bus error when attempting to access empty slots
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (6 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 07/20] nubus: add trace-events for empty slot accesses Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 09/20] macfb: don't register declaration ROM Mark Cave-Ayland
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

According to "Designing Cards and Drivers for the Macintosh Family" any attempt
to access an unimplemented address location on Nubus generates a bus error. MacOS
uses a custom bus error handler to detect empty Nubus slots, and with the current
implementation assumes that all slots are occupied as the Nubus transactions
never fail.

Switch nubus_slot_ops and nubus_super_slot_ops over to use {read,write}_with_attrs
and hard-code them to return MEMTX_DECODE_ERROR so that unoccupied Nubus slots
will generate the expected bus error.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bus.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index b398423c99..3db8272524 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -28,23 +28,23 @@ static NubusBus *nubus_find(void)
     return NUBUS_BUS(object_resolve_path_type("", TYPE_NUBUS_BUS, NULL));
 }
 
-static void nubus_slot_write(void *opaque, hwaddr addr, uint64_t val,
-                             unsigned int size)
+static MemTxResult nubus_slot_write(void *opaque, hwaddr addr, uint64_t val,
+                                    unsigned size, MemTxAttrs attrs)
 {
-    /* read only */
     trace_nubus_slot_write(addr, val, size);
+    return MEMTX_DECODE_ERROR;
 }
 
-static uint64_t nubus_slot_read(void *opaque, hwaddr addr,
-                                unsigned int size)
+static MemTxResult nubus_slot_read(void *opaque, hwaddr addr, uint64_t *data,
+                                   unsigned size, MemTxAttrs attrs)
 {
     trace_nubus_slot_read(addr, size);
-    return 0;
+    return MEMTX_DECODE_ERROR;
 }
 
 static const MemoryRegionOps nubus_slot_ops = {
-    .read  = nubus_slot_read,
-    .write = nubus_slot_write,
+    .read_with_attrs  = nubus_slot_read,
+    .write_with_attrs = nubus_slot_write,
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
@@ -52,23 +52,25 @@ static const MemoryRegionOps nubus_slot_ops = {
     },
 };
 
-static void nubus_super_slot_write(void *opaque, hwaddr addr, uint64_t val,
-                                   unsigned int size)
+static MemTxResult nubus_super_slot_write(void *opaque, hwaddr addr,
+                                          uint64_t val, unsigned size,
+                                          MemTxAttrs attrs)
 {
-    /* read only */
     trace_nubus_super_slot_write(addr, val, size);
+    return MEMTX_DECODE_ERROR;
 }
 
-static uint64_t nubus_super_slot_read(void *opaque, hwaddr addr,
-                                      unsigned int size)
+static MemTxResult nubus_super_slot_read(void *opaque, hwaddr addr,
+                                         uint64_t *data, unsigned size,
+                                         MemTxAttrs attrs)
 {
     trace_nubus_super_slot_read(addr, size);
-    return 0;
+    return MEMTX_DECODE_ERROR;
 }
 
 static const MemoryRegionOps nubus_super_slot_ops = {
-    .read  = nubus_super_slot_read,
-    .write = nubus_super_slot_write,
+    .read_with_attrs = nubus_super_slot_read,
+    .write_with_attrs = nubus_super_slot_write,
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
-- 
2.20.1



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

* [PATCH v5 09/20] macfb: don't register declaration ROM
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (7 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 08/20] nubus: generate bus error when attempting to access empty slots Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

The macfb device is an on-board framebuffer and so is initialised by the
system declaration ROM included within the MacOS toolbox ROM.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/display/macfb.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/display/macfb.c b/hw/display/macfb.c
index d8183b9bbd..76808b69cc 100644
--- a/hw/display/macfb.c
+++ b/hw/display/macfb.c
@@ -383,10 +383,6 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram);
 }
 
-const uint8_t macfb_rom[] = {
-    255, 0, 0, 0,
-};
-
 static void macfb_nubus_realize(DeviceState *dev, Error **errp)
 {
     NubusDevice *nd = NUBUS_DEVICE(dev);
@@ -399,8 +395,6 @@ static void macfb_nubus_realize(DeviceState *dev, Error **errp)
     macfb_common_realize(dev, ms, errp);
     memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl);
     memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram);
-
-    nubus_register_rom(nd, macfb_rom, sizeof(macfb_rom), 1, 9, 0xf);
 }
 
 static void macfb_sysbus_reset(DeviceState *d)
-- 
2.20.1



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

* [PATCH v5 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block()
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (8 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 09/20] macfb: don't register declaration ROM Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:12 ` [PATCH v5 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

Since there is no need to generate a dummy declaration ROM, remove both
nubus_register_rom() and nubus_register_format_block(). These will shortly be
replaced with a mechanism to optionally load a declaration ROM from disk to
allow real images to be used within QEMU.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-device.c  | 143 ---------------------------------------
 include/hw/nubus/nubus.h |  19 ------
 2 files changed, 162 deletions(-)

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 516b13d2d5..d4932d64a2 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -13,147 +13,6 @@
 #include "qapi/error.h"
 
 
-/* The Format Block Structure */
-
-#define FBLOCK_DIRECTORY_OFFSET 0
-#define FBLOCK_LENGTH           4
-#define FBLOCK_CRC              8
-#define FBLOCK_REVISION_LEVEL   12
-#define FBLOCK_FORMAT           13
-#define FBLOCK_TEST_PATTERN     14
-#define FBLOCK_RESERVED         18
-#define FBLOCK_BYTE_LANES       19
-
-#define FBLOCK_SIZE             20
-#define FBLOCK_PATTERN_VAL      0x5a932bc7
-
-static uint64_t nubus_fblock_read(void *opaque, hwaddr addr, unsigned int size)
-{
-    NubusDevice *dev = opaque;
-    uint64_t val;
-
-#define BYTE(v, b) (((v) >> (24 - 8 * (b))) & 0xff)
-    switch (addr) {
-    case FBLOCK_BYTE_LANES:
-        val = dev->byte_lanes;
-        val |= (val ^ 0xf) << 4;
-        break;
-    case FBLOCK_RESERVED:
-        val = 0x00;
-        break;
-    case FBLOCK_TEST_PATTERN...FBLOCK_TEST_PATTERN + 3:
-        val = BYTE(FBLOCK_PATTERN_VAL, addr - FBLOCK_TEST_PATTERN);
-        break;
-    case FBLOCK_FORMAT:
-        val = dev->rom_format;
-        break;
-    case FBLOCK_REVISION_LEVEL:
-        val = dev->rom_rev;
-        break;
-    case FBLOCK_CRC...FBLOCK_CRC + 3:
-        val = BYTE(dev->rom_crc, addr - FBLOCK_CRC);
-        break;
-    case FBLOCK_LENGTH...FBLOCK_LENGTH + 3:
-        val = BYTE(dev->rom_length, addr - FBLOCK_LENGTH);
-        break;
-    case FBLOCK_DIRECTORY_OFFSET...FBLOCK_DIRECTORY_OFFSET + 3:
-        val = BYTE(dev->directory_offset, addr - FBLOCK_DIRECTORY_OFFSET);
-        break;
-    default:
-        val = 0;
-        break;
-    }
-    return val;
-}
-
-static void nubus_fblock_write(void *opaque, hwaddr addr, uint64_t val,
-                               unsigned int size)
-{
-    /* read only */
-}
-
-static const MemoryRegionOps nubus_format_block_ops = {
-    .read = nubus_fblock_read,
-    .write = nubus_fblock_write,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    }
-};
-
-static void nubus_register_format_block(NubusDevice *dev)
-{
-    char *fblock_name;
-
-    fblock_name = g_strdup_printf("nubus-slot-%d-format-block",
-                                  dev->slot);
-
-    hwaddr fblock_offset = memory_region_size(&dev->slot_mem) - FBLOCK_SIZE;
-    memory_region_init_io(&dev->fblock_io, NULL, &nubus_format_block_ops,
-                          dev, fblock_name, FBLOCK_SIZE);
-    memory_region_add_subregion(&dev->slot_mem, fblock_offset,
-                                &dev->fblock_io);
-
-    g_free(fblock_name);
-}
-
-static void mac_nubus_rom_write(void *opaque, hwaddr addr, uint64_t val,
-                                       unsigned int size)
-{
-    /* read only */
-}
-
-static uint64_t mac_nubus_rom_read(void *opaque, hwaddr addr,
-                                    unsigned int size)
-{
-    NubusDevice *dev = opaque;
-
-    return dev->rom[addr];
-}
-
-static const MemoryRegionOps mac_nubus_rom_ops = {
-    .read  = mac_nubus_rom_read,
-    .write = mac_nubus_rom_write,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-};
-
-
-void nubus_register_rom(NubusDevice *dev, const uint8_t *rom, uint32_t size,
-                        int revision, int format, uint8_t byte_lanes)
-{
-    hwaddr rom_offset;
-    char *rom_name;
-
-    /* FIXME : really compute CRC */
-    dev->rom_length = 0;
-    dev->rom_crc = 0;
-
-    dev->rom_rev = revision;
-    dev->rom_format = format;
-
-    dev->byte_lanes = byte_lanes;
-    dev->directory_offset = -size;
-
-    /* ROM */
-
-    dev->rom = rom;
-    rom_name = g_strdup_printf("nubus-slot-%d-rom", dev->slot);
-    memory_region_init_io(&dev->rom_io, NULL, &mac_nubus_rom_ops,
-                          dev, rom_name, size);
-    memory_region_set_readonly(&dev->rom_io, true);
-
-    rom_offset = memory_region_size(&dev->slot_mem) - FBLOCK_SIZE +
-                 dev->directory_offset;
-    memory_region_add_subregion(&dev->slot_mem, rom_offset, &dev->rom_io);
-
-    g_free(rom_name);
-}
-
 static void nubus_device_realize(DeviceState *dev, Error **errp)
 {
     NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(dev));
@@ -179,8 +38,6 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&nubus->slot_io, slot_offset,
                                 &nd->slot_mem);
     g_free(name);
-
-    nubus_register_format_block(nd);
 }
 
 static Property nubus_device_properties[] = {
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 988e4a2361..2e629ffcea 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -44,25 +44,6 @@ struct NubusDevice {
     int32_t slot;
     MemoryRegion super_slot_mem;
     MemoryRegion slot_mem;
-
-    /* Format Block */
-
-    MemoryRegion fblock_io;
-
-    uint32_t rom_length;
-    uint32_t rom_crc;
-    uint8_t rom_rev;
-    uint8_t rom_format;
-    uint8_t byte_lanes;
-    int32_t directory_offset;
-
-    /* ROM */
-
-    MemoryRegion rom_io;
-    const uint8_t *rom;
 };
 
-void nubus_register_rom(NubusDevice *dev, const uint8_t *rom, uint32_t size,
-                        int revision, int format, uint8_t byte_lanes);
-
 #endif
-- 
2.20.1



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

* [PATCH v5 11/20] nubus-device: add romfile property for loading declaration ROMs
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (9 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
@ 2021-09-23  9:12 ` Mark Cave-Ayland
  2021-09-23  9:13 ` [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:12 UTC (permalink / raw)
  To: qemu-devel, laurent

The declaration ROM is located at the top-most address of the standard slot
space.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-device.c  | 44 +++++++++++++++++++++++++++++++++++++++-
 include/hw/nubus/nubus.h |  6 ++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index d4932d64a2..280f40e88a 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -9,16 +9,21 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/datadir.h"
+#include "hw/loader.h"
 #include "hw/nubus/nubus.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 
 
 static void nubus_device_realize(DeviceState *dev, Error **errp)
 {
     NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(dev));
     NubusDevice *nd = NUBUS_DEVICE(dev);
-    char *name;
+    char *name, *path;
     hwaddr slot_offset;
+    int64_t size;
+    int ret;
 
     /* Super */
     slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
@@ -38,10 +43,47 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&nubus->slot_io, slot_offset,
                                 &nd->slot_mem);
     g_free(name);
+
+    /* Declaration ROM */
+    if (nd->romfile != NULL) {
+        path = qemu_find_file(QEMU_FILE_TYPE_BIOS, nd->romfile);
+        if (path == NULL) {
+            path = g_strdup(nd->romfile);
+        }
+
+        size = get_image_size(path);
+        if (size < 0) {
+            error_setg(errp, "failed to find romfile \"%s\"", nd->romfile);
+            g_free(path);
+            return;
+        } else if (size == 0) {
+            error_setg(errp, "romfile \"%s\" is empty", nd->romfile);
+            g_free(path);
+            return;
+        } else if (size > NUBUS_DECL_ROM_MAX_SIZE) {
+            error_setg(errp, "romfile \"%s\" too large (maximum size 128K)",
+                       nd->romfile);
+            g_free(path);
+            return;
+        }
+
+        name = g_strdup_printf("nubus-slot-%x-declaration-rom", nd->slot);
+        memory_region_init_rom(&nd->decl_rom, OBJECT(dev), name, size,
+                               &error_abort);
+        ret = load_image_mr(path, &nd->decl_rom);
+        g_free(path);
+        if (ret < 0) {
+            error_setg(errp, "could not load romfile \"%s\"", nd->romfile);
+            return;
+        }
+        memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE - size,
+                                    &nd->decl_rom);
+    }
 }
 
 static Property nubus_device_properties[] = {
     DEFINE_PROP_INT32("slot", NubusDevice, slot, -1),
+    DEFINE_PROP_STRING("romfile", NubusDevice, romfile),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 2e629ffcea..73a9d8cb7f 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -12,6 +12,7 @@
 #include "hw/qdev-properties.h"
 #include "exec/address-spaces.h"
 #include "qom/object.h"
+#include "qemu/units.h"
 
 #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
 #define NUBUS_SUPER_SLOT_NB   0xe
@@ -38,12 +39,17 @@ struct NubusBus {
     uint32_t slot_available_mask;
 };
 
+#define NUBUS_DECL_ROM_MAX_SIZE    (128 * KiB)
+
 struct NubusDevice {
     DeviceState qdev;
 
     int32_t slot;
     MemoryRegion super_slot_mem;
     MemoryRegion slot_mem;
+
+    char *romfile;
+    MemoryRegion decl_rom;
 };
 
 #endif
-- 
2.20.1



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

* [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (10 preceding siblings ...)
  2021-09-23  9:12 ` [PATCH v5 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23 10:33   ` Laurent Vivier
  2021-09-23  9:13 ` [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

According to "Designing Cards and Drivers for the Macintosh Family" the Nubus
has its own 32-bit address space based upon physical slot addressing.

Move Nubus to its own 32-bit address space and then use memory region aliases
to map available slot and super slot ranges into the q800 system address
space via the Macintosh Nubus bridge.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/m68k/q800.c                      |  9 ++++-----
 hw/nubus/mac-nubus-bridge.c         | 16 ++++++++++++++--
 hw/nubus/nubus-bus.c                | 18 ++++++++++++++++++
 include/hw/nubus/mac-nubus-bridge.h |  2 ++
 include/hw/nubus/nubus.h            |  6 ++++++
 5 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 5ba87f789c..a07912b87c 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -67,9 +67,6 @@
 #define ASC_BASE              (IO_BASE + 0x14000)
 #define SWIM_BASE             (IO_BASE + 0x1E000)
 
-#define NUBUS_SUPER_SLOT_BASE 0x60000000
-#define NUBUS_SLOT_BASE       0xf0000000
-
 #define SONIC_PROM_SIZE       0x1000
 
 /*
@@ -396,8 +393,10 @@ static void q800_init(MachineState *machine)
 
     dev = qdev_new(TYPE_MAC_NUBUS_BRIDGE);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, NUBUS_SUPER_SLOT_BASE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
+                    MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
+                    MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
 
     nubus = MAC_NUBUS_BRIDGE(dev)->bus;
 
diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 3f075789e9..3af4f5d396 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -22,8 +22,20 @@ static void mac_nubus_bridge_init(Object *obj)
     s->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
                                                   MAC_NUBUS_SLOT_NB);
 
-    sysbus_init_mmio(sbd, &s->bus->super_slot_io);
-    sysbus_init_mmio(sbd, &s->bus->slot_io);
+    /* Aliases for slots 0x9 to 0xe */
+    memory_region_init_alias(&s->super_slot_alias, obj, "super-slot-alias",
+                             &s->bus->nubus_mr,
+                             MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE,
+                             MAC_NUBUS_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
+
+    memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
+                             &s->bus->nubus_mr,
+                             NUBUS_SLOT_BASE +
+                             MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE,
+                             MAC_NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
+
+    sysbus_init_mmio(sbd, &s->super_slot_alias);
+    sysbus_init_mmio(sbd, &s->slot_alias);
 }
 
 static void mac_nubus_bridge_class_init(ObjectClass *klass, void *data)
diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index 3db8272524..40d9068c59 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -78,25 +78,42 @@ static const MemoryRegionOps nubus_super_slot_ops = {
     },
 };
 
+static void nubus_unrealize(BusState *bus)
+{
+    NubusBus *nubus = NUBUS_BUS(bus);
+
+    address_space_destroy(&nubus->nubus_as);
+}
+
 static void nubus_realize(BusState *bus, Error **errp)
 {
+    NubusBus *nubus = NUBUS_BUS(bus);
+
     if (!nubus_find()) {
         error_setg(errp, "at most one %s device is permitted", TYPE_NUBUS_BUS);
         return;
     }
+
+    address_space_init(&nubus->nubus_as, &nubus->nubus_mr, "nubus");
 }
 
 static void nubus_init(Object *obj)
 {
     NubusBus *nubus = NUBUS_BUS(obj);
 
+    memory_region_init(&nubus->nubus_mr, obj, "nubus", 0x100000000);
+
     memory_region_init_io(&nubus->super_slot_io, obj, &nubus_super_slot_ops,
                           nubus, "nubus-super-slots",
                           (NUBUS_SUPER_SLOT_NB + 1) * NUBUS_SUPER_SLOT_SIZE);
+    memory_region_add_subregion(&nubus->nubus_mr, 0x0, &nubus->super_slot_io);
 
     memory_region_init_io(&nubus->slot_io, obj, &nubus_slot_ops,
                           nubus, "nubus-slots",
                           NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
+    memory_region_add_subregion(&nubus->nubus_mr,
+                                (NUBUS_SUPER_SLOT_NB + 1) *
+                                NUBUS_SUPER_SLOT_SIZE, &nubus->slot_io);
 
     nubus->slot_available_mask = MAKE_64BIT_MASK(NUBUS_FIRST_SLOT,
                                                  NUBUS_SLOT_NB);
@@ -151,6 +168,7 @@ static void nubus_class_init(ObjectClass *oc, void *data)
     BusClass *bc = BUS_CLASS(oc);
 
     bc->realize = nubus_realize;
+    bc->unrealize = nubus_unrealize;
     bc->check_address = nubus_check_address;
     bc->get_dev_path = nubus_get_dev_path;
 }
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index 118d67267d..04451d357c 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -23,6 +23,8 @@ struct MacNubusState {
     SysBusDevice sysbus_dev;
 
     NubusBus *bus;
+    MemoryRegion super_slot_alias;
+    MemoryRegion slot_alias;
 };
 
 #endif
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 73a9d8cb7f..f2c8c22c05 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -17,6 +17,9 @@
 #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
 #define NUBUS_SUPER_SLOT_NB   0xe
 
+#define NUBUS_SLOT_BASE       (NUBUS_SUPER_SLOT_SIZE * \
+                               (NUBUS_SUPER_SLOT_NB + 1))
+
 #define NUBUS_SLOT_SIZE       0x01000000
 #define NUBUS_FIRST_SLOT      0x0
 #define NUBUS_LAST_SLOT       0xf
@@ -33,6 +36,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(NubusBus, NUBUS_BUS)
 struct NubusBus {
     BusState qbus;
 
+    AddressSpace nubus_as;
+    MemoryRegion nubus_mr;
+
     MemoryRegion super_slot_io;
     MemoryRegion slot_io;
 
-- 
2.20.1



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

* [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (11 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23 10:35   ` Laurent Vivier
  2021-09-23  9:13 ` [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

This is to allow the Nubus bridge to store its own additional state. Also update
the comment in the file header to reflect that nubus-bridge is not specific to
the Macintosh.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/nubus/nubus-bridge.c             | 4 ++--
 include/hw/nubus/mac-nubus-bridge.h | 2 +-
 include/hw/nubus/nubus.h            | 6 ++++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c
index cd8c6a91eb..95662568c5 100644
--- a/hw/nubus/nubus-bridge.c
+++ b/hw/nubus/nubus-bridge.c
@@ -1,5 +1,5 @@
 /*
- * QEMU Macintosh Nubus
+ * QEMU Nubus
  *
  * Copyright (c) 2013-2018 Laurent Vivier <laurent@vivier.eu>
  *
@@ -22,7 +22,7 @@ static void nubus_bridge_class_init(ObjectClass *klass, void *data)
 static const TypeInfo nubus_bridge_info = {
     .name          = TYPE_NUBUS_BRIDGE,
     .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SysBusDevice),
+    .instance_size = sizeof(NubusBridge),
     .class_init    = nubus_bridge_class_init,
 };
 
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index 04451d357c..fa454f5fbe 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -20,7 +20,7 @@
 OBJECT_DECLARE_SIMPLE_TYPE(MacNubusState, MAC_NUBUS_BRIDGE)
 
 struct MacNubusState {
-    SysBusDevice sysbus_dev;
+    NubusBridge parent_obj;
 
     NubusBus *bus;
     MemoryRegion super_slot_alias;
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index f2c8c22c05..43bde7dd69 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -10,6 +10,7 @@
 #define HW_NUBUS_NUBUS_H
 
 #include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "qom/object.h"
 #include "qemu/units.h"
@@ -32,6 +33,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
 OBJECT_DECLARE_SIMPLE_TYPE(NubusBus, NUBUS_BUS)
 
 #define TYPE_NUBUS_BRIDGE "nubus-bridge"
+OBJECT_DECLARE_SIMPLE_TYPE(NubusBridge, NUBUS_BRIDGE);
 
 struct NubusBus {
     BusState qbus;
@@ -58,4 +60,8 @@ struct NubusDevice {
     MemoryRegion decl_rom;
 };
 
+struct NubusBridge {
+    SysBusDevice parent_obj;
+};
+
 #endif
-- 
2.20.1



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

* [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (12 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23 10:35   ` Laurent Vivier
  2021-09-23  9:13 ` [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

This better reflects that the mac-nubus-bridge device is derived from the
nubus-bridge device, and that the structure represents the state of the bridge
device and not the Nubus itself. Also update the comment in the file header to
reflect that mac-nubus-bridge is specific to the Macintosh.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/nubus/mac-nubus-bridge.c         | 8 +++++---
 include/hw/nubus/mac-nubus-bridge.h | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 3af4f5d396..e241c581b5 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -1,5 +1,7 @@
 /*
- *  Copyright (c) 2013-2018 Laurent Vivier <laurent@vivier.eu>
+ * QEMU Macintosh Nubus
+ *
+ * Copyright (c) 2013-2018 Laurent Vivier <laurent@vivier.eu>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -13,7 +15,7 @@
 
 static void mac_nubus_bridge_init(Object *obj)
 {
-    MacNubusState *s = MAC_NUBUS_BRIDGE(obj);
+    MacNubusBridge *s = MAC_NUBUS_BRIDGE(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
     s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
@@ -49,7 +51,7 @@ static const TypeInfo mac_nubus_bridge_info = {
     .name          = TYPE_MAC_NUBUS_BRIDGE,
     .parent        = TYPE_NUBUS_BRIDGE,
     .instance_init = mac_nubus_bridge_init,
-    .instance_size = sizeof(MacNubusState),
+    .instance_size = sizeof(MacNubusBridge),
     .class_init    = mac_nubus_bridge_class_init,
 };
 
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index fa454f5fbe..b595e1b7ef 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -17,9 +17,9 @@
 #define MAC_NUBUS_SLOT_NB    (MAC_NUBUS_LAST_SLOT - MAC_NUBUS_FIRST_SLOT + 1)
 
 #define TYPE_MAC_NUBUS_BRIDGE "mac-nubus-bridge"
-OBJECT_DECLARE_SIMPLE_TYPE(MacNubusState, MAC_NUBUS_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(MacNubusBridge, MAC_NUBUS_BRIDGE)
 
-struct MacNubusState {
+struct MacNubusBridge {
     NubusBridge parent_obj;
 
     NubusBus *bus;
-- 
2.20.1



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

* [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (13 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23 10:36   ` Laurent Vivier
  2021-09-23  9:13 ` [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

Now that Nubus has its own address space rather than mapping directly into the
system bus, move the Nubus reference from MacNubusBridge to NubusBridge.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/m68k/q800.c                      |  2 +-
 hw/nubus/mac-nubus-bridge.c         | 11 +++++------
 hw/nubus/nubus-bridge.c             |  9 +++++++++
 include/hw/nubus/mac-nubus-bridge.h |  1 -
 include/hw/nubus/nubus.h            |  2 ++
 5 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index a07912b87c..9bdea1a362 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -398,7 +398,7 @@ static void q800_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
                     MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
 
-    nubus = MAC_NUBUS_BRIDGE(dev)->bus;
+    nubus = NUBUS_BRIDGE(dev)->bus;
 
     /* framebuffer in nubus slot #9 */
 
diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index e241c581b5..db8640eed2 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -16,22 +16,21 @@
 static void mac_nubus_bridge_init(Object *obj)
 {
     MacNubusBridge *s = MAC_NUBUS_BRIDGE(obj);
+    NubusBridge *nb = NUBUS_BRIDGE(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
-
     /* Macintosh only has slots 0x9 to 0xe available */
-    s->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
-                                                  MAC_NUBUS_SLOT_NB);
+    nb->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
+                                                   MAC_NUBUS_SLOT_NB);
 
     /* Aliases for slots 0x9 to 0xe */
     memory_region_init_alias(&s->super_slot_alias, obj, "super-slot-alias",
-                             &s->bus->nubus_mr,
+                             &nb->bus->nubus_mr,
                              MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE,
                              MAC_NUBUS_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
 
     memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
-                             &s->bus->nubus_mr,
+                             &nb->bus->nubus_mr,
                              NUBUS_SLOT_BASE +
                              MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE,
                              MAC_NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c
index 95662568c5..3b68d4435c 100644
--- a/hw/nubus/nubus-bridge.c
+++ b/hw/nubus/nubus-bridge.c
@@ -12,6 +12,14 @@
 #include "hw/sysbus.h"
 #include "hw/nubus/nubus.h"
 
+
+static void nubus_bridge_init(Object *obj)
+{
+    NubusBridge *s = NUBUS_BRIDGE(obj);
+
+    s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
+}
+
 static void nubus_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -22,6 +30,7 @@ static void nubus_bridge_class_init(ObjectClass *klass, void *data)
 static const TypeInfo nubus_bridge_info = {
     .name          = TYPE_NUBUS_BRIDGE,
     .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_init = nubus_bridge_init,
     .instance_size = sizeof(NubusBridge),
     .class_init    = nubus_bridge_class_init,
 };
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index b595e1b7ef..70ab50ab2d 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -22,7 +22,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(MacNubusBridge, MAC_NUBUS_BRIDGE)
 struct MacNubusBridge {
     NubusBridge parent_obj;
 
-    NubusBus *bus;
     MemoryRegion super_slot_alias;
     MemoryRegion slot_alias;
 };
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 43bde7dd69..607cdef968 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -62,6 +62,8 @@ struct NubusDevice {
 
 struct NubusBridge {
     SysBusDevice parent_obj;
+
+    NubusBus *bus;
 };
 
 #endif
-- 
2.20.1



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

* [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (14 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23 10:37   ` Laurent Vivier
  2021-09-23  9:13 ` [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

Since nubus-bridge is a container for NubusBus then it should be embedded
directly within the bridge device using qbus_create_inplace().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/m68k/q800.c              | 2 +-
 hw/nubus/mac-nubus-bridge.c | 9 +++++----
 hw/nubus/nubus-bridge.c     | 3 ++-
 include/hw/nubus/nubus.h    | 2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 9bdea1a362..074acf4fdc 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -398,7 +398,7 @@ static void q800_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
                     MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
 
-    nubus = NUBUS_BRIDGE(dev)->bus;
+    nubus = &NUBUS_BRIDGE(dev)->bus;
 
     /* framebuffer in nubus slot #9 */
 
diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index db8640eed2..a0da5a8b2f 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -18,19 +18,20 @@ static void mac_nubus_bridge_init(Object *obj)
     MacNubusBridge *s = MAC_NUBUS_BRIDGE(obj);
     NubusBridge *nb = NUBUS_BRIDGE(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    NubusBus *bus = &nb->bus;
 
     /* Macintosh only has slots 0x9 to 0xe available */
-    nb->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
-                                                   MAC_NUBUS_SLOT_NB);
+    bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
+                                               MAC_NUBUS_SLOT_NB);
 
     /* Aliases for slots 0x9 to 0xe */
     memory_region_init_alias(&s->super_slot_alias, obj, "super-slot-alias",
-                             &nb->bus->nubus_mr,
+                             &bus->nubus_mr,
                              MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE,
                              MAC_NUBUS_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
 
     memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
-                             &nb->bus->nubus_mr,
+                             &bus->nubus_mr,
                              NUBUS_SLOT_BASE +
                              MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE,
                              MAC_NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c
index 3b68d4435c..1adda7f5a6 100644
--- a/hw/nubus/nubus-bridge.c
+++ b/hw/nubus/nubus-bridge.c
@@ -16,8 +16,9 @@
 static void nubus_bridge_init(Object *obj)
 {
     NubusBridge *s = NUBUS_BRIDGE(obj);
+    NubusBus *bus = &s->bus;
 
-    s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
+    qbus_create_inplace(bus, sizeof(s->bus), TYPE_NUBUS_BUS, DEVICE(s), NULL);
 }
 
 static void nubus_bridge_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 607cdef968..3620247be2 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -63,7 +63,7 @@ struct NubusDevice {
 struct NubusBridge {
     SysBusDevice parent_obj;
 
-    NubusBus *bus;
+    NubusBus bus;
 };
 
 #endif
-- 
2.20.1



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

* [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (15 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23  9:52   ` Philippe Mathieu-Daudé
  2021-09-23  9:13 ` [PATCH v5 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

This is to allow Macintosh machines to further specify which slots are available
since the number of addressable slots may not match the number of physical slots
present in the machine.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bridge.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c
index 1adda7f5a6..2c7c4ee121 100644
--- a/hw/nubus/nubus-bridge.c
+++ b/hw/nubus/nubus-bridge.c
@@ -21,11 +21,18 @@ static void nubus_bridge_init(Object *obj)
     qbus_create_inplace(bus, sizeof(s->bus), TYPE_NUBUS_BUS, DEVICE(s), NULL);
 }
 
+static Property nubus_bridge_properties[] = {
+    DEFINE_PROP_UINT32("slot-available-mask", NubusBridge,
+                       bus.slot_available_mask, 0xffff),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void nubus_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->fw_name = "nubus";
+    device_class_set_props(dc, nubus_bridge_properties);
 }
 
 static const TypeInfo nubus_bridge_info = {
-- 
2.20.1



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

* [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (16 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23  9:49   ` Philippe Mathieu-Daudé
  2021-09-23  9:13 ` [PATCH v5 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
  2021-09-23  9:13 ` [PATCH v5 20/20] q800: configure nubus available slots for Quadra 800 Mark Cave-Ayland
  19 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

Each Nubus slot has an IRQ line that can be used to request service from the
CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
gpios accordingly, and introduce a new nubus_set_irq() function that can be used
by Nubus devices to control the slot IRQ.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/nubus/nubus-bridge.c  | 2 ++
 hw/nubus/nubus-device.c  | 8 ++++++++
 include/hw/nubus/nubus.h | 6 ++++++
 3 files changed, 16 insertions(+)

diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c
index 2c7c4ee121..0366d925a9 100644
--- a/hw/nubus/nubus-bridge.c
+++ b/hw/nubus/nubus-bridge.c
@@ -19,6 +19,8 @@ static void nubus_bridge_init(Object *obj)
     NubusBus *bus = &s->bus;
 
     qbus_create_inplace(bus, sizeof(s->bus), TYPE_NUBUS_BUS, DEVICE(s), NULL);
+
+    qdev_init_gpio_out(DEVICE(s), bus->irqs, NUBUS_IRQS);
 }
 
 static Property nubus_bridge_properties[] = {
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 280f40e88a..0f1852f671 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -10,12 +10,20 @@
 
 #include "qemu/osdep.h"
 #include "qemu/datadir.h"
+#include "hw/irq.h"
 #include "hw/loader.h"
 #include "hw/nubus/nubus.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 
 
+void nubus_set_irq(NubusDevice *nd, int level)
+{
+    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
+
+    qemu_set_irq(nubus->irqs[nd->slot], level);
+}
+
 static void nubus_device_realize(DeviceState *dev, Error **errp)
 {
     NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(dev));
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 3620247be2..65d7b078b8 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -26,6 +26,8 @@
 #define NUBUS_LAST_SLOT       0xf
 #define NUBUS_SLOT_NB         (NUBUS_LAST_SLOT - NUBUS_FIRST_SLOT + 1)
 
+#define NUBUS_IRQS            16
+
 #define TYPE_NUBUS_DEVICE "nubus-device"
 OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
 
@@ -45,6 +47,8 @@ struct NubusBus {
     MemoryRegion slot_io;
 
     uint32_t slot_available_mask;
+
+    qemu_irq irqs[NUBUS_IRQS];
 };
 
 #define NUBUS_DECL_ROM_MAX_SIZE    (128 * KiB)
@@ -60,6 +64,8 @@ struct NubusDevice {
     MemoryRegion decl_rom;
 };
 
+void nubus_set_irq(NubusDevice *nd, int level);
+
 struct NubusBridge {
     SysBusDevice parent_obj;
 
-- 
2.20.1



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

* [PATCH v5 19/20] q800: wire up nubus IRQs
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (17 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  2021-09-23  9:13 ` [PATCH v5 20/20] q800: configure nubus available slots for Quadra 800 Mark Cave-Ayland
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

Nubus IRQs are routed to the CPU through the VIA2 device so wire up the IRQs
using gpios accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 074acf4fdc..5bc9df58a0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -398,6 +398,12 @@ static void q800_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
                     MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
 
+    for (i = 0; i < VIA2_NUBUS_IRQ_NB; i++) {
+        qdev_connect_gpio_out(dev, 9 + i,
+                              qdev_get_gpio_in_named(via2_dev, "nubus-irq",
+                                                     VIA2_NUBUS_IRQ_9 + i));
+    }
+
     nubus = &NUBUS_BRIDGE(dev)->bus;
 
     /* framebuffer in nubus slot #9 */
-- 
2.20.1



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

* [PATCH v5 20/20] q800: configure nubus available slots for Quadra 800
  2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
                   ` (18 preceding siblings ...)
  2021-09-23  9:13 ` [PATCH v5 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
@ 2021-09-23  9:13 ` Mark Cave-Ayland
  19 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23  9:13 UTC (permalink / raw)
  To: qemu-devel, laurent

Slot 0x9 is reserved for use by the in-built framebuffer whilst only slots
0xc, 0xd and 0xe physically exist on the Quadra 800.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 5bc9df58a0..09b3366024 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -78,6 +78,13 @@
 
 #define MAC_CLOCK  3686418
 
+/*
+ * Slot 0x9 is reserved for use by the in-built framebuffer whilst only
+ * slots 0xc, 0xd and 0xe physically exist on the Quadra 800
+ */
+#define Q800_NUBUS_SLOTS_AVAILABLE    (BIT(0x9) | BIT(0xc) | BIT(0xd) | \
+                                       BIT(0xe))
+
 /*
  * The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
  * that performs a variety of functions (RAM management, clock generation, ...).
@@ -392,6 +399,8 @@ static void q800_init(MachineState *machine)
     /* NuBus */
 
     dev = qdev_new(TYPE_MAC_NUBUS_BRIDGE);
+    qdev_prop_set_uint32(dev, "slot-available-mask",
+                         Q800_NUBUS_SLOTS_AVAILABLE);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0,
                     MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE);
-- 
2.20.1



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

* Re: [PATCH v5 01/20] nubus: add comment indicating reference documents
  2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
@ 2021-09-23  9:35   ` Philippe Mathieu-Daudé
  2021-09-23 10:01   ` Laurent Vivier
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-23  9:35 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/23/21 11:12, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/nubus/nubus-bus.c | 8 ++++++++
>   1 file changed, 8 insertions(+)

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


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
@ 2021-09-23  9:42   ` Philippe Mathieu-Daudé
  2021-09-24  7:00     ` Mark Cave-Ayland
  2021-09-23 10:28   ` Laurent Vivier
  2021-09-23 14:16   ` BALATON Zoltan
  2 siblings, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-23  9:42 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/23/21 11:12, Mark Cave-Ayland wrote:
> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
> for future Nubus devices to be plugged into arbitrary slots from the command line
> using a new qdev "slot" parameter for nubus devices.
> 
> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/nubus/mac-nubus-bridge.c         |  4 ++++
>   hw/nubus/nubus-bus.c                |  5 +++--
>   hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>   include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>   include/hw/nubus/nubus.h            | 13 ++++++------
>   5 files changed, 43 insertions(+), 15 deletions(-)

> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index 4e23df1280..562650a05b 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -160,14 +160,28 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>       NubusDevice *nd = NUBUS_DEVICE(dev);
>       char *name;
>       hwaddr slot_offset;
> -
> -    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
> -            nubus->current_slot > NUBUS_LAST_SLOT) {
> -        error_setg(errp, "Cannot register nubus card, not enough slots");
> -        return;
> +    uint16_t s;
> +
> +    if (nd->slot == -1) {
> +        /* No slot specified, find first available free slot */
> +        s = ctz32(nubus->slot_available_mask);

Nitpicking:

            int s = ctz32(nubus->slot_available_mask);

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

> +        if (s != 32) {
> +            nd->slot = s;
> +        } else {
> +            error_setg(errp, "Cannot register nubus card, no free slot "
> +                             "available");
> +            return;
> +        }
> +    } else {
> +        /* Slot specified, make sure the slot is available */
> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
> +            error_setg(errp, "Cannot register nubus card, slot %d is "
> +                             "unavailable or already occupied", nd->slot);
> +            return;
> +        }
>       }


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

* Re: [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()
  2021-09-23  9:12 ` [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
@ 2021-09-23  9:45   ` Philippe Mathieu-Daudé
  2021-09-24  7:01     ` Mark Cave-Ayland
  2021-09-23 10:30   ` Laurent Vivier
  1 sibling, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-23  9:45 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/23/21 11:12, Mark Cave-Ayland wrote:
> Allow Nubus to manage the slot allocations itself using the BusClass check_address()
> virtual function rather than managing this during NubusDevice realize().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/nubus/nubus-bus.c    | 30 ++++++++++++++++++++++++++++++
>   hw/nubus/nubus-device.c | 22 ----------------------
>   2 files changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
> index 3cd7534864..d4daaa36f2 100644
> --- a/hw/nubus/nubus-bus.c
> +++ b/hw/nubus/nubus-bus.c
> @@ -96,11 +96,41 @@ static void nubus_init(Object *obj)
>                                                    NUBUS_SLOT_NB);
>   }
>   
> +static bool nubus_check_address(BusState *bus, DeviceState *dev, Error **errp)
> +{
> +    NubusDevice *nd = NUBUS_DEVICE(dev);
> +    NubusBus *nubus = NUBUS_BUS(bus);
> +    uint16_t s;
> +
> +    if (nd->slot == -1) {
> +        /* No slot specified, find first available free slot */
> +        s = ctz32(nubus->slot_available_mask);

Same comment than previous patch:

            int s = ctz32(nubus->slot_available_mask);

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

> +        if (s != 32) {
> +            nd->slot = s;
> +        } else {
> +            error_setg(errp, "Cannot register nubus card, no free slot "
> +                             "available");
> +            return false;
> +        }
> +    } else {
> +        /* Slot specified, make sure the slot is available */
> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
> +            error_setg(errp, "Cannot register nubus card, slot %d is "
> +                             "unavailable or already occupied", nd->slot);
> +            return false;
> +        }
> +    }
> +
> +    nubus->slot_available_mask &= ~BIT(nd->slot);
> +    return true;
> +}


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-23  9:13 ` [PATCH v5 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
@ 2021-09-23  9:49   ` Philippe Mathieu-Daudé
  2021-09-24  7:06     ` Mark Cave-Ayland
  0 siblings, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-23  9:49 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/23/21 11:13, Mark Cave-Ayland wrote:
> Each Nubus slot has an IRQ line that can be used to request service from the
> CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
> gpios accordingly, and introduce a new nubus_set_irq() function that can be used
> by Nubus devices to control the slot IRQ.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/nubus/nubus-bridge.c  | 2 ++
>   hw/nubus/nubus-device.c  | 8 ++++++++
>   include/hw/nubus/nubus.h | 6 ++++++
>   3 files changed, 16 insertions(+)

>   static Property nubus_bridge_properties[] = {
> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index 280f40e88a..0f1852f671 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -10,12 +10,20 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/datadir.h"
> +#include "hw/irq.h"
>   #include "hw/loader.h"
>   #include "hw/nubus/nubus.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   
>   
> +void nubus_set_irq(NubusDevice *nd, int level)
> +{
> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
> +

A trace-event could be helpful here, otherwise:

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

> +    qemu_set_irq(nubus->irqs[nd->slot], level);
> +}


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

* Re: [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property
  2021-09-23  9:13 ` [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
@ 2021-09-23  9:52   ` Philippe Mathieu-Daudé
  2021-09-24  7:09     ` Mark Cave-Ayland
  0 siblings, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-23  9:52 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/23/21 11:13, Mark Cave-Ayland wrote:
> This is to allow Macintosh machines to further specify which slots are available
> since the number of addressable slots may not match the number of physical slots
> present in the machine.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/nubus/nubus-bridge.c | 7 +++++++
>   1 file changed, 7 insertions(+)

> +static Property nubus_bridge_properties[] = {
> +    DEFINE_PROP_UINT32("slot-available-mask", NubusBridge,
> +                       bus.slot_available_mask, 0xffff),

What about using DEFINE_PROP_UINT16() here and uint16_t in
patch 04/20 "nubus: use bitmap to manage available slots"?

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

> +    DEFINE_PROP_END_OF_LIST()
> +};


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

* Re: [PATCH v5 01/20] nubus: add comment indicating reference documents
  2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
  2021-09-23  9:35   ` Philippe Mathieu-Daudé
@ 2021-09-23 10:01   ` Laurent Vivier
  1 sibling, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:01 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:12, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/nubus/nubus-bus.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
> index 5c13452308..f4410803ff 100644
> --- a/hw/nubus/nubus-bus.c
> +++ b/hw/nubus/nubus-bus.c
> @@ -8,6 +8,14 @@
>   *
>   */
>  
> +/*
> + * References:
> + *   Nubus Specification (TI)
> + *     http://www.bitsavers.org/pdf/ti/nubus/2242825-0001_NuBus_Spec1983.pdf
> + *
> + *   Designing Cards and Drivers for the Macintosh Family (Apple)
> + */
> +
>  #include "qemu/osdep.h"
>  #include "hw/nubus/nubus.h"
>  #include "qapi/error.h"
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


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

* Re: [PATCH v5 03/20] nubus-device: expose separate super slot memory region
  2021-09-23  9:12 ` [PATCH v5 03/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
@ 2021-09-23 10:04   ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:04 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:12, Mark Cave-Ayland a écrit :
> According to "Designing Cards and Drivers for the Macintosh Family" each physical
> nubus slot can access 2 separate address ranges: a super slot memory region which
> is 256MB and a standard slot memory region which is 16MB.
> 
> Currently a Nubus device uses the physical slot number to determine whether it is
> using a standard slot memory region or a super slot memory region rather than
> exposing both memory regions for use as required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/nubus/nubus-device.c  | 36 ++++++++++++++++++------------------
>  include/hw/nubus/nubus.h |  1 +
>  2 files changed, 19 insertions(+), 18 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
  2021-09-23  9:42   ` Philippe Mathieu-Daudé
@ 2021-09-23 10:28   ` Laurent Vivier
  2021-09-23 14:16   ` BALATON Zoltan
  2 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:28 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:12, Mark Cave-Ayland a écrit :
> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
> for future Nubus devices to be plugged into arbitrary slots from the command line
> using a new qdev "slot" parameter for nubus devices.
> 
> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/nubus/mac-nubus-bridge.c         |  4 ++++
>  hw/nubus/nubus-bus.c                |  5 +++--
>  hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>  include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>  include/hw/nubus/nubus.h            | 13 ++++++------
>  5 files changed, 43 insertions(+), 15 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



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

* Re: [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()
  2021-09-23  9:12 ` [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
  2021-09-23  9:45   ` Philippe Mathieu-Daudé
@ 2021-09-23 10:30   ` Laurent Vivier
  1 sibling, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:30 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:12, Mark Cave-Ayland a écrit :
> Allow Nubus to manage the slot allocations itself using the BusClass check_address()
> virtual function rather than managing this during NubusDevice realize().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/nubus/nubus-bus.c    | 30 ++++++++++++++++++++++++++++++
>  hw/nubus/nubus-device.c | 22 ----------------------
>  2 files changed, 30 insertions(+), 22 deletions(-)
> 
>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



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

* Re: [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space
  2021-09-23  9:13 ` [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
@ 2021-09-23 10:33   ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:33 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
> According to "Designing Cards and Drivers for the Macintosh Family" the Nubus
> has its own 32-bit address space based upon physical slot addressing.
> 
> Move Nubus to its own 32-bit address space and then use memory region aliases
> to map available slot and super slot ranges into the q800 system address
> space via the Macintosh Nubus bridge.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/m68k/q800.c                      |  9 ++++-----
>  hw/nubus/mac-nubus-bridge.c         | 16 ++++++++++++++--
>  hw/nubus/nubus-bus.c                | 18 ++++++++++++++++++
>  include/hw/nubus/mac-nubus-bridge.h |  2 ++
>  include/hw/nubus/nubus.h            |  6 ++++++
>  5 files changed, 44 insertions(+), 7 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



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

* Re: [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure
  2021-09-23  9:13 ` [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
@ 2021-09-23 10:35   ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:35 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
> This is to allow the Nubus bridge to store its own additional state. Also update
> the comment in the file header to reflect that nubus-bridge is not specific to
> the Macintosh.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/nubus/nubus-bridge.c             | 4 ++--
>  include/hw/nubus/mac-nubus-bridge.h | 2 +-
>  include/hw/nubus/nubus.h            | 6 ++++++
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

I think it could be merged with next patch.



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

* Re: [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23  9:13 ` [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
@ 2021-09-23 10:35   ` Laurent Vivier
  2021-09-23 12:50     ` Mark Cave-Ayland
  0 siblings, 1 reply; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:35 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
> This better reflects that the mac-nubus-bridge device is derived from the
> nubus-bridge device, and that the structure represents the state of the bridge
> device and not the Nubus itself. Also update the comment in the file header to
> reflect that mac-nubus-bridge is specific to the Macintosh.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/nubus/mac-nubus-bridge.c         | 8 +++++---
>  include/hw/nubus/mac-nubus-bridge.h | 4 ++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 


Reviewed-by: Laurent Vivier <laurent@vivier.eu>

(it could be merged with previous one)



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

* Re: [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge
  2021-09-23  9:13 ` [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
@ 2021-09-23 10:36   ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:36 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
> Now that Nubus has its own address space rather than mapping directly into the
> system bus, move the Nubus reference from MacNubusBridge to NubusBridge.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/m68k/q800.c                      |  2 +-
>  hw/nubus/mac-nubus-bridge.c         | 11 +++++------
>  hw/nubus/nubus-bridge.c             |  9 +++++++++
>  include/hw/nubus/mac-nubus-bridge.h |  1 -
>  include/hw/nubus/nubus.h            |  2 ++
>  5 files changed, 17 insertions(+), 8 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



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

* Re: [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge
  2021-09-23  9:13 ` [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
@ 2021-09-23 10:37   ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 10:37 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
> Since nubus-bridge is a container for NubusBus then it should be embedded
> directly within the bridge device using qbus_create_inplace().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/m68k/q800.c              | 2 +-
>  hw/nubus/mac-nubus-bridge.c | 9 +++++----
>  hw/nubus/nubus-bridge.c     | 3 ++-
>  include/hw/nubus/nubus.h    | 2 +-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 


Reviewed-by: Laurent Vivier <laurent@vivier.eu>



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

* Re: [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23 10:35   ` Laurent Vivier
@ 2021-09-23 12:50     ` Mark Cave-Ayland
  2021-09-23 13:53       ` Laurent Vivier
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23 12:50 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel

On 23/09/2021 11:35, Laurent Vivier wrote:

> Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
>> This better reflects that the mac-nubus-bridge device is derived from the
>> nubus-bridge device, and that the structure represents the state of the bridge
>> device and not the Nubus itself. Also update the comment in the file header to
>> reflect that mac-nubus-bridge is specific to the Macintosh.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/nubus/mac-nubus-bridge.c         | 8 +++++---
>>   include/hw/nubus/mac-nubus-bridge.h | 4 ++--
>>   2 files changed, 7 insertions(+), 5 deletions(-)
>>
> 
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> (it could be merged with previous one)

I like to try and keep renames on a per-device basis if possible, even if it's just 
to help rebasing during development.

Other than that, is there anything else outstanding you think would require a v6 series?


ATB,

Mark.


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

* Re: [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23 12:50     ` Mark Cave-Ayland
@ 2021-09-23 13:53       ` Laurent Vivier
  2021-09-23 16:03         ` Mark Cave-Ayland
  0 siblings, 1 reply; 52+ messages in thread
From: Laurent Vivier @ 2021-09-23 13:53 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel

Le 23/09/2021 à 14:50, Mark Cave-Ayland a écrit :
> On 23/09/2021 11:35, Laurent Vivier wrote:
> 
>> Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
>>> This better reflects that the mac-nubus-bridge device is derived from the
>>> nubus-bridge device, and that the structure represents the state of the bridge
>>> device and not the Nubus itself. Also update the comment in the file header to
>>> reflect that mac-nubus-bridge is specific to the Macintosh.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>   hw/nubus/mac-nubus-bridge.c         | 8 +++++---
>>>   include/hw/nubus/mac-nubus-bridge.h | 4 ++--
>>>   2 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>
>> (it could be merged with previous one)
> 
> I like to try and keep renames on a per-device basis if possible, even if it's just to help rebasing
> during development.

Ok. No problem.

> Other than that, is there anything else outstanding you think would require a v6 series?

Except if you want to address comments from Philippe, I can collect this series and send a PR.

Thanks,
Laurent


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
  2021-09-23  9:42   ` Philippe Mathieu-Daudé
  2021-09-23 10:28   ` Laurent Vivier
@ 2021-09-23 14:16   ` BALATON Zoltan
  2021-09-24  7:16     ` Mark Cave-Ayland
  2 siblings, 1 reply; 52+ messages in thread
From: BALATON Zoltan @ 2021-09-23 14:16 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: qemu-devel, laurent

On Thu, 23 Sep 2021, Mark Cave-Ayland wrote:
> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
> for future Nubus devices to be plugged into arbitrary slots from the command line
> using a new qdev "slot" parameter for nubus devices.
>
> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".

Small typo: "a Macintosh machnies", either a or s is not needed.

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/nubus/mac-nubus-bridge.c         |  4 ++++
> hw/nubus/nubus-bus.c                |  5 +++--
> hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
> include/hw/nubus/mac-nubus-bridge.h |  4 ++++
> include/hw/nubus/nubus.h            | 13 ++++++------
> 5 files changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
> index 7c329300b8..3f075789e9 100644
> --- a/hw/nubus/mac-nubus-bridge.c
> +++ b/hw/nubus/mac-nubus-bridge.c
> @@ -18,6 +18,10 @@ static void mac_nubus_bridge_init(Object *obj)
>
>     s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
>
> +    /* Macintosh only has slots 0x9 to 0xe available */
> +    s->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
> +                                                  MAC_NUBUS_SLOT_NB);
> +
>     sysbus_init_mmio(sbd, &s->bus->super_slot_io);
>     sysbus_init_mmio(sbd, &s->bus->slot_io);
> }
> diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
> index f4410803ff..3cd7534864 100644
> --- a/hw/nubus/nubus-bus.c
> +++ b/hw/nubus/nubus-bus.c
> @@ -86,13 +86,14 @@ static void nubus_init(Object *obj)
>
>     memory_region_init_io(&nubus->super_slot_io, obj, &nubus_super_slot_ops,
>                           nubus, "nubus-super-slots",
> -                          NUBUS_SUPER_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
> +                          (NUBUS_SUPER_SLOT_NB + 1) * NUBUS_SUPER_SLOT_SIZE);
>
>     memory_region_init_io(&nubus->slot_io, obj, &nubus_slot_ops,
>                           nubus, "nubus-slots",
>                           NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
>
> -    nubus->current_slot = NUBUS_FIRST_SLOT;
> +    nubus->slot_available_mask = MAKE_64BIT_MASK(NUBUS_FIRST_SLOT,
> +                                                 NUBUS_SLOT_NB);
> }
>
> static void nubus_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index 4e23df1280..562650a05b 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -160,14 +160,28 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>     NubusDevice *nd = NUBUS_DEVICE(dev);
>     char *name;
>     hwaddr slot_offset;
> -
> -    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
> -            nubus->current_slot > NUBUS_LAST_SLOT) {
> -        error_setg(errp, "Cannot register nubus card, not enough slots");
> -        return;
> +    uint16_t s;
> +
> +    if (nd->slot == -1) {
> +        /* No slot specified, find first available free slot */
> +        s = ctz32(nubus->slot_available_mask);
> +        if (s != 32) {
> +            nd->slot = s;
> +        } else {
> +            error_setg(errp, "Cannot register nubus card, no free slot "
> +                             "available");
> +            return;
> +        }
> +    } else {
> +        /* Slot specified, make sure the slot is available */
> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
> +            error_setg(errp, "Cannot register nubus card, slot %d is "
> +                             "unavailable or already occupied", nd->slot);
> +            return;
> +        }
>     }
>
> -    nd->slot = nubus->current_slot++;
> +    nubus->slot_available_mask &= ~BIT(nd->slot);
>
>     /* Super */
>     slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
> @@ -191,12 +205,18 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>     nubus_register_format_block(nd);
> }
>
> +static Property nubus_device_properties[] = {
> +    DEFINE_PROP_INT32("slot", NubusDevice, slot, -1),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> static void nubus_device_class_init(ObjectClass *oc, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(oc);
>
>     dc->realize = nubus_device_realize;
>     dc->bus_type = TYPE_NUBUS_BUS;
> +    device_class_set_props(dc, nubus_device_properties);
> }
>
> static const TypeInfo nubus_device_type_info = {
> diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
> index 36aa098dd4..118d67267d 100644
> --- a/include/hw/nubus/mac-nubus-bridge.h
> +++ b/include/hw/nubus/mac-nubus-bridge.h
> @@ -12,6 +12,10 @@
> #include "hw/nubus/nubus.h"
> #include "qom/object.h"
>
> +#define MAC_NUBUS_FIRST_SLOT 0x9
> +#define MAC_NUBUS_LAST_SLOT  0xe
> +#define MAC_NUBUS_SLOT_NB    (MAC_NUBUS_LAST_SLOT - MAC_NUBUS_FIRST_SLOT + 1)
> +
> #define TYPE_MAC_NUBUS_BRIDGE "mac-nubus-bridge"
> OBJECT_DECLARE_SIMPLE_TYPE(MacNubusState, MAC_NUBUS_BRIDGE)
>
> diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
> index 89b0976aaa..988e4a2361 100644
> --- a/include/hw/nubus/nubus.h
> +++ b/include/hw/nubus/nubus.h
> @@ -14,13 +14,12 @@
> #include "qom/object.h"
>
> #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
> -#define NUBUS_SUPER_SLOT_NB   0x9
> +#define NUBUS_SUPER_SLOT_NB   0xe
>
> #define NUBUS_SLOT_SIZE       0x01000000
> -#define NUBUS_SLOT_NB         0xF
> -
> -#define NUBUS_FIRST_SLOT      0x9
> -#define NUBUS_LAST_SLOT       0xF
> +#define NUBUS_FIRST_SLOT      0x0
> +#define NUBUS_LAST_SLOT       0xf
> +#define NUBUS_SLOT_NB         (NUBUS_LAST_SLOT - NUBUS_FIRST_SLOT + 1)
>
> #define TYPE_NUBUS_DEVICE "nubus-device"
> OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
> @@ -36,13 +35,13 @@ struct NubusBus {
>     MemoryRegion super_slot_io;
>     MemoryRegion slot_io;
>
> -    int current_slot;
> +    uint32_t slot_available_mask;
> };
>
> struct NubusDevice {
>     DeviceState qdev;
>
> -    int slot;
> +    int32_t slot;

Why uint32_t? Considering its max value even uint8_t would be enough 
although maybe invalid value would be 255 instead of -1 then. As this was 
added in previous patch you could avoid churn here by introducing it with 
the right type in that patch already. (But feel free to ignore it if you 
have no time for more changes, the current version works so if you don't 
do another version for other reasons this probably don't worth the effort 
alone.)

Regards,
BALATON Zoltan


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

* Re: [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23 13:53       ` Laurent Vivier
@ 2021-09-23 16:03         ` Mark Cave-Ayland
  2021-09-24  6:37           ` Mark Cave-Ayland
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-23 16:03 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel

On 23/09/2021 14:53, Laurent Vivier wrote:

> Le 23/09/2021 à 14:50, Mark Cave-Ayland a écrit :
>> On 23/09/2021 11:35, Laurent Vivier wrote:
>>
>>> Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
>>>> This better reflects that the mac-nubus-bridge device is derived from the
>>>> nubus-bridge device, and that the structure represents the state of the bridge
>>>> device and not the Nubus itself. Also update the comment in the file header to
>>>> reflect that mac-nubus-bridge is specific to the Macintosh.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>    hw/nubus/mac-nubus-bridge.c         | 8 +++++---
>>>>    include/hw/nubus/mac-nubus-bridge.h | 4 ++--
>>>>    2 files changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>
>>>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>
>>> (it could be merged with previous one)
>>
>> I like to try and keep renames on a per-device basis if possible, even if it's just to help rebasing
>> during development.
> 
> Ok. No problem.
> 
>> Other than that, is there anything else outstanding you think would require a v6 series?
> 
> Except if you want to address comments from Philippe, I can collect this series and send a PR.

Okay I see there are a couple more comments on v5 - let me take a quick look.


ATB,

Mark.


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

* Re: [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
  2021-09-23 16:03         ` Mark Cave-Ayland
@ 2021-09-24  6:37           ` Mark Cave-Ayland
  0 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  6:37 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel

On 23/09/2021 17:03, Mark Cave-Ayland wrote:

> On 23/09/2021 14:53, Laurent Vivier wrote:
> 
>> Le 23/09/2021 à 14:50, Mark Cave-Ayland a écrit :
>>> On 23/09/2021 11:35, Laurent Vivier wrote:
>>>
>>>> Le 23/09/2021 à 11:13, Mark Cave-Ayland a écrit :
>>>>> This better reflects that the mac-nubus-bridge device is derived from the
>>>>> nubus-bridge device, and that the structure represents the state of the bridge
>>>>> device and not the Nubus itself. Also update the comment in the file header to
>>>>> reflect that mac-nubus-bridge is specific to the Macintosh.
>>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>>    hw/nubus/mac-nubus-bridge.c         | 8 +++++---
>>>>>    include/hw/nubus/mac-nubus-bridge.h | 4 ++--
>>>>>    2 files changed, 7 insertions(+), 5 deletions(-)
>>>>>
>>>>
>>>>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>>
>>>> (it could be merged with previous one)
>>>
>>> I like to try and keep renames on a per-device basis if possible, even if it's 
>>> just to help rebasing
>>> during development.
>>
>> Ok. No problem.
>>
>>> Other than that, is there anything else outstanding you think would require a v6 
>>> series?
>>
>> Except if you want to address comments from Philippe, I can collect this series and 
>> send a PR.
> 
> Okay I see there are a couple more comments on v5 - let me take a quick look.

There's probably just about enough minor comments to make it worth doing a v6, so I 
will update and resend.


ATB,

Mark.


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23  9:42   ` Philippe Mathieu-Daudé
@ 2021-09-24  7:00     ` Mark Cave-Ayland
  0 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  7:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 23/09/2021 10:42, Philippe Mathieu-Daudé wrote:
> On 9/23/21 11:12, Mark Cave-Ayland wrote:
>> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
>> for future Nubus devices to be plugged into arbitrary slots from the command line
>> using a new qdev "slot" parameter for nubus devices.
>>
>> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
>> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/nubus/mac-nubus-bridge.c         |  4 ++++
>>   hw/nubus/nubus-bus.c                |  5 +++--
>>   hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>>   include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>>   include/hw/nubus/nubus.h            | 13 ++++++------
>>   5 files changed, 43 insertions(+), 15 deletions(-)
> 
>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>> index 4e23df1280..562650a05b 100644
>> --- a/hw/nubus/nubus-device.c
>> +++ b/hw/nubus/nubus-device.c
>> @@ -160,14 +160,28 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>>       NubusDevice *nd = NUBUS_DEVICE(dev);
>>       char *name;
>>       hwaddr slot_offset;
>> -
>> -    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
>> -            nubus->current_slot > NUBUS_LAST_SLOT) {
>> -        error_setg(errp, "Cannot register nubus card, not enough slots");
>> -        return;
>> +    uint16_t s;
>> +
>> +    if (nd->slot == -1) {
>> +        /* No slot specified, find first available free slot */
>> +        s = ctz32(nubus->slot_available_mask);
> 
> Nitpicking:
> 
>             int s = ctz32(nubus->slot_available_mask);
> 
> Otherwise,
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I'll make sure this is included in v6.

>> +        if (s != 32) {
>> +            nd->slot = s;
>> +        } else {
>> +            error_setg(errp, "Cannot register nubus card, no free slot "
>> +                             "available");
>> +            return;
>> +        }
>> +    } else {
>> +        /* Slot specified, make sure the slot is available */
>> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
>> +            error_setg(errp, "Cannot register nubus card, slot %d is "
>> +                             "unavailable or already occupied", nd->slot);
>> +            return;
>> +        }
>>       }


ATB,

Mark.


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

* Re: [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()
  2021-09-23  9:45   ` Philippe Mathieu-Daudé
@ 2021-09-24  7:01     ` Mark Cave-Ayland
  0 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  7:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 23/09/2021 10:45, Philippe Mathieu-Daudé wrote:

> On 9/23/21 11:12, Mark Cave-Ayland wrote:
>> Allow Nubus to manage the slot allocations itself using the BusClass check_address()
>> virtual function rather than managing this during NubusDevice realize().
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   hw/nubus/nubus-bus.c    | 30 ++++++++++++++++++++++++++++++
>>   hw/nubus/nubus-device.c | 22 ----------------------
>>   2 files changed, 30 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
>> index 3cd7534864..d4daaa36f2 100644
>> --- a/hw/nubus/nubus-bus.c
>> +++ b/hw/nubus/nubus-bus.c
>> @@ -96,11 +96,41 @@ static void nubus_init(Object *obj)
>>                                                    NUBUS_SLOT_NB);
>>   }
>> +static bool nubus_check_address(BusState *bus, DeviceState *dev, Error **errp)
>> +{
>> +    NubusDevice *nd = NUBUS_DEVICE(dev);
>> +    NubusBus *nubus = NUBUS_BUS(bus);
>> +    uint16_t s;
>> +
>> +    if (nd->slot == -1) {
>> +        /* No slot specified, find first available free slot */
>> +        s = ctz32(nubus->slot_available_mask);
> 
> Same comment than previous patch:
> 
>             int s = ctz32(nubus->slot_available_mask);

... and same here.

> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +        if (s != 32) {
>> +            nd->slot = s;
>> +        } else {
>> +            error_setg(errp, "Cannot register nubus card, no free slot "
>> +                             "available");
>> +            return false;
>> +        }
>> +    } else {
>> +        /* Slot specified, make sure the slot is available */
>> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
>> +            error_setg(errp, "Cannot register nubus card, slot %d is "
>> +                             "unavailable or already occupied", nd->slot);
>> +            return false;
>> +        }
>> +    }
>> +
>> +    nubus->slot_available_mask &= ~BIT(nd->slot);
>> +    return true;
>> +}


ATB,

Mark.


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-23  9:49   ` Philippe Mathieu-Daudé
@ 2021-09-24  7:06     ` Mark Cave-Ayland
  2021-09-24  9:01       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  7:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:

> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>> Each Nubus slot has an IRQ line that can be used to request service from the
>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
>> gpios accordingly, and introduce a new nubus_set_irq() function that can be used
>> by Nubus devices to control the slot IRQ.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   hw/nubus/nubus-bridge.c  | 2 ++
>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>   include/hw/nubus/nubus.h | 6 ++++++
>>   3 files changed, 16 insertions(+)
> 
>>   static Property nubus_bridge_properties[] = {
>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>> index 280f40e88a..0f1852f671 100644
>> --- a/hw/nubus/nubus-device.c
>> +++ b/hw/nubus/nubus-device.c
>> @@ -10,12 +10,20 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/datadir.h"
>> +#include "hw/irq.h"
>>   #include "hw/loader.h"
>>   #include "hw/nubus/nubus.h"
>>   #include "qapi/error.h"
>>   #include "qemu/error-report.h"
>> +void nubus_set_irq(NubusDevice *nd, int level)
>> +{
>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>> +
> 
> A trace-event could be helpful here, otherwise:
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>> +}

I think adding a trace event here would just be too verbose (particularly if you have 
more than one nubus device) and not particularly helpful: normally the part you want 
to debug is the in the device itself looking at which constituent flags combine to 
raise/lower the interrupt line. And once you've done that then you've already got a 
suitable trace-event in place...


ATB,

Mark.


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

* Re: [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property
  2021-09-23  9:52   ` Philippe Mathieu-Daudé
@ 2021-09-24  7:09     ` Mark Cave-Ayland
  0 siblings, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  7:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 23/09/2021 10:52, Philippe Mathieu-Daudé wrote:

> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>> This is to allow Macintosh machines to further specify which slots are available
>> since the number of addressable slots may not match the number of physical slots
>> present in the machine.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   hw/nubus/nubus-bridge.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
> 
>> +static Property nubus_bridge_properties[] = {
>> +    DEFINE_PROP_UINT32("slot-available-mask", NubusBridge,
>> +                       bus.slot_available_mask, 0xffff),
> 
> What about using DEFINE_PROP_UINT16() here and uint16_t in
> patch 04/20 "nubus: use bitmap to manage available slots"?
> 
> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Yes, that seems to work fine so I'll make this change in v6.

>> +    DEFINE_PROP_END_OF_LIST()
>> +};


ATB,

Mark.


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-23 14:16   ` BALATON Zoltan
@ 2021-09-24  7:16     ` Mark Cave-Ayland
  2021-09-24  8:50       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  7:16 UTC (permalink / raw)
  To: BALATON Zoltan; +Cc: qemu-devel, laurent

On 23/09/2021 15:16, BALATON Zoltan wrote:

> On Thu, 23 Sep 2021, Mark Cave-Ayland wrote:
>> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
>> for future Nubus devices to be plugged into arbitrary slots from the command line
>> using a new qdev "slot" parameter for nubus devices.
>>
>> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
>> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".
> 
> Small typo: "a Macintosh machnies", either a or s is not needed.

Thanks - I've updated this for v6.

>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/nubus/mac-nubus-bridge.c         |  4 ++++
>> hw/nubus/nubus-bus.c                |  5 +++--
>> hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>> include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>> include/hw/nubus/nubus.h            | 13 ++++++------
>> 5 files changed, 43 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
>> index 7c329300b8..3f075789e9 100644
>> --- a/hw/nubus/mac-nubus-bridge.c
>> +++ b/hw/nubus/mac-nubus-bridge.c
>> @@ -18,6 +18,10 @@ static void mac_nubus_bridge_init(Object *obj)
>>
>>     s->bus = NUBUS_BUS(qbus_create(TYPE_NUBUS_BUS, DEVICE(s), NULL));
>>
>> +    /* Macintosh only has slots 0x9 to 0xe available */
>> +    s->bus->slot_available_mask = MAKE_64BIT_MASK(MAC_NUBUS_FIRST_SLOT,
>> +                                                  MAC_NUBUS_SLOT_NB);
>> +
>>     sysbus_init_mmio(sbd, &s->bus->super_slot_io);
>>     sysbus_init_mmio(sbd, &s->bus->slot_io);
>> }
>> diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
>> index f4410803ff..3cd7534864 100644
>> --- a/hw/nubus/nubus-bus.c
>> +++ b/hw/nubus/nubus-bus.c
>> @@ -86,13 +86,14 @@ static void nubus_init(Object *obj)
>>
>>     memory_region_init_io(&nubus->super_slot_io, obj, &nubus_super_slot_ops,
>>                           nubus, "nubus-super-slots",
>> -                          NUBUS_SUPER_SLOT_NB * NUBUS_SUPER_SLOT_SIZE);
>> +                          (NUBUS_SUPER_SLOT_NB + 1) * NUBUS_SUPER_SLOT_SIZE);
>>
>>     memory_region_init_io(&nubus->slot_io, obj, &nubus_slot_ops,
>>                           nubus, "nubus-slots",
>>                           NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
>>
>> -    nubus->current_slot = NUBUS_FIRST_SLOT;
>> +    nubus->slot_available_mask = MAKE_64BIT_MASK(NUBUS_FIRST_SLOT,
>> +                                                 NUBUS_SLOT_NB);
>> }
>>
>> static void nubus_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>> index 4e23df1280..562650a05b 100644
>> --- a/hw/nubus/nubus-device.c
>> +++ b/hw/nubus/nubus-device.c
>> @@ -160,14 +160,28 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>>     NubusDevice *nd = NUBUS_DEVICE(dev);
>>     char *name;
>>     hwaddr slot_offset;
>> -
>> -    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
>> -            nubus->current_slot > NUBUS_LAST_SLOT) {
>> -        error_setg(errp, "Cannot register nubus card, not enough slots");
>> -        return;
>> +    uint16_t s;
>> +
>> +    if (nd->slot == -1) {
>> +        /* No slot specified, find first available free slot */
>> +        s = ctz32(nubus->slot_available_mask);
>> +        if (s != 32) {
>> +            nd->slot = s;
>> +        } else {
>> +            error_setg(errp, "Cannot register nubus card, no free slot "
>> +                             "available");
>> +            return;
>> +        }
>> +    } else {
>> +        /* Slot specified, make sure the slot is available */
>> +        if (!(nubus->slot_available_mask & BIT(nd->slot))) {
>> +            error_setg(errp, "Cannot register nubus card, slot %d is "
>> +                             "unavailable or already occupied", nd->slot);
>> +            return;
>> +        }
>>     }
>>
>> -    nd->slot = nubus->current_slot++;
>> +    nubus->slot_available_mask &= ~BIT(nd->slot);
>>
>>     /* Super */
>>     slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
>> @@ -191,12 +205,18 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>>     nubus_register_format_block(nd);
>> }
>>
>> +static Property nubus_device_properties[] = {
>> +    DEFINE_PROP_INT32("slot", NubusDevice, slot, -1),
>> +    DEFINE_PROP_END_OF_LIST()
>> +};
>> +
>> static void nubus_device_class_init(ObjectClass *oc, void *data)
>> {
>>     DeviceClass *dc = DEVICE_CLASS(oc);
>>
>>     dc->realize = nubus_device_realize;
>>     dc->bus_type = TYPE_NUBUS_BUS;
>> +    device_class_set_props(dc, nubus_device_properties);
>> }
>>
>> static const TypeInfo nubus_device_type_info = {
>> diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
>> index 36aa098dd4..118d67267d 100644
>> --- a/include/hw/nubus/mac-nubus-bridge.h
>> +++ b/include/hw/nubus/mac-nubus-bridge.h
>> @@ -12,6 +12,10 @@
>> #include "hw/nubus/nubus.h"
>> #include "qom/object.h"
>>
>> +#define MAC_NUBUS_FIRST_SLOT 0x9
>> +#define MAC_NUBUS_LAST_SLOT  0xe
>> +#define MAC_NUBUS_SLOT_NB    (MAC_NUBUS_LAST_SLOT - MAC_NUBUS_FIRST_SLOT + 1)
>> +
>> #define TYPE_MAC_NUBUS_BRIDGE "mac-nubus-bridge"
>> OBJECT_DECLARE_SIMPLE_TYPE(MacNubusState, MAC_NUBUS_BRIDGE)
>>
>> diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
>> index 89b0976aaa..988e4a2361 100644
>> --- a/include/hw/nubus/nubus.h
>> +++ b/include/hw/nubus/nubus.h
>> @@ -14,13 +14,12 @@
>> #include "qom/object.h"
>>
>> #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
>> -#define NUBUS_SUPER_SLOT_NB   0x9
>> +#define NUBUS_SUPER_SLOT_NB   0xe
>>
>> #define NUBUS_SLOT_SIZE       0x01000000
>> -#define NUBUS_SLOT_NB         0xF
>> -
>> -#define NUBUS_FIRST_SLOT      0x9
>> -#define NUBUS_LAST_SLOT       0xF
>> +#define NUBUS_FIRST_SLOT      0x0
>> +#define NUBUS_LAST_SLOT       0xf
>> +#define NUBUS_SLOT_NB         (NUBUS_LAST_SLOT - NUBUS_FIRST_SLOT + 1)
>>
>> #define TYPE_NUBUS_DEVICE "nubus-device"
>> OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
>> @@ -36,13 +35,13 @@ struct NubusBus {
>>     MemoryRegion super_slot_io;
>>     MemoryRegion slot_io;
>>
>> -    int current_slot;
>> +    uint32_t slot_available_mask;
>> };
>>
>> struct NubusDevice {
>>     DeviceState qdev;
>>
>> -    int slot;
>> +    int32_t slot;
> 
> Why uint32_t? Considering its max value even uint8_t would be enough although maybe 
> invalid value would be 255 instead of -1 then. As this was added in previous patch 
> you could avoid churn here by introducing it with the right type in that patch 
> already. (But feel free to ignore it if you have no time for more changes, the 
> current version works so if you don't do another version for other reasons this 
> probably don't worth the effort alone.)

I think it makes sense to keep this signed since -1 is used for other bus 
implementations to indicate that an explicit slot hasn't been assigned. Potentially 
the slot number could be represented by an 8-bit value, however it seems there is no 
DEFINE_PROP_INT8 or DEFINE_PROP_INT16. Fortunately the slot number is restricted by 
the available slots bitmask anyhow, so this shouldn't be an issue.


ATB,

Mark.


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-24  7:16     ` Mark Cave-Ayland
@ 2021-09-24  8:50       ` Philippe Mathieu-Daudé
  2021-09-24 10:51         ` BALATON Zoltan
  0 siblings, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-24  8:50 UTC (permalink / raw)
  To: Mark Cave-Ayland, BALATON Zoltan; +Cc: qemu-devel, laurent

On 9/24/21 09:16, Mark Cave-Ayland wrote:
> On 23/09/2021 15:16, BALATON Zoltan wrote:
> 
>> On Thu, 23 Sep 2021, Mark Cave-Ayland wrote:
>>> Convert nubus_device_realize() to use a bitmap to manage available 
>>> slots to allow
>>> for future Nubus devices to be plugged into arbitrary slots from the 
>>> command line
>>> using a new qdev "slot" parameter for nubus devices.
>>>
>>> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a 
>>> Macintosh
>>> machines as documented in "Desigining Cards and Drivers for the 
>>> Macintosh Family".
>>
>> Small typo: "a Macintosh machnies", either a or s is not needed.
> 
> Thanks - I've updated this for v6.
> 
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> hw/nubus/mac-nubus-bridge.c         |  4 ++++
>>> hw/nubus/nubus-bus.c                |  5 +++--
>>> hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>>> include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>>> include/hw/nubus/nubus.h            | 13 ++++++------
>>> 5 files changed, 43 insertions(+), 15 deletions(-)

>>> struct NubusDevice {
>>>     DeviceState qdev;
>>>
>>> -    int slot;
>>> +    int32_t slot;
>>
>> Why uint32_t? Considering its max value even uint8_t would be enough 
>> although maybe invalid value would be 255 instead of -1 then. As this 
>> was added in previous patch you could avoid churn here by introducing 
>> it with the right type in that patch already. (But feel free to ignore 
>> it if you have no time for more changes, the current version works so 
>> if you don't do another version for other reasons this probably don't 
>> worth the effort alone.)
> 
> I think it makes sense to keep this signed since -1 is used for other 
> bus implementations to indicate that an explicit slot hasn't been 
> assigned. Potentially the slot number could be represented by an 8-bit 
> value, however it seems there is no DEFINE_PROP_INT8 or 
> DEFINE_PROP_INT16. Fortunately the slot number is restricted by the 
> available slots bitmask anyhow, so this shouldn't be an issue.

I wondered the same and noticed there is no DEFINE_PROP_INT8, so didn't
want to bother Mark furthermore :) Adding & using DEFINE_PROP_INT8 seems
a good idea, but to be fair with the repository we'd need to audit the
other places where DEFINE_PROP_INT32 isn't justified and update. Extra
work for not much gain, so I'm find with this patch. Can be improved on
top.


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-24  7:06     ` Mark Cave-Ayland
@ 2021-09-24  9:01       ` Philippe Mathieu-Daudé
  2021-09-24  9:05         ` Philippe Mathieu-Daudé
  2021-09-24  9:43         ` Mark Cave-Ayland
  0 siblings, 2 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-24  9:01 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/24/21 09:06, Mark Cave-Ayland wrote:
> On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:
> 
>> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>>> Each Nubus slot has an IRQ line that can be used to request service 
>>> from the
>>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired 
>>> up using qdev
>>> gpios accordingly, and introduce a new nubus_set_irq() function that 
>>> can be used
>>> by Nubus devices to control the slot IRQ.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>   hw/nubus/nubus-bridge.c  | 2 ++
>>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>>   include/hw/nubus/nubus.h | 6 ++++++
>>>   3 files changed, 16 insertions(+)
>>
>>>   static Property nubus_bridge_properties[] = {
>>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>>> index 280f40e88a..0f1852f671 100644
>>> --- a/hw/nubus/nubus-device.c
>>> +++ b/hw/nubus/nubus-device.c
>>> @@ -10,12 +10,20 @@
>>>   #include "qemu/osdep.h"
>>>   #include "qemu/datadir.h"
>>> +#include "hw/irq.h"
>>>   #include "hw/loader.h"
>>>   #include "hw/nubus/nubus.h"
>>>   #include "qapi/error.h"
>>>   #include "qemu/error-report.h"
>>> +void nubus_set_irq(NubusDevice *nd, int level)
>>> +{
>>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>>> +
>>
>> A trace-event could be helpful here, otherwise:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>>> +}
> 
> I think adding a trace event here would just be too verbose 
> (particularly if you have more than one nubus device) and not 
> particularly helpful: normally the part you want to debug is the in the 
> device itself looking at which constituent flags combine to raise/lower 
> the interrupt line. And once you've done that then you've already got a 
> suitable trace-event in place...

But devices accessing the bus are not aware of which devices are plugged
onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
bus, to propagate the interrupt up to the CPU? OK so then the trace
event is irrelevant indeed. I understood this API as any external device
could trigger an IRQ to device on the bus. I wonder if renaming as
nubus_device_set_irq() could make it clearer. Or even simpler, add
a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
to avoid any confusion, and we are good.


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-24  9:01       ` Philippe Mathieu-Daudé
@ 2021-09-24  9:05         ` Philippe Mathieu-Daudé
  2021-09-29  6:42           ` Mark Cave-Ayland
  2021-09-24  9:43         ` Mark Cave-Ayland
  1 sibling, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-24  9:05 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, laurent

On 9/24/21 11:01, Philippe Mathieu-Daudé wrote:
> On 9/24/21 09:06, Mark Cave-Ayland wrote:
>> On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:
>>
>>> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>>>> Each Nubus slot has an IRQ line that can be used to request service 
>>>> from the
>>>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired 
>>>> up using qdev
>>>> gpios accordingly, and introduce a new nubus_set_irq() function that 
>>>> can be used
>>>> by Nubus devices to control the slot IRQ.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>>   hw/nubus/nubus-bridge.c  | 2 ++
>>>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>>>   include/hw/nubus/nubus.h | 6 ++++++
>>>>   3 files changed, 16 insertions(+)
>>>
>>>>   static Property nubus_bridge_properties[] = {
>>>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>>>> index 280f40e88a..0f1852f671 100644
>>>> --- a/hw/nubus/nubus-device.c
>>>> +++ b/hw/nubus/nubus-device.c
>>>> @@ -10,12 +10,20 @@
>>>>   #include "qemu/osdep.h"
>>>>   #include "qemu/datadir.h"
>>>> +#include "hw/irq.h"
>>>>   #include "hw/loader.h"
>>>>   #include "hw/nubus/nubus.h"
>>>>   #include "qapi/error.h"
>>>>   #include "qemu/error-report.h"
>>>> +void nubus_set_irq(NubusDevice *nd, int level)
>>>> +{
>>>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>>>> +
>>>
>>> A trace-event could be helpful here, otherwise:
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>>>> +}
>>
>> I think adding a trace event here would just be too verbose 
>> (particularly if you have more than one nubus device) and not 
>> particularly helpful: normally the part you want to debug is the in 
>> the device itself looking at which constituent flags combine to 
>> raise/lower the interrupt line. And once you've done that then you've 
>> already got a suitable trace-event in place...
> 
> But devices accessing the bus are not aware of which devices are plugged
> onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
> bus, to propagate the interrupt up to the CPU? OK so then the trace
> event is irrelevant indeed. I understood this API as any external device
> could trigger an IRQ to device on the bus. I wonder if renaming as
> nubus_device_set_irq() could make it clearer. Or even simpler, add
> a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
> to avoid any confusion, and we are good.

Just noticed v6 was sent, so the function description could either
- sent as reply to v6 patch and squashed by Laurent when applying
- sent later as a new cleanup patch on top
- never added

Up to you, I don't mind mind much the outcome.


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-24  9:01       ` Philippe Mathieu-Daudé
  2021-09-24  9:05         ` Philippe Mathieu-Daudé
@ 2021-09-24  9:43         ` Mark Cave-Ayland
  1 sibling, 0 replies; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-24  9:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 24/09/2021 10:01, Philippe Mathieu-Daudé wrote:
> On 9/24/21 09:06, Mark Cave-Ayland wrote:
>> On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:
>>
>>> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>>>> Each Nubus slot has an IRQ line that can be used to request service from the
>>>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
>>>> gpios accordingly, and introduce a new nubus_set_irq() function that can be used
>>>> by Nubus devices to control the slot IRQ.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>>   hw/nubus/nubus-bridge.c  | 2 ++
>>>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>>>   include/hw/nubus/nubus.h | 6 ++++++
>>>>   3 files changed, 16 insertions(+)
>>>
>>>>   static Property nubus_bridge_properties[] = {
>>>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>>>> index 280f40e88a..0f1852f671 100644
>>>> --- a/hw/nubus/nubus-device.c
>>>> +++ b/hw/nubus/nubus-device.c
>>>> @@ -10,12 +10,20 @@
>>>>   #include "qemu/osdep.h"
>>>>   #include "qemu/datadir.h"
>>>> +#include "hw/irq.h"
>>>>   #include "hw/loader.h"
>>>>   #include "hw/nubus/nubus.h"
>>>>   #include "qapi/error.h"
>>>>   #include "qemu/error-report.h"
>>>> +void nubus_set_irq(NubusDevice *nd, int level)
>>>> +{
>>>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>>>> +
>>>
>>> A trace-event could be helpful here, otherwise:
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>>>> +}
>>
>> I think adding a trace event here would just be too verbose (particularly if you 
>> have more than one nubus device) and not particularly helpful: normally the part 
>> you want to debug is the in the device itself looking at which constituent flags 
>> combine to raise/lower the interrupt line. And once you've done that then you've 
>> already got a suitable trace-event in place...
> 
> But devices accessing the bus are not aware of which devices are plugged
> onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
> bus, to propagate the interrupt up to the CPU?

Yes indeed, that is correct.

> OK so then the trace
> event is irrelevant indeed. I understood this API as any external device
> could trigger an IRQ to device on the bus. I wonder if renaming as
> nubus_device_set_irq() could make it clearer. Or even simpler, add
> a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
> to avoid any confusion, and we are good.

The function name and signature nubus_set_irq(NubusDevice *nd, int level) was chosen 
because it was intended to be the Nubus equivalent of PCI's pci_set_irq(PCIDevice *d, 
int level) where the first parameter of both functions nicely indicates that this is 
to be called by the device. I don't think we've had any reports of confusion with the 
existing pci_set_irq() function usage?

Another thing that makes me less worried is that the next series after this contains 
a number of changes for the macfb device, including the addition of VBL interrupts 
implemented using nubus_set_irq() so at that point there will be a grep-able usage 
example in the codebase.


ATB,

Mark.


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

* Re: [PATCH v5 04/20] nubus: use bitmap to manage available slots
  2021-09-24  8:50       ` Philippe Mathieu-Daudé
@ 2021-09-24 10:51         ` BALATON Zoltan
  0 siblings, 0 replies; 52+ messages in thread
From: BALATON Zoltan @ 2021-09-24 10:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Mark Cave-Ayland, qemu-devel, laurent

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

On Fri, 24 Sep 2021, Philippe Mathieu-Daudé wrote:
> On 9/24/21 09:16, Mark Cave-Ayland wrote:
>> On 23/09/2021 15:16, BALATON Zoltan wrote:
>> 
>>> On Thu, 23 Sep 2021, Mark Cave-Ayland wrote:
>>>> Convert nubus_device_realize() to use a bitmap to manage available slots 
>>>> to allow
>>>> for future Nubus devices to be plugged into arbitrary slots from the 
>>>> command line
>>>> using a new qdev "slot" parameter for nubus devices.
>>>> 
>>>> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a 
>>>> Macintosh
>>>> machines as documented in "Desigining Cards and Drivers for the Macintosh 
>>>> Family".
>>> 
>>> Small typo: "a Macintosh machnies", either a or s is not needed.
>> 
>> Thanks - I've updated this for v6.
>> 
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> ---
>>>> hw/nubus/mac-nubus-bridge.c         |  4 ++++
>>>> hw/nubus/nubus-bus.c                |  5 +++--
>>>> hw/nubus/nubus-device.c             | 32 +++++++++++++++++++++++------
>>>> include/hw/nubus/mac-nubus-bridge.h |  4 ++++
>>>> include/hw/nubus/nubus.h            | 13 ++++++------
>>>> 5 files changed, 43 insertions(+), 15 deletions(-)
>
>>>> struct NubusDevice {
>>>>     DeviceState qdev;
>>>> 
>>>> -    int slot;
>>>> +    int32_t slot;
>>> 
>>> Why uint32_t? Considering its max value even uint8_t would be enough 
>>> although maybe invalid value would be 255 instead of -1 then. As this was 
>>> added in previous patch you could avoid churn here by introducing it with 
>>> the right type in that patch already. (But feel free to ignore it if you 
>>> have no time for more changes, the current version works so if you don't 
>>> do another version for other reasons this probably don't worth the effort 
>>> alone.)
>> 
>> I think it makes sense to keep this signed since -1 is used for other bus 
>> implementations to indicate that an explicit slot hasn't been assigned. 
>> Potentially the slot number could be represented by an 8-bit value, however 
>> it seems there is no DEFINE_PROP_INT8 or DEFINE_PROP_INT16. Fortunately the 
>> slot number is restricted by the available slots bitmask anyhow, so this 
>> shouldn't be an issue.
>
> I wondered the same and noticed there is no DEFINE_PROP_INT8, so didn't
> want to bother Mark furthermore :) Adding & using DEFINE_PROP_INT8 seems
> a good idea, but to be fair with the repository we'd need to audit the
> other places where DEFINE_PROP_INT32 isn't justified and update. Extra
> work for not much gain, so I'm find with this patch. Can be improved on
> top.

That's why I said UINT8 for prop and treat -1 as 0xff but I agree this is 
not a big deal so I've also said I'm OK with the current version. If it 
would be more effort than Mark is willing to put in this now I can 
understand that and not pushing it. It's not something that's wrong or 
worth holding the series back for just a possible minor improvement to 
avoid some code churn.

Regards,
BALATON Zoltan

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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-24  9:05         ` Philippe Mathieu-Daudé
@ 2021-09-29  6:42           ` Mark Cave-Ayland
  2021-09-29  8:38             ` Laurent Vivier
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Cave-Ayland @ 2021-09-29  6:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel, laurent

On 24/09/2021 10:05, Philippe Mathieu-Daudé wrote:

> On 9/24/21 11:01, Philippe Mathieu-Daudé wrote:
>> On 9/24/21 09:06, Mark Cave-Ayland wrote:
>>> On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:
>>>
>>>> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>>>>> Each Nubus slot has an IRQ line that can be used to request service from the
>>>>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
>>>>> gpios accordingly, and introduce a new nubus_set_irq() function that can be used
>>>>> by Nubus devices to control the slot IRQ.
>>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>>> ---
>>>>>   hw/nubus/nubus-bridge.c  | 2 ++
>>>>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>>>>   include/hw/nubus/nubus.h | 6 ++++++
>>>>>   3 files changed, 16 insertions(+)
>>>>
>>>>>   static Property nubus_bridge_properties[] = {
>>>>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>>>>> index 280f40e88a..0f1852f671 100644
>>>>> --- a/hw/nubus/nubus-device.c
>>>>> +++ b/hw/nubus/nubus-device.c
>>>>> @@ -10,12 +10,20 @@
>>>>>   #include "qemu/osdep.h"
>>>>>   #include "qemu/datadir.h"
>>>>> +#include "hw/irq.h"
>>>>>   #include "hw/loader.h"
>>>>>   #include "hw/nubus/nubus.h"
>>>>>   #include "qapi/error.h"
>>>>>   #include "qemu/error-report.h"
>>>>> +void nubus_set_irq(NubusDevice *nd, int level)
>>>>> +{
>>>>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>>>>> +
>>>>
>>>> A trace-event could be helpful here, otherwise:
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>
>>>>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>>>>> +}
>>>
>>> I think adding a trace event here would just be too verbose (particularly if you 
>>> have more than one nubus device) and not particularly helpful: normally the part 
>>> you want to debug is the in the device itself looking at which constituent flags 
>>> combine to raise/lower the interrupt line. And once you've done that then you've 
>>> already got a suitable trace-event in place...
>>
>> But devices accessing the bus are not aware of which devices are plugged
>> onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
>> bus, to propagate the interrupt up to the CPU? OK so then the trace
>> event is irrelevant indeed. I understood this API as any external device
>> could trigger an IRQ to device on the bus. I wonder if renaming as
>> nubus_device_set_irq() could make it clearer. Or even simpler, add
>> a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
>> to avoid any confusion, and we are good.
> 
> Just noticed v6 was sent, so the function description could either
> - sent as reply to v6 patch and squashed by Laurent when applying
> - sent later as a new cleanup patch on top
> - never added
> 
> Up to you, I don't mind mind much the outcome.

I'm happy enough with the current version given the existing precedent of 
pci_set_irq() and that the next set of q800 patches will make use of nubus_set_irq() 
to provide a working reference in-tree.

Laurent, do you have any further comments on the series?


ATB,

Mark.


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

* Re: [PATCH v5 18/20] nubus: add support for slot IRQs
  2021-09-29  6:42           ` Mark Cave-Ayland
@ 2021-09-29  8:38             ` Laurent Vivier
  0 siblings, 0 replies; 52+ messages in thread
From: Laurent Vivier @ 2021-09-29  8:38 UTC (permalink / raw)
  To: Mark Cave-Ayland, Philippe Mathieu-Daudé, qemu-devel

Le 29/09/2021 à 08:42, Mark Cave-Ayland a écrit :
> On 24/09/2021 10:05, Philippe Mathieu-Daudé wrote:
> 
>> On 9/24/21 11:01, Philippe Mathieu-Daudé wrote:
>>> On 9/24/21 09:06, Mark Cave-Ayland wrote:
>>>> On 23/09/2021 10:49, Philippe Mathieu-Daudé wrote:
>>>>
>>>>> On 9/23/21 11:13, Mark Cave-Ayland wrote:
>>>>>> Each Nubus slot has an IRQ line that can be used to request service from the
>>>>>> CPU. Connect the IRQs to the Nubus bridge so that they can be wired up using qdev
>>>>>> gpios accordingly, and introduce a new nubus_set_irq() function that can be used
>>>>>> by Nubus devices to control the slot IRQ.
>>>>>>
>>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>>>> ---
>>>>>>   hw/nubus/nubus-bridge.c  | 2 ++
>>>>>>   hw/nubus/nubus-device.c  | 8 ++++++++
>>>>>>   include/hw/nubus/nubus.h | 6 ++++++
>>>>>>   3 files changed, 16 insertions(+)
>>>>>
>>>>>>   static Property nubus_bridge_properties[] = {
>>>>>> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>>>>>> index 280f40e88a..0f1852f671 100644
>>>>>> --- a/hw/nubus/nubus-device.c
>>>>>> +++ b/hw/nubus/nubus-device.c
>>>>>> @@ -10,12 +10,20 @@
>>>>>>   #include "qemu/osdep.h"
>>>>>>   #include "qemu/datadir.h"
>>>>>> +#include "hw/irq.h"
>>>>>>   #include "hw/loader.h"
>>>>>>   #include "hw/nubus/nubus.h"
>>>>>>   #include "qapi/error.h"
>>>>>>   #include "qemu/error-report.h"
>>>>>> +void nubus_set_irq(NubusDevice *nd, int level)
>>>>>> +{
>>>>>> +    NubusBus *nubus = NUBUS_BUS(qdev_get_parent_bus(DEVICE(nd)));
>>>>>> +
>>>>>
>>>>> A trace-event could be helpful here, otherwise:
>>>>>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>
>>>>>> +    qemu_set_irq(nubus->irqs[nd->slot], level);
>>>>>> +}
>>>>
>>>> I think adding a trace event here would just be too verbose (particularly if you have more than
>>>> one nubus device) and not particularly helpful: normally the part you want to debug is the in
>>>> the device itself looking at which constituent flags combine to raise/lower the interrupt line.
>>>> And once you've done that then you've already got a suitable trace-event in place...
>>>
>>> But devices accessing the bus are not aware of which devices are plugged
>>> onto it. Wait, what is suppose to call nubus_set_irq()? Devices on the
>>> bus, to propagate the interrupt up to the CPU? OK so then the trace
>>> event is irrelevant indeed. I understood this API as any external device
>>> could trigger an IRQ to device on the bus. I wonder if renaming as
>>> nubus_device_set_irq() could make it clearer. Or even simpler, add
>>> a comment in "hw/nubus/nubus.h" explaining what nubus_set_irq() is for
>>> to avoid any confusion, and we are good.
>>
>> Just noticed v6 was sent, so the function description could either
>> - sent as reply to v6 patch and squashed by Laurent when applying
>> - sent later as a new cleanup patch on top
>> - never added
>>
>> Up to you, I don't mind mind much the outcome.
> 
> I'm happy enough with the current version given the existing precedent of pci_set_irq() and that the
> next set of q800 patches will make use of nubus_set_irq() to provide a working reference in-tree.
> 
> Laurent, do you have any further comments on the series?

No, I'm going to queue the v6 to my branch and send the PR.

Thanks,
Laurent


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

end of thread, other threads:[~2021-09-29  8:41 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23  9:12 [PATCH v5 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 01/20] nubus: add comment indicating reference documents Mark Cave-Ayland
2021-09-23  9:35   ` Philippe Mathieu-Daudé
2021-09-23 10:01   ` Laurent Vivier
2021-09-23  9:12 ` [PATCH v5 02/20] nubus-device: rename slot_nb variable to slot Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 03/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
2021-09-23 10:04   ` Laurent Vivier
2021-09-23  9:12 ` [PATCH v5 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
2021-09-23  9:42   ` Philippe Mathieu-Daudé
2021-09-24  7:00     ` Mark Cave-Ayland
2021-09-23 10:28   ` Laurent Vivier
2021-09-23 14:16   ` BALATON Zoltan
2021-09-24  7:16     ` Mark Cave-Ayland
2021-09-24  8:50       ` Philippe Mathieu-Daudé
2021-09-24 10:51         ` BALATON Zoltan
2021-09-23  9:12 ` [PATCH v5 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
2021-09-23  9:45   ` Philippe Mathieu-Daudé
2021-09-24  7:01     ` Mark Cave-Ayland
2021-09-23 10:30   ` Laurent Vivier
2021-09-23  9:12 ` [PATCH v5 06/20] nubus: implement BusClass get_dev_path() Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 07/20] nubus: add trace-events for empty slot accesses Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 08/20] nubus: generate bus error when attempting to access empty slots Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 09/20] macfb: don't register declaration ROM Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
2021-09-23  9:12 ` [PATCH v5 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
2021-09-23  9:13 ` [PATCH v5 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
2021-09-23 10:33   ` Laurent Vivier
2021-09-23  9:13 ` [PATCH v5 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
2021-09-23 10:35   ` Laurent Vivier
2021-09-23  9:13 ` [PATCH v5 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
2021-09-23 10:35   ` Laurent Vivier
2021-09-23 12:50     ` Mark Cave-Ayland
2021-09-23 13:53       ` Laurent Vivier
2021-09-23 16:03         ` Mark Cave-Ayland
2021-09-24  6:37           ` Mark Cave-Ayland
2021-09-23  9:13 ` [PATCH v5 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
2021-09-23 10:36   ` Laurent Vivier
2021-09-23  9:13 ` [PATCH v5 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
2021-09-23 10:37   ` Laurent Vivier
2021-09-23  9:13 ` [PATCH v5 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
2021-09-23  9:52   ` Philippe Mathieu-Daudé
2021-09-24  7:09     ` Mark Cave-Ayland
2021-09-23  9:13 ` [PATCH v5 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
2021-09-23  9:49   ` Philippe Mathieu-Daudé
2021-09-24  7:06     ` Mark Cave-Ayland
2021-09-24  9:01       ` Philippe Mathieu-Daudé
2021-09-24  9:05         ` Philippe Mathieu-Daudé
2021-09-29  6:42           ` Mark Cave-Ayland
2021-09-29  8:38             ` Laurent Vivier
2021-09-24  9:43         ` Mark Cave-Ayland
2021-09-23  9:13 ` [PATCH v5 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
2021-09-23  9:13 ` [PATCH v5 20/20] q800: configure nubus available slots for Quadra 800 Mark Cave-Ayland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.