* [PATCH v3 01/20] nubus-device: rename slot_nb variable to slot
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 02/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
` (18 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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] 34+ messages in thread* [PATCH v3 02/20] nubus-device: expose separate super slot memory region
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 01/20] nubus-device: rename slot_nb variable to slot Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 03/20] nubus-device: add device slot parameter Mark Cave-Ayland
` (17 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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..36203848e5 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 - 6) * 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] 34+ messages in thread* [PATCH v3 03/20] nubus-device: add device slot parameter
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 01/20] nubus-device: rename slot_nb variable to slot Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 02/20] nubus-device: expose separate super slot memory region Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
` (16 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 UTC (permalink / raw)
To: qemu-devel, laurent
This prepares for allowing Nubus devices to be placed in a specific slot instead
of always being auto-allocated by the bus itself.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/nubus/nubus-device.c | 6 ++++++
include/hw/nubus/nubus.h | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 36203848e5..c1832f73da 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -191,12 +191,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/nubus.h b/include/hw/nubus/nubus.h
index 89b0976aaa..357f621d15 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;
+ int32_t slot;
MemoryRegion super_slot_mem;
MemoryRegion slot_mem;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 04/20] nubus: use bitmap to manage available slots
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (2 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 03/20] nubus-device: add device slot parameter Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:36 ` Philippe Mathieu-Daudé
2021-09-16 10:05 ` [PATCH v3 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
` (15 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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.
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 | 3 +++
hw/nubus/nubus-bus.c | 2 +-
hw/nubus/nubus-device.c | 29 +++++++++++++++++++++++++----
include/hw/nubus/nubus.h | 4 ++--
4 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 7c329300b8..c1d77e2bc7 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -18,6 +18,9 @@ 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(9, 6);
+
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 5c13452308..404c1032e0 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -84,7 +84,7 @@ static void nubus_init(Object *obj)
nubus, "nubus-slots",
NUBUS_SLOT_NB * NUBUS_SLOT_SIZE);
- nubus->current_slot = NUBUS_FIRST_SLOT;
+ nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
}
static void nubus_class_init(ObjectClass *oc, void *data)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index c1832f73da..d91a1e4af3 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -160,14 +160,35 @@ 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;
+ }
+ }
- if (nubus->current_slot < NUBUS_FIRST_SLOT ||
- nubus->current_slot > NUBUS_LAST_SLOT) {
- error_setg(errp, "Cannot register nubus card, not enough slots");
+ if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
+ error_setg(errp, "Cannot register nubus card, slot must be "
+ "between %d and %d", NUBUS_FIRST_SLOT,
+ NUBUS_LAST_SLOT);
return;
}
- nd->slot = nubus->current_slot++;
+ nubus->slot_available_mask &= ~BIT(nd->slot);
/* Super */
slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
index 357f621d15..8ff4736259 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -19,7 +19,7 @@
#define NUBUS_SLOT_SIZE 0x01000000
#define NUBUS_SLOT_NB 0xF
-#define NUBUS_FIRST_SLOT 0x9
+#define NUBUS_FIRST_SLOT 0x0
#define NUBUS_LAST_SLOT 0xF
#define TYPE_NUBUS_DEVICE "nubus-device"
@@ -36,7 +36,7 @@ struct NubusBus {
MemoryRegion super_slot_io;
MemoryRegion slot_io;
- int current_slot;
+ uint32_t slot_available_mask;
};
struct NubusDevice {
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 04/20] nubus: use bitmap to manage available slots
2021-09-16 10:05 ` [PATCH v3 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
@ 2021-09-16 10:36 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-16 10:36 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, laurent
On 9/16/21 12:05 PM, 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.
>
> 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 | 3 +++
> hw/nubus/nubus-bus.c | 2 +-
> hw/nubus/nubus-device.c | 29 +++++++++++++++++++++++++----
> include/hw/nubus/nubus.h | 4 ++--
> 4 files changed, 31 insertions(+), 7 deletions(-)
> - if (nubus->current_slot < NUBUS_FIRST_SLOT ||
> - nubus->current_slot > NUBUS_LAST_SLOT) {
> - error_setg(errp, "Cannot register nubus card, not enough slots");
> + if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
> + error_setg(errp, "Cannot register nubus card, slot must be "
> + "between %d and %d", NUBUS_FIRST_SLOT,
> + NUBUS_LAST_SLOT);
> return;
> }
>
> - nd->slot = nubus->current_slot++;
> + nubus->slot_available_mask &= ~BIT(nd->slot);
Having slot_available_mask an unsigned long (instead of
uint32_t) we could use the clearer:
clear_bit(nd->slot, &nubus->slot_available_mask);
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> /* Super */
> slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
> diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
> index 357f621d15..8ff4736259 100644
> --- a/include/hw/nubus/nubus.h
> +++ b/include/hw/nubus/nubus.h
> #define TYPE_NUBUS_DEVICE "nubus-device"
> @@ -36,7 +36,7 @@ struct NubusBus {
> MemoryRegion super_slot_io;
> MemoryRegion slot_io;
>
> - int current_slot;
> + uint32_t slot_available_mask;
> };
>
> struct NubusDevice {
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address()
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (3 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 04/20] nubus: use bitmap to manage available slots Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:37 ` Philippe Mathieu-Daudé
2021-09-16 10:05 ` [PATCH v3 06/20] nubus: implement BusClass get_dev_path() Mark Cave-Ayland
` (14 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
hw/nubus/nubus-bus.c | 37 +++++++++++++++++++++++++++++++++++++
hw/nubus/nubus-device.c | 29 -----------------------------
2 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/hw/nubus/nubus-bus.c b/hw/nubus/nubus-bus.c
index 404c1032e0..141876b579 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -87,11 +87,48 @@ static void nubus_init(Object *obj)
nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
}
+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;
+ }
+ }
+
+ if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
+ error_setg(errp, "Cannot register nubus card, slot must be "
+ "between %d and %d", NUBUS_FIRST_SLOT,
+ NUBUS_LAST_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 d91a1e4af3..7a32c8c95b 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -160,35 +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;
- }
- }
-
- if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
- error_setg(errp, "Cannot register nubus card, slot must be "
- "between %d and %d", NUBUS_FIRST_SLOT,
- NUBUS_LAST_SLOT);
- return;
- }
-
- nubus->slot_available_mask &= ~BIT(nd->slot);
/* Super */
slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 06/20] nubus: implement BusClass get_dev_path()
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (4 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 05/20] nubus: move slot bitmap checks from NubusDevice realize() to BusClass check_address() Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 07/20] nubus: add trace-events for empty slot accesses Mark Cave-Ayland
` (13 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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 141876b579..63e7d66b95 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -87,6 +87,21 @@ static void nubus_init(Object *obj)
nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
}
+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);
@@ -129,6 +144,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] 34+ messages in thread* [PATCH v3 07/20] nubus: add trace-events for empty slot accesses
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (5 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 06/20] nubus: implement BusClass get_dev_path() Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 08/20] nubus: generate bus error when attempting to access empty slots Mark Cave-Ayland
` (12 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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 63e7d66b95..39182db065 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "hw/nubus/nubus.h"
#include "qapi/error.h"
+#include "trace.h"
static NubusBus *nubus_find(void)
@@ -23,12 +24,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;
}
@@ -38,7 +40,7 @@ static const MemoryRegionOps nubus_slot_ops = {
.endianness = DEVICE_BIG_ENDIAN,
.valid = {
.min_access_size = 1,
- .max_access_size = 1,
+ .max_access_size = 4,
},
};
@@ -46,11 +48,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;
}
@@ -60,7 +64,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 2711cbb789..612ce671b5 100644
--- a/meson.build
+++ b/meson.build
@@ -2135,6 +2135,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] 34+ messages in thread* [PATCH v3 08/20] nubus: generate bus error when attempting to access empty slots
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (6 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 07/20] nubus: add trace-events for empty slot accesses Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 09/20] macfb: don't register declaration ROM Mark Cave-Ayland
` (11 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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 39182db065..a617459a4f 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -20,23 +20,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,
@@ -44,23 +44,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] 34+ messages in thread* [PATCH v3 09/20] macfb: don't register declaration ROM
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (7 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 08/20] nubus: generate bus error when attempting to access empty slots Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
` (10 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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] 34+ messages in thread* [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block()
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (8 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 09/20] macfb: don't register declaration ROM Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-19 21:10 ` Philippe Mathieu-Daudé
2021-09-16 10:05 ` [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
` (9 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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 7a32c8c95b..9c1992ceb0 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 8ff4736259..87a97516c7 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -45,25 +45,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] 34+ messages in thread* Re: [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block()
2021-09-16 10:05 ` [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
@ 2021-09-19 21:10 ` Philippe Mathieu-Daudé
2021-09-21 7:24 ` Mark Cave-Ayland
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-19 21:10 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, laurent
On 9/16/21 12:05, Mark Cave-Ayland wrote:
> 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.
Nubus neophyte question: is it OK to load a device without
optional ROM nor dummy?
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/nubus/nubus-device.c | 143 ---------------------------------------
> include/hw/nubus/nubus.h | 19 ------
> 2 files changed, 162 deletions(-)
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block()
2021-09-19 21:10 ` Philippe Mathieu-Daudé
@ 2021-09-21 7:24 ` Mark Cave-Ayland
0 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-21 7:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, laurent
On 19/09/2021 22:10, Philippe Mathieu-Daudé wrote:
> On 9/16/21 12:05, Mark Cave-Ayland wrote:
>> 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.
>
> Nubus neophyte question: is it OK to load a device without
> optional ROM nor dummy?
Yes, since the physical existence of the card is determined by whether the card
accesses generate a bus fault, as per the Apple documentation, which is fixed in
patch 8. The macfb device is an example of this since at least for the Quadra ROM its
resources are not on the card itself, but stored in the MacOS toolbox ROM (normally
referenced as slot 0xf).
Having said that, without a declaration ROM being present then your driver would have
to be able to identify the card the hard way, and at least Linux attempts to parse
the MacOS resources during enumeration. So I'd expect declaration ROMs to be required
for nubus devices that don't have corresponding drivers included in a standard MacOS
toolbox ROM which is likely to be the majority of cases.
ATB,
Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (9 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 10/20] nubus-device: remove nubus_register_rom() and nubus_register_format_block() Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 11:05 ` Philippe Mathieu-Daudé
2021-09-16 10:05 ` [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
` (8 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
hw/nubus/nubus-device.c | 43 +++++++++++++++++++++++++++++++++++++++-
include/hw/nubus/nubus.h | 6 ++++++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 9c1992ceb0..dbb3bb7efd 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 - 6) * NUBUS_SUPER_SLOT_SIZE;
@@ -38,10 +43,46 @@ 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_fatal);
+ ret = load_image_mr(path, &nd->decl_rom);
+ g_free(path);
+ if (ret < 0) {
+ warn_report("nubus-device: could not load prom '%s'", nd->romfile);
+ }
+ 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 87a97516c7..0c9f50c32e 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 0x9
@@ -39,12 +40,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] 34+ messages in thread* Re: [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs
2021-09-16 10:05 ` [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
@ 2021-09-16 11:05 ` Philippe Mathieu-Daudé
2021-09-16 13:06 ` Markus Armbruster
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-16 11:05 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, laurent; +Cc: Markus Armbruster
On 9/16/21 12:05 PM, Mark Cave-Ayland wrote:
> 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>
> ---
> hw/nubus/nubus-device.c | 43 +++++++++++++++++++++++++++++++++++++++-
> include/hw/nubus/nubus.h | 6 ++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
> @@ -38,10 +43,46 @@ 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_fatal);
> + ret = load_image_mr(path, &nd->decl_rom);
load_image_mr() already calls get_image_size(), rom_add_file() and
qemu_find_file(). *But* it doesn't takes and Error handle, and report
error using fprintf()... So unfortunately rom_add*() functions are
kinda outdated and you are doing the right thing to propagate detailled
errors. Therefore:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> + g_free(path);
> + if (ret < 0) {
> + warn_report("nubus-device: could not load prom '%s'", nd->romfile);
> + }
> + memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE - size,
> + &nd->decl_rom);
> + }
> }
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs
2021-09-16 11:05 ` Philippe Mathieu-Daudé
@ 2021-09-16 13:06 ` Markus Armbruster
2021-09-16 14:19 ` Mark Cave-Ayland
0 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2021-09-16 13:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Mark Cave-Ayland, qemu-devel, laurent
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> On 9/16/21 12:05 PM, Mark Cave-Ayland wrote:
>> 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>
>> ---
>> hw/nubus/nubus-device.c | 43 +++++++++++++++++++++++++++++++++++++++-
>> include/hw/nubus/nubus.h | 6 ++++++
>> 2 files changed, 48 insertions(+), 1 deletion(-)
>
>> @@ -38,10 +43,46 @@ 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_fatal);
Is this error expected to happen?
If yes, you should quite probably propagate it.
If no, &error_abort.
>> + ret = load_image_mr(path, &nd->decl_rom);
>
> load_image_mr() already calls get_image_size(), rom_add_file() and
> qemu_find_file(). *But* it doesn't takes and Error handle, and report
> error using fprintf()...
... except when they don't:
int load_image_mr(const char *filename, MemoryRegion *mr)
{
int size;
if (!memory_access_is_direct(mr, false)) {
/* Can only load an image into RAM or ROM */
---> return -1;
}
size = get_image_size(filename);
if (size < 0 || size > memory_region_size(mr)) {
return -1;
}
if (size > 0) {
if (rom_add_file_mr(filename, mr, -1) < 0) {
return -1;
}
}
return size;
}
Hot mess!
> So unfortunately rom_add*() functions are
> kinda outdated and you are doing the right thing to propagate detailled
> errors.
I can't see errors being propagated, only a warn_report()...
> Therefore:
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> + g_free(path);
>> + if (ret < 0) {
>> + warn_report("nubus-device: could not load prom '%s'", nd->romfile);
... here.
>> + }
>> + memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE - size,
>> + &nd->decl_rom);
>> + }
>> }
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs
2021-09-16 13:06 ` Markus Armbruster
@ 2021-09-16 14:19 ` Mark Cave-Ayland
2021-09-17 9:53 ` Markus Armbruster
0 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 14:19 UTC (permalink / raw)
To: Markus Armbruster, Philippe Mathieu-Daudé; +Cc: qemu-devel, laurent
On 16/09/2021 14:06, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>
>> On 9/16/21 12:05 PM, Mark Cave-Ayland wrote:
>>> 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>
>>> ---
>>> hw/nubus/nubus-device.c | 43 +++++++++++++++++++++++++++++++++++++++-
>>> include/hw/nubus/nubus.h | 6 ++++++
>>> 2 files changed, 48 insertions(+), 1 deletion(-)
>>
>>> @@ -38,10 +43,46 @@ 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_fatal);
>
> Is this error expected to happen?
>
> If yes, you should quite probably propagate it.
>
> If no, &error_abort.
(goes and looks)
Ultimately this gets set from memory_region_init_rom_device_nomigrate() where err is
returned from qemu_ram_alloc() which is fairly fatal. So I guess this should be
&error_abort then?
Note that I copied that part of the logic from hw/pci/pci.c's pci_add_option_rom() so
it may also need to be adjusted there.
>>> + ret = load_image_mr(path, &nd->decl_rom);
>>
>> load_image_mr() already calls get_image_size(), rom_add_file() and
>> qemu_find_file(). *But* it doesn't takes and Error handle, and report
>> error using fprintf()...
>
> ... except when they don't:
>
> int load_image_mr(const char *filename, MemoryRegion *mr)
> {
> int size;
>
> if (!memory_access_is_direct(mr, false)) {
> /* Can only load an image into RAM or ROM */
> ---> return -1;
> }
>
> size = get_image_size(filename);
>
> if (size < 0 || size > memory_region_size(mr)) {
> return -1;
> }
> if (size > 0) {
> if (rom_add_file_mr(filename, mr, -1) < 0) {
> return -1;
> }
> }
> return size;
> }
>
> Hot mess!
>
>> So unfortunately rom_add*() functions are
>> kinda outdated and you are doing the right thing to propagate detailled
>> errors.
>
> I can't see errors being propagated, only a warn_report()...
>
>> Therefore:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>>> + g_free(path);
>>> + if (ret < 0) {
>>> + warn_report("nubus-device: could not load prom '%s'", nd->romfile);
>
> ... here.
Looking again at pci_add_option_rom() then perhaps this should be error_setg()
instead: if you are explicitly trying to load a ROM image, then you should at least
be able to get the filename correct.
>>> + }
>>> + memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE - size,
>>> + &nd->decl_rom);
>>> + }
>>> }
ATB,
Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs
2021-09-16 14:19 ` Mark Cave-Ayland
@ 2021-09-17 9:53 ` Markus Armbruster
0 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2021-09-17 9:53 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: laurent, Philippe Mathieu-Daudé, qemu-devel
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> On 16/09/2021 14:06, Markus Armbruster wrote:
>
>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>
>>> On 9/16/21 12:05 PM, Mark Cave-Ayland wrote:
>>>> 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>
>>>> ---
>>>> hw/nubus/nubus-device.c | 43 +++++++++++++++++++++++++++++++++++++++-
>>>> include/hw/nubus/nubus.h | 6 ++++++
>>>> 2 files changed, 48 insertions(+), 1 deletion(-)
>>>
>>>> @@ -38,10 +43,46 @@ 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_fatal);
>> Is this error expected to happen?
>> If yes, you should quite probably propagate it.
>> If no, &error_abort.
>
> (goes and looks)
>
> Ultimately this gets set from
> memory_region_init_rom_device_nomigrate() where err is returned from
> qemu_ram_alloc() which is fairly fatal. So I guess this should be
> &error_abort then?
There are two schools of thought on handling out-of-memory conditions.
One school argues that attempting to recover by failing the operation is
expensive and futile. It's expensive, because it creates a huge number
of failure paths that wouldn't otherwise exists, and won't be tested.
It's futile, because by the time malloc() fails, the process is doomed
anyway. That's g_malloc(). It aborts on OOM.
The other school disagrees, and writes the error paths. In this case,
propagate to caller.
In QEMU, we of course do both, and with no clear guidance on when to do
what. All we have is talk about aborting only on "small" allocations,
whatever "small" may be.
I'm cool with &error_abort here.
> Note that I copied that part of the logic from hw/pci/pci.c's
> pci_add_option_rom() so it may also need to be adjusted there.
We're quite prone to use &error_fatal or NULL where we should use
&error_abort.
>>>> + ret = load_image_mr(path, &nd->decl_rom);
>>>
>>> load_image_mr() already calls get_image_size(), rom_add_file() and
>>> qemu_find_file(). *But* it doesn't takes and Error handle, and report
>>> error using fprintf()...
>>
>> ... except when they don't:
>> int load_image_mr(const char *filename, MemoryRegion *mr)
>> {
>> int size;
>> if (!memory_access_is_direct(mr, false)) {
>> /* Can only load an image into RAM or ROM */
>> ---> return -1;
>> }
>> size = get_image_size(filename);
>> if (size < 0 || size > memory_region_size(mr)) {
>> return -1;
>> }
>> if (size > 0) {
>> if (rom_add_file_mr(filename, mr, -1) < 0) {
>> return -1;
>> }
>> }
>> return size;
>> }
>> Hot mess!
>>
>>> So unfortunately rom_add*() functions are
>>> kinda outdated and you are doing the right thing to propagate detailled
>>> errors.
>>
>> I can't see errors being propagated, only a warn_report()...
>>
>>> Therefore:
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>>> + g_free(path);
>>>> + if (ret < 0) {
>>>> + warn_report("nubus-device: could not load prom '%s'", nd->romfile);
>> ... here.
>
> Looking again at pci_add_option_rom() then perhaps this should be
> error_setg() instead: if you are explicitly trying to load a ROM
> image, then you should at least be able to get the filename correct.
Makes sense to me.
>>>> + }
>>>> + memory_region_add_subregion(&nd->slot_mem, NUBUS_SLOT_SIZE - size,
>>>> + &nd->decl_rom);
>>>> + }
>>>> }
>
>
> ATB,
>
> Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (10 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 11/20] nubus-device: add romfile property for loading declaration ROMs Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 12:48 ` BALATON Zoltan
2021-09-16 10:05 ` [PATCH v3 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
` (7 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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 | 8 +++-----
hw/nubus/mac-nubus-bridge.c | 15 +++++++++++++--
hw/nubus/nubus-bus.c | 18 ++++++++++++++++++
hw/nubus/nubus-device.c | 2 +-
include/hw/nubus/mac-nubus-bridge.h | 2 ++
include/hw/nubus/nubus.h | 10 +++++++---
6 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 5ba87f789c..0a0051a296 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,9 @@ 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, 9 * NUBUS_SUPER_SLOT_SIZE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
+ 9 * 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 c1d77e2bc7..574bc7831e 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -21,8 +21,19 @@ static void mac_nubus_bridge_init(Object *obj)
/* Macintosh only has slots 0x9 to 0xe available */
s->bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
- 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,
+ 9 * NUBUS_SUPER_SLOT_SIZE,
+ 6 * NUBUS_SUPER_SLOT_SIZE);
+
+ memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
+ &s->bus->nubus_mr,
+ NUBUS_SLOT_BASE + 9 * NUBUS_SLOT_SIZE,
+ 6 * 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 a617459a4f..ab3515fb75 100644
--- a/hw/nubus/nubus-bus.c
+++ b/hw/nubus/nubus-bus.c
@@ -70,25 +70,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 * 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 * NUBUS_SUPER_SLOT_SIZE,
+ &nubus->slot_io);
nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
}
@@ -149,6 +166,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/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index dbb3bb7efd..bb574f970a 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -26,7 +26,7 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
int ret;
/* Super */
- slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
+ 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,
diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
index 36aa098dd4..650fd24eab 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -19,6 +19,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 0c9f50c32e..0b9f74d4ac 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -15,13 +15,14 @@
#include "qemu/units.h"
#define NUBUS_SUPER_SLOT_SIZE 0x10000000U
-#define NUBUS_SUPER_SLOT_NB 0x9
+#define NUBUS_SUPER_SLOT_NB 0xf
+#define NUBUS_SLOT_BASE (NUBUS_SUPER_SLOT_SIZE * NUBUS_SUPER_SLOT_NB)
#define NUBUS_SLOT_SIZE 0x01000000
-#define NUBUS_SLOT_NB 0xF
+#define NUBUS_SLOT_NB 0xf
#define NUBUS_FIRST_SLOT 0x0
-#define NUBUS_LAST_SLOT 0xF
+#define NUBUS_LAST_SLOT 0xf
#define TYPE_NUBUS_DEVICE "nubus-device"
OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
@@ -34,6 +35,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] 34+ messages in thread* Re: [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space
2021-09-16 10:05 ` [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
@ 2021-09-16 12:48 ` BALATON Zoltan
2021-09-16 14:00 ` Mark Cave-Ayland
0 siblings, 1 reply; 34+ messages in thread
From: BALATON Zoltan @ 2021-09-16 12:48 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: qemu-devel, laurent
[-- Attachment #1: Type: text/plain, Size: 7965 bytes --]
On Thu, 16 Sep 2021, Mark Cave-Ayland wrote:
> 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 | 8 +++-----
> hw/nubus/mac-nubus-bridge.c | 15 +++++++++++++--
> hw/nubus/nubus-bus.c | 18 ++++++++++++++++++
> hw/nubus/nubus-device.c | 2 +-
> include/hw/nubus/mac-nubus-bridge.h | 2 ++
> include/hw/nubus/nubus.h | 10 +++++++---
> 6 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 5ba87f789c..0a0051a296 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,9 @@ 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, 9 * NUBUS_SUPER_SLOT_SIZE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
> + 9 * 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 c1d77e2bc7..574bc7831e 100644
> --- a/hw/nubus/mac-nubus-bridge.c
> +++ b/hw/nubus/mac-nubus-bridge.c
> @@ -21,8 +21,19 @@ static void mac_nubus_bridge_init(Object *obj)
> /* Macintosh only has slots 0x9 to 0xe available */
> s->bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
>
> - 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,
> + 9 * NUBUS_SUPER_SLOT_SIZE,
> + 6 * NUBUS_SUPER_SLOT_SIZE);
Sorry for not spotting it yesterday in v2 but I only had time to have a
closer look now. Do these 9 and 6 worth a #define? Are these something
like MAC_FIST_SLOT and MAC_NUM_SLOTS? As they maybe always appear together
with NUBUS_SUPER_SLOT_SIZE (I haven't checked all but most look like that)
so those products could have a #define just to make it shorter in these
calls. (Are those the #defines that you've removed above?) Maybe
#define MAC_FIRST_SLOT 9
#define MAC_NUM_SLOTS 6
then use these to
#define MAC_SLOTS_MASK MAKE_64BIT_MASK(MAC_FIRST_SLOT, MAC_NUM_SLOTS)
and similarly the memory address and size as
#define MAC_SLOT_BASE 9 * NUBUS_SUPER_SLOT_SIZE
#define MAC_SLOT_SIZE 6 * NUBUS_SUPER_SLOT_SIZE
or so and then use these latter three where they appear now open coded
could be shorter and clearer but I don't mind either way so if you want to
keep the current version that's OK with me as well. (I may have got the
names for these wrong but hopefully you get the idea, I haven't tried to
understand in detail what these really are.)
Regards,
BALATON Zoltan
> +
> + memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
> + &s->bus->nubus_mr,
> + NUBUS_SLOT_BASE + 9 * NUBUS_SLOT_SIZE,
> + 6 * 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 a617459a4f..ab3515fb75 100644
> --- a/hw/nubus/nubus-bus.c
> +++ b/hw/nubus/nubus-bus.c
> @@ -70,25 +70,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 * 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 * NUBUS_SUPER_SLOT_SIZE,
> + &nubus->slot_io);
>
> nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
> }
> @@ -149,6 +166,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/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index dbb3bb7efd..bb574f970a 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -26,7 +26,7 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
> int ret;
>
> /* Super */
> - slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
> + 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,
> diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
> index 36aa098dd4..650fd24eab 100644
> --- a/include/hw/nubus/mac-nubus-bridge.h
> +++ b/include/hw/nubus/mac-nubus-bridge.h
> @@ -19,6 +19,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 0c9f50c32e..0b9f74d4ac 100644
> --- a/include/hw/nubus/nubus.h
> +++ b/include/hw/nubus/nubus.h
> @@ -15,13 +15,14 @@
> #include "qemu/units.h"
>
> #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
> -#define NUBUS_SUPER_SLOT_NB 0x9
> +#define NUBUS_SUPER_SLOT_NB 0xf
>
> +#define NUBUS_SLOT_BASE (NUBUS_SUPER_SLOT_SIZE * NUBUS_SUPER_SLOT_NB)
> #define NUBUS_SLOT_SIZE 0x01000000
> -#define NUBUS_SLOT_NB 0xF
> +#define NUBUS_SLOT_NB 0xf
>
> #define NUBUS_FIRST_SLOT 0x0
> -#define NUBUS_LAST_SLOT 0xF
> +#define NUBUS_LAST_SLOT 0xf
>
> #define TYPE_NUBUS_DEVICE "nubus-device"
> OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
> @@ -34,6 +35,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;
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space
2021-09-16 12:48 ` BALATON Zoltan
@ 2021-09-16 14:00 ` Mark Cave-Ayland
2021-09-16 16:50 ` BALATON Zoltan
0 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 14:00 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, laurent
On 16/09/2021 13:48, BALATON Zoltan wrote:
> On Thu, 16 Sep 2021, Mark Cave-Ayland wrote:
>> 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 | 8 +++-----
>> hw/nubus/mac-nubus-bridge.c | 15 +++++++++++++--
>> hw/nubus/nubus-bus.c | 18 ++++++++++++++++++
>> hw/nubus/nubus-device.c | 2 +-
>> include/hw/nubus/mac-nubus-bridge.h | 2 ++
>> include/hw/nubus/nubus.h | 10 +++++++---
>> 6 files changed, 44 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 5ba87f789c..0a0051a296 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,9 @@ 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, 9 * NUBUS_SUPER_SLOT_SIZE);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
>> + 9 * 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 c1d77e2bc7..574bc7831e 100644
>> --- a/hw/nubus/mac-nubus-bridge.c
>> +++ b/hw/nubus/mac-nubus-bridge.c
>> @@ -21,8 +21,19 @@ static void mac_nubus_bridge_init(Object *obj)
>> /* Macintosh only has slots 0x9 to 0xe available */
>> s->bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
>>
>> - 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,
>> + 9 * NUBUS_SUPER_SLOT_SIZE,
>> + 6 * NUBUS_SUPER_SLOT_SIZE);
>
> Sorry for not spotting it yesterday in v2 but I only had time to have a closer look
> now. Do these 9 and 6 worth a #define? Are these something like MAC_FIST_SLOT and
> MAC_NUM_SLOTS? As they maybe always appear together with NUBUS_SUPER_SLOT_SIZE (I
> haven't checked all but most look like that) so those products could have a #define
> just to make it shorter in these calls. (Are those the #defines that you've removed
> above?) Maybe
>
> #define MAC_FIRST_SLOT 9
> #define MAC_NUM_SLOTS 6
>
> then use these to
>
> #define MAC_SLOTS_MASK MAKE_64BIT_MASK(MAC_FIRST_SLOT, MAC_NUM_SLOTS)
>
> and similarly the memory address and size as
>
> #define MAC_SLOT_BASE 9 * NUBUS_SUPER_SLOT_SIZE #define MAC_SLOT_SIZE 6 *
> NUBUS_SUPER_SLOT_SIZE
>
> or so and then use these latter three where they appear now open coded could be
> shorter and clearer but I don't mind either way so if you want to keep the current
> version that's OK with me as well. (I may have got the names for these wrong but
> hopefully you get the idea, I haven't tried to understand in detail what these really
> are.)
>
> Regards,
> BALATON Zoltan
I'm not amazingly sold on this since it introduces another layer of indirection: in
its current form you can immediately correlate all of mask, offset, and size with the
comment without having to look up another set of constants first.
>> +
>> + memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
>> + &s->bus->nubus_mr,
>> + NUBUS_SLOT_BASE + 9 * NUBUS_SLOT_SIZE,
>> + 6 * 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 a617459a4f..ab3515fb75 100644
>> --- a/hw/nubus/nubus-bus.c
>> +++ b/hw/nubus/nubus-bus.c
>> @@ -70,25 +70,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 * 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 * NUBUS_SUPER_SLOT_SIZE,
>> + &nubus->slot_io);
>>
>> nubus->slot_available_mask = MAKE_64BIT_MASK(0, 16);
>> }
>> @@ -149,6 +166,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/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
>> index dbb3bb7efd..bb574f970a 100644
>> --- a/hw/nubus/nubus-device.c
>> +++ b/hw/nubus/nubus-device.c
>> @@ -26,7 +26,7 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>> int ret;
>>
>> /* Super */
>> - slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
>> + 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,
>> diff --git a/include/hw/nubus/mac-nubus-bridge.h b/include/hw/nubus/mac-nubus-bridge.h
>> index 36aa098dd4..650fd24eab 100644
>> --- a/include/hw/nubus/mac-nubus-bridge.h
>> +++ b/include/hw/nubus/mac-nubus-bridge.h
>> @@ -19,6 +19,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 0c9f50c32e..0b9f74d4ac 100644
>> --- a/include/hw/nubus/nubus.h
>> +++ b/include/hw/nubus/nubus.h
>> @@ -15,13 +15,14 @@
>> #include "qemu/units.h"
>>
>> #define NUBUS_SUPER_SLOT_SIZE 0x10000000U
>> -#define NUBUS_SUPER_SLOT_NB 0x9
>> +#define NUBUS_SUPER_SLOT_NB 0xf
>>
>> +#define NUBUS_SLOT_BASE (NUBUS_SUPER_SLOT_SIZE * NUBUS_SUPER_SLOT_NB)
>> #define NUBUS_SLOT_SIZE 0x01000000
>> -#define NUBUS_SLOT_NB 0xF
>> +#define NUBUS_SLOT_NB 0xf
>>
>> #define NUBUS_FIRST_SLOT 0x0
>> -#define NUBUS_LAST_SLOT 0xF
>> +#define NUBUS_LAST_SLOT 0xf
>>
>> #define TYPE_NUBUS_DEVICE "nubus-device"
>> OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
>> @@ -34,6 +35,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;
ATB,
Mark.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space
2021-09-16 14:00 ` Mark Cave-Ayland
@ 2021-09-16 16:50 ` BALATON Zoltan
0 siblings, 0 replies; 34+ messages in thread
From: BALATON Zoltan @ 2021-09-16 16:50 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: qemu-devel, laurent
[-- Attachment #1: Type: text/plain, Size: 4630 bytes --]
On Thu, 16 Sep 2021, Mark Cave-Ayland wrote:
> On 16/09/2021 13:48, BALATON Zoltan wrote:
>
>> On Thu, 16 Sep 2021, Mark Cave-Ayland wrote:
>>> 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 | 8 +++-----
>>> hw/nubus/mac-nubus-bridge.c | 15 +++++++++++++--
>>> hw/nubus/nubus-bus.c | 18 ++++++++++++++++++
>>> hw/nubus/nubus-device.c | 2 +-
>>> include/hw/nubus/mac-nubus-bridge.h | 2 ++
>>> include/hw/nubus/nubus.h | 10 +++++++---
>>> 6 files changed, 44 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>> index 5ba87f789c..0a0051a296 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,9 @@ 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, 9 * NUBUS_SUPER_SLOT_SIZE);
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
>>> + 9 * 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 c1d77e2bc7..574bc7831e 100644
>>> --- a/hw/nubus/mac-nubus-bridge.c
>>> +++ b/hw/nubus/mac-nubus-bridge.c
>>> @@ -21,8 +21,19 @@ static void mac_nubus_bridge_init(Object *obj)
>>> /* Macintosh only has slots 0x9 to 0xe available */
>>> s->bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
>>>
>>> - 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,
>>> + 9 * NUBUS_SUPER_SLOT_SIZE,
>>> + 6 * NUBUS_SUPER_SLOT_SIZE);
>>
>> Sorry for not spotting it yesterday in v2 but I only had time to have a
>> closer look now. Do these 9 and 6 worth a #define? Are these something like
>> MAC_FIST_SLOT and MAC_NUM_SLOTS? As they maybe always appear together with
>> NUBUS_SUPER_SLOT_SIZE (I haven't checked all but most look like that) so
>> those products could have a #define just to make it shorter in these calls.
>> (Are those the #defines that you've removed above?) Maybe
>>
>> #define MAC_FIRST_SLOT 9
>> #define MAC_NUM_SLOTS 6
>>
>> then use these to
>>
>> #define MAC_SLOTS_MASK MAKE_64BIT_MASK(MAC_FIRST_SLOT, MAC_NUM_SLOTS)
>>
>> and similarly the memory address and size as
>>
>> #define MAC_SLOT_BASE 9 * NUBUS_SUPER_SLOT_SIZE #define MAC_SLOT_SIZE 6 *
>> NUBUS_SUPER_SLOT_SIZE
>>
>> or so and then use these latter three where they appear now open coded
>> could be shorter and clearer but I don't mind either way so if you want to
>> keep the current version that's OK with me as well. (I may have got the
>> names for these wrong but hopefully you get the idea, I haven't tried to
>> understand in detail what these really are.)
>>
>> Regards,
>> BALATON Zoltan
>
> I'm not amazingly sold on this since it introduces another layer of
> indirection: in its current form you can immediately correlate all of mask,
> offset, and size with the comment without having to look up another set of
> constants first.
As I said I'm OK with it as it as too.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 13/20] nubus-bridge: introduce separate NubusBridge structure
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (11 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 12/20] nubus: move nubus to its own 32-bit address space Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
` (6 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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 650fd24eab..7365038966 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -16,7 +16,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 0b9f74d4ac..bb2e70e1d1 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"
@@ -31,6 +32,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;
@@ -57,4 +59,8 @@ struct NubusDevice {
MemoryRegion decl_rom;
};
+struct NubusBridge {
+ SysBusDevice parent_obj;
+};
+
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (12 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 13/20] nubus-bridge: introduce separate NubusBridge structure Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
` (5 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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 574bc7831e..76ea403f80 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));
@@ -47,7 +49,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 7365038966..d9bb11db31 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -13,9 +13,9 @@
#include "qom/object.h"
#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] 34+ messages in thread* [PATCH v3 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (13 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 14/20] mac-nubus-bridge: rename MacNubusState to MacNubusBridge Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
` (4 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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 | 9 ++++-----
hw/nubus/nubus-bridge.c | 9 +++++++++
include/hw/nubus/mac-nubus-bridge.h | 1 -
include/hw/nubus/nubus.h | 2 ++
5 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 0a0051a296..46befe0898 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -397,7 +397,7 @@ static void q800_init(MachineState *machine)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
9 * 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 76ea403f80..4854e7e546 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -16,21 +16,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);
- 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(9, 6);
+ nb->bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
/* 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,
9 * NUBUS_SUPER_SLOT_SIZE,
6 * 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 + 9 * NUBUS_SLOT_SIZE,
6 * 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 d9bb11db31..45ec610d52 100644
--- a/include/hw/nubus/mac-nubus-bridge.h
+++ b/include/hw/nubus/mac-nubus-bridge.h
@@ -18,7 +18,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 bb2e70e1d1..2c83e7f7b8 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -61,6 +61,8 @@ struct NubusDevice {
struct NubusBridge {
SysBusDevice parent_obj;
+
+ NubusBus *bus;
};
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (14 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 15/20] nubus: move NubusBus from mac-nubus-bridge to nubus-bridge Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
` (3 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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 | 7 ++++---
hw/nubus/nubus-bridge.c | 3 ++-
include/hw/nubus/nubus.h | 2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 46befe0898..e34df1a829 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -397,7 +397,7 @@ static void q800_init(MachineState *machine)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
9 * 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 4854e7e546..f764b2f562 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -18,18 +18,19 @@ 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(9, 6);
+ bus->slot_available_mask = MAKE_64BIT_MASK(9, 6);
/* 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,
9 * NUBUS_SUPER_SLOT_SIZE,
6 * NUBUS_SUPER_SLOT_SIZE);
memory_region_init_alias(&s->slot_alias, obj, "slot-alias",
- &nb->bus->nubus_mr,
+ &bus->nubus_mr,
NUBUS_SLOT_BASE + 9 * NUBUS_SLOT_SIZE,
6 * 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 2c83e7f7b8..cf9a585a91 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -62,7 +62,7 @@ struct NubusDevice {
struct NubusBridge {
SysBusDevice parent_obj;
- NubusBus *bus;
+ NubusBus bus;
};
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 17/20] nubus-bridge: make slot_available_mask a qdev property
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (15 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 16/20] nubus-bridge: embed the NubusBus object directly within nubus-bridge Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
` (2 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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] 34+ messages in thread* [PATCH v3 18/20] nubus: add support for slot IRQs
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (16 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 17/20] nubus-bridge: make slot_available_mask a qdev property Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
2021-09-16 10:05 ` [PATCH v3 20/20] q800: configure nubus available slots for Quadra 800 Mark Cave-Ayland
19 siblings, 0 replies; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
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 bb574f970a..a5cbd88153 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 cf9a585a91..1c487f74ac 100644
--- a/include/hw/nubus/nubus.h
+++ b/include/hw/nubus/nubus.h
@@ -25,6 +25,8 @@
#define NUBUS_FIRST_SLOT 0x0
#define NUBUS_LAST_SLOT 0xf
+#define NUBUS_IRQS 16
+
#define TYPE_NUBUS_DEVICE "nubus-device"
OBJECT_DECLARE_SIMPLE_TYPE(NubusDevice, NUBUS_DEVICE)
@@ -44,6 +46,8 @@ struct NubusBus {
MemoryRegion slot_io;
uint32_t slot_available_mask;
+
+ qemu_irq irqs[NUBUS_IRQS];
};
#define NUBUS_DECL_ROM_MAX_SIZE (128 * KiB)
@@ -59,6 +63,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] 34+ messages in thread* [PATCH v3 19/20] q800: wire up nubus IRQs
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (17 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 18/20] nubus: add support for slot IRQs Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-19 21:13 ` Philippe Mathieu-Daudé
2021-09-16 10:05 ` [PATCH v3 20/20] q800: configure nubus available slots for Quadra 800 Mark Cave-Ayland
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
hw/m68k/q800.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index e34df1a829..fbc45a301f 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -396,6 +396,11 @@ static void q800_init(MachineState *machine)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 9 * NUBUS_SUPER_SLOT_SIZE);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
9 * 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;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 19/20] q800: wire up nubus IRQs
2021-09-16 10:05 ` [PATCH v3 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
@ 2021-09-19 21:13 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-19 21:13 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, laurent
On 9/16/21 12:05, Mark Cave-Ayland wrote:
> 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>
> ---
> hw/m68k/q800.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index e34df1a829..fbc45a301f 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -396,6 +396,11 @@ static void q800_init(MachineState *machine)
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 9 * NUBUS_SUPER_SLOT_SIZE);
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
> 9 * 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));
> + }
Hopefully Laurent can double-check the definition values.
Otherwise for the qdev API usage:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 20/20] q800: configure nubus available slots for Quadra 800
2021-09-16 10:05 [PATCH v3 00/20] nubus: bus, device, bridge, IRQ and address space improvements Mark Cave-Ayland
` (18 preceding siblings ...)
2021-09-16 10:05 ` [PATCH v3 19/20] q800: wire up nubus IRQs Mark Cave-Ayland
@ 2021-09-16 10:05 ` Mark Cave-Ayland
2021-09-19 21:14 ` Philippe Mathieu-Daudé
19 siblings, 1 reply; 34+ messages in thread
From: Mark Cave-Ayland @ 2021-09-16 10:05 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>
---
hw/m68k/q800.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index fbc45a301f..63f42764eb 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, 9 * NUBUS_SUPER_SLOT_SIZE);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread