Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
Am 1. März 2023 16:42:16 UTC schrieb Mark Cave-Ayland
:
>On 23/02/2023 20:46, Bernhard Beschow wrote:
>>
>>
>> Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
>> :
>>> On 06/02/2023 23:40, Bernhard Beschow wrote:
>>>
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
> On 05/02/2023 22:21, BALATON Zoltan wrote:
>
>> On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
>>> On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64
++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d,
ISABus *isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i,
2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev,
Error **errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice
*dev, Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports
only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports
only one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 23/02/2023 20:46, Bernhard Beschow wrote:
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
On 06/02/2023 23:40, Bernhard Beschow wrote:
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev,
Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+ device_class_set_props(dc,
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On Thu, Jan 26, 2023 at 10:17:37PM +0100, Bernhard Beschow wrote:
> diff --git a/hw/isa/piix.c b/hw/isa/piix.c
> index 54a1246a9d..f9974c2a77 100644
> --- a/hw/isa/piix.c
> +++ b/hw/isa/piix.c
> @@ -345,9 +345,14 @@ static void pci_piix_realize(PCIDevice *dev, const char
> *uhci_type,
>
> /* IDE */
> qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn + 1);
> +qdev_prop_set_bit(DEVICE(>ide), "user-created", false);
> if (!qdev_realize(DEVICE(>ide), BUS(pci_bus), errp)) {
> return;
> }
> +qdev_connect_gpio_out(DEVICE(>ide), 0,
> + qdev_get_gpio_in(DEVICE(>pic), 14));
> +qdev_connect_gpio_out(DEVICE(>ide), 1,
> + qdev_get_gpio_in(DEVICE(>pic), 15));
>
OK, but I think we should prefix this with "x-" so we don't commit
to this as a stable API.
> /* USB */
> if (d->has_usb) {
> --
> 2.39.1
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
>On 06/02/2023 23:40, Bernhard Beschow wrote:
>
>> Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
>> :
>>> On 05/02/2023 22:21, BALATON Zoltan wrote:
>>>
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
> On 26/01/2023 21:17, Bernhard Beschow wrote:
>> Internal instances now defer interrupt wiring to the caller which
>> decouples them from the ISABus. User-created devices still fish out the
>> ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
>> The latter mechanism is considered a workaround and intended to be
>> removed once a deprecation period for user-created PIIX IDE devices is
>> over.
>>
>> Signed-off-by: Bernhard Beschow
>> ---
>> include/hw/ide/pci.h | 1 +
>> hw/ide/piix.c | 64 ++--
>> hw/isa/piix.c | 5
>> 3 files changed, 56 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>> index 24c0b7a2dd..ee2c8781b7 100644
>> --- a/include/hw/ide/pci.h
>> +++ b/include/hw/ide/pci.h
>> @@ -54,6 +54,7 @@ struct PCIIDEState {
>> MemoryRegion bmdma_bar;
>> MemoryRegion cmd_bar[2];
>> MemoryRegion data_bar[2];
>> + bool user_created;
>> };
>> static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>> index 5980045db0..f0d95761ac 100644
>> --- a/hw/ide/piix.c
>> +++ b/hw/ide/piix.c
>> @@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
>> }
>> }
>> +static void piix_ide_set_irq(void *opaque, int n, int level)
>> +{
>> + PCIIDEState *d = opaque;
>> +
>> + qemu_set_irq(d->isa_irqs[n], level);
>> +}
>> +
>> static void piix_ide_reset(DeviceState *dev)
>> {
>> PCIIDEState *d = PCI_IDE(dev);
>> @@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d,
>> ISABus *isa_bus)
>> };
>> int i;
>> + if (isa_bus) {
>> + d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
>> + d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
>> + } else {
>> + qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
>> + }
>> +
>> for (i = 0; i < 2; i++) {
>> ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>> ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
>> port_info[i].iobase2);
>> - ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
>> + ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
>> bmdma_init(>bus[i], >bmdma[i], d);
>> d->bmdma[i].bus = >bus[i];
>> @@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev,
>> Error **errp)
>> {
>> PCIIDEState *d = PCI_IDE(dev);
>> uint8_t *pci_conf = dev->config;
>> - ISABus *isa_bus;
>> - bool ambiguous;
>> + ISABus *isa_bus = NULL;
>> pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
>> @@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice
>> *dev, Error **errp)
>> vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
>> - isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
>> ));
>> - if (ambiguous) {
>> - error_setg(errp,
>> - "More than one ISA bus found while %s supports only
>> one",
>> - object_get_typename(OBJECT(dev)));
>> - return;
>> - }
>> - if (!isa_bus) {
>> - error_setg(errp, "No ISA bus found while %s requires one",
>> - object_get_typename(OBJECT(dev)));
>> - return;
>> + if (d->user_created) {
>> + bool ambiguous;
>> +
>> + isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
>> + ));
>> +
>> + if (ambiguous) {
>> + error_setg(errp,
>> + "More than one ISA bus found while %s supports
>> only one",
>> + object_get_typename(OBJECT(dev)));
>> + return;
>> + }
>> +
>> + if (!isa_bus) {
>> + error_setg(errp, "No ISA bus found while %s requires one",
>> + object_get_typename(OBJECT(dev)));
>> + return;
>> + }
>> }
>> pci_piix_init_ports(d, isa_bus);
>> }
>> +static void pci_piix_ide_init(Object *obj)
>> +{
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
>> +}
>> +
>>
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 8/2/23 01:18, Bernhard Beschow wrote:
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
On 06/02/2023 23:40, Bernhard Beschow wrote:
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev,
Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+ device_class_set_props(dc, piix_ide_properties);
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 8/2/23 01:43, BALATON Zoltan wrote:
On Wed, 8 Feb 2023, Bernhard Beschow wrote:
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
On 06/02/2023 23:40, Bernhard Beschow wrote:
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish
out the
ISABus from the QOM tree and the interrupt wiring remains in
PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE
devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64
++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 54a1246a9d..f9974c2a77 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -345,9 +345,14 @@ static void pci_piix_realize(PCIDevice
*dev, const char *uhci_type,
/* IDE */
qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn +
1);
+ qdev_prop_set_bit(DEVICE(>ide), "user-created", false);
if (!qdev_realize(DEVICE(>ide), BUS(pci_bus), errp)) {
return;
}
+ qdev_connect_gpio_out(DEVICE(>ide), 0,
+ qdev_get_gpio_in(DEVICE(>pic), 14));
+ qdev_connect_gpio_out(DEVICE(>ide), 1,
+ qdev_get_gpio_in(DEVICE(>pic), 15));
/* USB */
if (d->has_usb) {
I haven't checked the datasheet, but I suspect this will be
similar to the cmd646/via PCI-IDE interfaces in that there will
be a PCI configuration register that will switch between ISA
compatibility mode (and ISA irqs) and PCI mode (with PCI IRQs).
So it would be the device configuration that would specify PCI or
ISA mode, rather than the presence of an ISABus.
I forgot about this topic already and haven't follwed this series
either so what I say may not fully make sense but I think CMD646
and via-ide are different. CMD646 is a PCI device and should use
PCI interrupts while via-ide is part of a southbridge/superio
complex and connected to the ISA PICs within that southbride, so I
think via-ide always uses ISA IRQs and the ISA btidge within the
same chip may convert that to PCI IRQs or not (that part is where
I'm lost also because we may not actually model it that way).
After a long debate we managed to find a solution back then that
works for every guest we use it for now so I think we don't want
to touch it now until some real need arises. It does not worth the
trouble and added complexity to model something that is not used
just for the sake of correctness. By the time we find a use for
that, the ISA emulation may evolve so it's easier to implement the
missing switching between isa and native mode or we may want to do
it differently
(such as we do things differently now compared to what we did years
ago). So I think it does not worth keeping the ISA model from being
simplified for some theoretical uses in the future which we may not
actually do any time soon.
Indeed we don't want (and have the time) to model features we don't need
or will never use. However taking the time to correctly understand these
devices help us to correctly model them. In particular when design flaws
have been identified in some models.
But I don't want to get into this again so
just shared my thoughts and feel free to ignore it. I don't care where
these patches go as long as the VIA model keeps working for me.
We certainly want to keep what we currently have working :)
I have a vague memory that ISA compatibility mode was part of the
original PCI-BMDMA specification, but it has been a while since I
last looked.
Bernhard, is there any mention of this in the PIIX datasheet(s)?
For reference the cmd646 datasheet specifies that ISA mode or PCI
mode is determined by register PROG_IF (0x9) in PCI configuration
space.
I've found the following:
"Only PCI masters have access to the IDE port. ISA Bus masters
cannot access the IDE I/O port addresses. Memory targeted by the IDE
interface acting as a PCI Bus master on behalf of IDE DMA slaves
must reside on PCI, usually main memory implemented by the
host-to-PCI bridge."
And:
"PIIX4 can act as a PCI Bus master on behalf of an IDE slave
device."
Does this perhaps mean that piix-ide does indeed have no ISA bus?
I'd be amazed if that were the case: certainly when the first
motherboards came out with PCI and ISA slots, I'd expect the IDE
legacy mode to be enabled by default since BIOSes and OSs such as DOS
wouldn't have been PCI aware and would access the ISA ioports
directly. From memory the OF PCI specification has mention of
workarounds such as mapping the old VGA
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On Wed, 8 Feb 2023, Bernhard Beschow wrote:
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
On 06/02/2023 23:40, Bernhard Beschow wrote:
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+ device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
Am 7. Februar 2023 20:52:02 UTC schrieb Mark Cave-Ayland
:
>On 06/02/2023 23:40, Bernhard Beschow wrote:
>
>> Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
>> :
>>> On 05/02/2023 22:21, BALATON Zoltan wrote:
>>>
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
> On 26/01/2023 21:17, Bernhard Beschow wrote:
>> Internal instances now defer interrupt wiring to the caller which
>> decouples them from the ISABus. User-created devices still fish out the
>> ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
>> The latter mechanism is considered a workaround and intended to be
>> removed once a deprecation period for user-created PIIX IDE devices is
>> over.
>>
>> Signed-off-by: Bernhard Beschow
>> ---
>> include/hw/ide/pci.h | 1 +
>> hw/ide/piix.c | 64 ++--
>> hw/isa/piix.c | 5
>> 3 files changed, 56 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>> index 24c0b7a2dd..ee2c8781b7 100644
>> --- a/include/hw/ide/pci.h
>> +++ b/include/hw/ide/pci.h
>> @@ -54,6 +54,7 @@ struct PCIIDEState {
>> MemoryRegion bmdma_bar;
>> MemoryRegion cmd_bar[2];
>> MemoryRegion data_bar[2];
>> + bool user_created;
>> };
>> static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>> index 5980045db0..f0d95761ac 100644
>> --- a/hw/ide/piix.c
>> +++ b/hw/ide/piix.c
>> @@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
>> }
>> }
>> +static void piix_ide_set_irq(void *opaque, int n, int level)
>> +{
>> + PCIIDEState *d = opaque;
>> +
>> + qemu_set_irq(d->isa_irqs[n], level);
>> +}
>> +
>> static void piix_ide_reset(DeviceState *dev)
>> {
>> PCIIDEState *d = PCI_IDE(dev);
>> @@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d,
>> ISABus *isa_bus)
>> };
>> int i;
>> + if (isa_bus) {
>> + d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
>> + d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
>> + } else {
>> + qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
>> + }
>> +
>> for (i = 0; i < 2; i++) {
>> ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>> ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
>> port_info[i].iobase2);
>> - ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
>> + ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
>> bmdma_init(>bus[i], >bmdma[i], d);
>> d->bmdma[i].bus = >bus[i];
>> @@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev,
>> Error **errp)
>> {
>> PCIIDEState *d = PCI_IDE(dev);
>> uint8_t *pci_conf = dev->config;
>> - ISABus *isa_bus;
>> - bool ambiguous;
>> + ISABus *isa_bus = NULL;
>> pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
>> @@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice
>> *dev, Error **errp)
>> vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
>> - isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
>> ));
>> - if (ambiguous) {
>> - error_setg(errp,
>> - "More than one ISA bus found while %s supports only
>> one",
>> - object_get_typename(OBJECT(dev)));
>> - return;
>> - }
>> - if (!isa_bus) {
>> - error_setg(errp, "No ISA bus found while %s requires one",
>> - object_get_typename(OBJECT(dev)));
>> - return;
>> + if (d->user_created) {
>> + bool ambiguous;
>> +
>> + isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
>> + ));
>> +
>> + if (ambiguous) {
>> + error_setg(errp,
>> + "More than one ISA bus found while %s supports
>> only one",
>> + object_get_typename(OBJECT(dev)));
>> + return;
>> + }
>> +
>> + if (!isa_bus) {
>> + error_setg(errp, "No ISA bus found while %s requires one",
>> + object_get_typename(OBJECT(dev)));
>> + return;
>> + }
>> }
>> pci_piix_init_ports(d, isa_bus);
>> }
>> +static void pci_piix_ide_init(Object *obj)
>> +{
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
>> +}
>> +
>>
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 06/02/2023 23:40, Bernhard Beschow wrote:
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+ device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info = {
.name = TYPE_PIIX3_IDE,
.parent = TYPE_PCI_IDE,
+ .instance_init =
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 7/2/23 00:40, Bernhard Beschow wrote: Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland : On 05/02/2023 22:21, BALATON Zoltan wrote: On Sun, 5 Feb 2023, Mark Cave-Ayland wrote: On 26/01/2023 21:17, Bernhard Beschow wrote: Internal instances now defer interrupt wiring to the caller which decouples them from the ISABus. User-created devices still fish out the ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE. The latter mechanism is considered a workaround and intended to be removed once a deprecation period for user-created PIIX IDE devices is over. Signed-off-by: Bernhard Beschow --- include/hw/ide/pci.h | 1 + hw/ide/piix.c | 64 ++-- hw/isa/piix.c | 5 3 files changed, 56 insertions(+), 14 deletions(-) I haven't checked the datasheet, but I suspect this will be similar to the cmd646/via PCI-IDE interfaces in that there will be a PCI configuration register that will switch between ISA compatibility mode (and ISA irqs) and PCI mode (with PCI IRQs). So it would be the device configuration that would specify PCI or ISA mode, rather than the presence of an ISABus. I forgot about this topic already and haven't follwed this series either so what I say may not fully make sense but I think CMD646 and via-ide are different. CMD646 is a PCI device and should use PCI interrupts while via-ide is part of a southbridge/superio complex and connected to the ISA PICs within that southbride, so I think via-ide always uses ISA IRQs and the ISA btidge within the same chip may convert that to PCI IRQs or not (that part is where I'm lost also because we may not actually model it that way). After a long debate we managed to find a solution back then that works for every guest we use it for now so I think we don't want to touch it now until some real need arises. It does not worth the trouble and added complexity to model something that is not used just for the sake of correctness. By the time we find a use for that, the ISA emulation may evolve so it's easier to implement the missing switching between isa and native mode or we may want to do it differently (such as we do things differently now compared to what we did years ago). So I think it does not worth keeping the ISA model from being simplified for some theoretical uses in the future which we may not actually do any time soon. But I don't want to get into this again so just shared my thoughts and feel free to ignore it. I don't care where these patches go as long as the VIA model keeps working for me. I have a vague memory that ISA compatibility mode was part of the original PCI-BMDMA specification, but it has been a while since I last looked. Bernhard, is there any mention of this in the PIIX datasheet(s)? For reference the cmd646 datasheet specifies that ISA mode or PCI mode is determined by register PROG_IF (0x9) in PCI configuration space. I've found the following: "Only PCI masters have access to the IDE port. ISA Bus masters cannot access the IDE I/O port addresses. Memory targeted by the IDE interface acting as a PCI Bus master on behalf of IDE DMA slaves must reside on PCI, usually main memory implemented by the host-to-PCI bridge." And: "PIIX4 can act as a PCI Bus master on behalf of an IDE slave device." Does this perhaps mean that piix-ide does indeed have no ISA bus? Yes :)
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland
:
>On 05/02/2023 22:21, BALATON Zoltan wrote:
>
>> On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
>>> On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d,
ISABus *isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev,
Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only
one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports
only one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE:
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 06/02/2023 06:51, Philippe Mathieu-Daudé wrote: On 5/2/23 23:32, Mark Cave-Ayland wrote: On 05/02/2023 22:21, BALATON Zoltan wrote: On Sun, 5 Feb 2023, Mark Cave-Ayland wrote: On 26/01/2023 21:17, Bernhard Beschow wrote: Internal instances now defer interrupt wiring to the caller which decouples them from the ISABus. User-created devices still fish out the ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE. The latter mechanism is considered a workaround and intended to be removed once a deprecation period for user-created PIIX IDE devices is over. Signed-off-by: Bernhard Beschow --- include/hw/ide/pci.h | 1 + hw/ide/piix.c | 64 ++-- hw/isa/piix.c | 5 3 files changed, 56 insertions(+), 14 deletions(-) I haven't checked the datasheet, but I suspect this will be similar to the cmd646/via PCI-IDE interfaces in that there will be a PCI configuration register that will switch between ISA compatibility mode (and ISA irqs) and PCI mode (with PCI IRQs). So it would be the device configuration that would specify PCI or ISA mode, rather than the presence of an ISABus. I forgot about this topic already and haven't follwed this series either so what I say may not fully make sense but I think CMD646 and via-ide are different. CMD646 is a PCI device and should use PCI interrupts while via-ide is part of a southbridge/superio complex and connected to the ISA PICs within that southbride, so I think via-ide always uses ISA IRQs and the ISA btidge within the same chip may convert that to PCI IRQs or not (that part is where I'm lost also because we may not actually model it that way). After a long debate we managed to find a solution back then that works for every guest we use it for now so I think we don't want to touch it now until some real need arises. It does not worth the trouble and added complexity to model something that is not used just for the sake of correctness. By the time we find a use for that, the ISA emulation may evolve so it's easier to implement the missing switching between isa and native mode or we may want to do it differently (such as we do things differently now compared to what we did years ago). So I think it does not worth keeping the ISA model from being simplified for some theoretical uses in the future which we may not actually do any time soon. But I don't want to get into this again so just shared my thoughts and feel free to ignore it. I don't care where these patches go as long as the VIA model keeps working for me. I have a vague memory that ISA compatibility mode was part of the original PCI-BMDMA specification, but it has been a while since I last looked. Bernhard, is there any mention of this in the PIIX datasheet(s)? For reference the cmd646 datasheet specifies that ISA mode or PCI mode is determined by register PROG_IF (0x9) in PCI configuration space. Orthogonal to this discussion, one problem I see here is a device exposing 2 interfaces: ISA and PCI. QOM does support interfaces, but ISA and PCI aren't QOM interfaces. The QOM cast macros are written as a QOM object can only inherit one parent. Should we stick to QDev and convert ISA/PCI as QOM interfaces? That could solve some QDev IDE limitations... The normal approach to this is to encapsulate the chip functionality as a set of common functions, for example see esp.c and then have separate files such as esp-pci.c and esp-isa.c which can be compiled accordingly using Kconfig dependencies. FWIW the ability to support a legacy mode is only something that would be present in the generation of mixed PCI/ISA motherboards, and so probably not something that is worth the effort of reworking separate PCI and ISA QOM interfaces. ATB, Mark.
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 5/2/23 23:32, Mark Cave-Ayland wrote: On 05/02/2023 22:21, BALATON Zoltan wrote: On Sun, 5 Feb 2023, Mark Cave-Ayland wrote: On 26/01/2023 21:17, Bernhard Beschow wrote: Internal instances now defer interrupt wiring to the caller which decouples them from the ISABus. User-created devices still fish out the ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE. The latter mechanism is considered a workaround and intended to be removed once a deprecation period for user-created PIIX IDE devices is over. Signed-off-by: Bernhard Beschow --- include/hw/ide/pci.h | 1 + hw/ide/piix.c | 64 ++-- hw/isa/piix.c | 5 3 files changed, 56 insertions(+), 14 deletions(-) I haven't checked the datasheet, but I suspect this will be similar to the cmd646/via PCI-IDE interfaces in that there will be a PCI configuration register that will switch between ISA compatibility mode (and ISA irqs) and PCI mode (with PCI IRQs). So it would be the device configuration that would specify PCI or ISA mode, rather than the presence of an ISABus. I forgot about this topic already and haven't follwed this series either so what I say may not fully make sense but I think CMD646 and via-ide are different. CMD646 is a PCI device and should use PCI interrupts while via-ide is part of a southbridge/superio complex and connected to the ISA PICs within that southbride, so I think via-ide always uses ISA IRQs and the ISA btidge within the same chip may convert that to PCI IRQs or not (that part is where I'm lost also because we may not actually model it that way). After a long debate we managed to find a solution back then that works for every guest we use it for now so I think we don't want to touch it now until some real need arises. It does not worth the trouble and added complexity to model something that is not used just for the sake of correctness. By the time we find a use for that, the ISA emulation may evolve so it's easier to implement the missing switching between isa and native mode or we may want to do it differently (such as we do things differently now compared to what we did years ago). So I think it does not worth keeping the ISA model from being simplified for some theoretical uses in the future which we may not actually do any time soon. But I don't want to get into this again so just shared my thoughts and feel free to ignore it. I don't care where these patches go as long as the VIA model keeps working for me. I have a vague memory that ISA compatibility mode was part of the original PCI-BMDMA specification, but it has been a while since I last looked. Bernhard, is there any mention of this in the PIIX datasheet(s)? For reference the cmd646 datasheet specifies that ISA mode or PCI mode is determined by register PROG_IF (0x9) in PCI configuration space. Orthogonal to this discussion, one problem I see here is a device exposing 2 interfaces: ISA and PCI. QOM does support interfaces, but ISA and PCI aren't QOM interfaces. The QOM cast macros are written as a QOM object can only inherit one parent. Should we stick to QDev and convert ISA/PCI as QOM interfaces? That could solve some QDev IDE limitations...
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 05/02/2023 22:21, BALATON Zoltan wrote:
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c | 64 ++--
hw/isa/piix.c | 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+ bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+ PCIIDEState *d = opaque;
+
+ qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+ if (isa_bus) {
+ d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+ d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+ } else {
+ qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+ }
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
- ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
- ISABus *isa_bus;
- bool ambiguous;
+ ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
- isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
- if (ambiguous) {
- error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
- return;
- }
- if (!isa_bus) {
- error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
- return;
+ if (d->user_created) {
+ bool ambiguous;
+
+ isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+ if (ambiguous) {
+ error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ if (!isa_bus) {
+ error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+ DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+ device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info = {
.name = TYPE_PIIX3_IDE,
.parent = TYPE_PCI_IDE,
+ .instance_init = pci_piix_ide_init,
.class_init = piix3_ide_class_init,
};
@@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass *klass, void
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c| 64 ++--
hw/isa/piix.c| 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+PCIIDEState *d = opaque;
+
+qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d,
ISABus *isa_bus)
};
int i;
+if (isa_bus) {
+d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+} else {
+qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+}
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
-ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
-ISABus *isa_bus;
-bool ambiguous;
+ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev,
Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
-isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
));
-if (ambiguous) {
-error_setg(errp,
- "More than one ISA bus found while %s supports only
one",
- object_get_typename(OBJECT(dev)));
-return;
-}
-if (!isa_bus) {
-error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
-return;
+if (d->user_created) {
+bool ambiguous;
+
+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+if (ambiguous) {
+error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
+
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass,
void *data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info = {
.name = TYPE_PIIX3_IDE,
.parent= TYPE_PCI_IDE,
+.instance_init = pci_piix_ide_init,
.class_init= piix3_ide_class_init,
};
@@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass
*klass, void *data)
k->class_id =
Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
On 26/01/2023 21:17, Bernhard Beschow wrote:
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c| 64 ++--
hw/isa/piix.c| 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+PCIIDEState *d = opaque;
+
+qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+if (isa_bus) {
+d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+} else {
+qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+}
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
-ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
-ISABus *isa_bus;
-bool ambiguous;
+ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
-isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, ));
-if (ambiguous) {
-error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
-return;
-}
-if (!isa_bus) {
-error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
-return;
+if (d->user_created) {
+bool ambiguous;
+
+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+if (ambiguous) {
+error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
+
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info = {
.name = TYPE_PIIX3_IDE,
.parent= TYPE_PCI_IDE,
+.instance_init = pci_piix_ide_init,
.class_init= piix3_ide_class_init,
};
@@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_STORAGE_IDE;
[PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances
Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.
Signed-off-by: Bernhard Beschow
---
include/hw/ide/pci.h | 1 +
hw/ide/piix.c| 64 ++--
hw/isa/piix.c| 5
3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
+bool user_created;
};
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+PCIIDEState *d = opaque;
+
+qemu_set_irq(d->isa_irqs[n], level);
+}
+
static void piix_ide_reset(DeviceState *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus
*isa_bus)
};
int i;
+if (isa_bus) {
+d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+} else {
+qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+}
+
for (i = 0; i < 2; i++) {
ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
port_info[i].iobase2);
-ide_init2(>bus[i], isa_bus->irqs[port_info[i].isairq]);
+ide_init2(>bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
{
PCIIDEState *d = PCI_IDE(dev);
uint8_t *pci_conf = dev->config;
-ISABus *isa_bus;
-bool ambiguous;
+ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
**errp)
vmstate_register(VMSTATE_IF(dev), 0, _ide_pci, d);
-isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, ));
-if (ambiguous) {
-error_setg(errp,
- "More than one ISA bus found while %s supports only one",
- object_get_typename(OBJECT(dev)));
-return;
-}
-if (!isa_bus) {
-error_setg(errp, "No ISA bus found while %s requires one",
- object_get_typename(OBJECT(dev)));
-return;
+if (d->user_created) {
+bool ambiguous;
+
+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+ ));
+
+if (ambiguous) {
+error_setg(errp,
+ "More than one ISA bus found while %s supports only
one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
+
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+ object_get_typename(OBJECT(dev)));
+return;
+}
}
pci_piix_init_ports(d, isa_bus);
}
+static void pci_piix_ide_init(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static Property piix_ide_properties[] = {
+DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
}
static const TypeInfo piix3_ide_info = {
.name = TYPE_PIIX3_IDE,
.parent= TYPE_PCI_IDE,
+.instance_init = pci_piix_ide_init,
.class_init= piix3_ide_class_init,
};
@@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
}
