[PATCH 00/12] hw/acpi/piix4: remove legacy piix4_pm_init() function
Whilst reviewing Bernhard's PIIX Southbridge QOMifcation patches at https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg04329.html, I noticed that we should first eliminate the legacy device init function piix4_pm_init(). This series moves the outstanding logic from piix4_pm_init() into the relevant instance init() and realize() functions, changes the IRQs to use qdev gpios, and then finally removes the now-unused piix4_pm_initfn() function. Signed-off-by: Mark Cave-Ayland Mark Cave-Ayland (12): hw/acpi/piix4: move xen_enabled() logic from piix4_pm_init() to piix4_pm_realize() hw/acpi/piix4: change smm_enabled from int to bool hw/acpi/piix4: convert smm_enabled bool to qdev property hw/acpi/piix4: move PIIX4PMState into separate piix4.h header hw/acpi/piix4: alter piix4_pm_init() to return PIIX4PMState hw/acpi/piix4: rename piix4_pm_init() to piix4_pm_initfn() hw/acpi/piix4: introduce piix4_pm_init() instance init function hw/acpi/piix4: use qdev gpio to wire up sci_irq hw/acpi/piix4: use qdev gpio to wire up smi_irq hw/i386/pc_piix: create PIIX4_PM device directly instead of using piix4_pm_initfn() hw/isa/piix4.c: create PIIX4_PM device directly instead of using piix4_pm_initfn() hw/acpi/piix4: remove unused piix4_pm_initfn() function hw/acpi/piix4.c | 77 ++- hw/i386/acpi-build.c | 1 + hw/i386/pc_piix.c | 16 +--- hw/isa/piix4.c| 11 +++-- include/hw/acpi/piix4.h | 75 ++ include/hw/southbridge/piix.h | 6 --- 6 files changed, 107 insertions(+), 79 deletions(-) create mode 100644 include/hw/acpi/piix4.h -- 2.20.1
[PATCH 06/12] hw/acpi/piix4: rename piix4_pm_init() to piix4_pm_initfn()
When QOMifying a device it is typical to use _init() as the suffix for an
instance_init function, however this name is already in use by the legacy
piix4_pm_init() wrapper function. Eventually the wrapper function will be
removed, but for now rename it to piix4_pm_initfn() to avoid a naming
collision.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 6 +++---
hw/i386/pc_piix.c | 6 +++---
hw/isa/piix4.c| 6 +++---
include/hw/southbridge/piix.h | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 05c129b6af..d897d2dee6 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -497,9 +497,9 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
piix4_pm_add_properties(s);
}
-PIIX4PMState *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-qemu_irq sci_irq, qemu_irq smi_irq,
-int smm_enabled)
+PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
+ qemu_irq sci_irq, qemu_irq smi_irq,
+ int smm_enabled)
{
PCIDevice *pci_dev;
DeviceState *dev;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0300be5ef4..ae21946981 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -285,9 +285,9 @@ static void pc_init1(MachineState *machine,
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
/* TODO: Populate SPD eeprom data. */
-piix4_pm = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
- x86ms->gsi[9], smi_irq,
- x86_machine_is_smm_enabled(x86ms));
+piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100,
+ x86ms->gsi[9], smi_irq,
+ x86_machine_is_smm_enabled(x86ms));
pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 7d9bedd1bb..33a7015ea3 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -311,9 +311,9 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
if (smbus) {
-pms = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
-qdev_get_gpio_in_named(dev, "isa", 9),
-NULL, 0);
+pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100,
+ qdev_get_gpio_in_named(dev, "isa", 9),
+ NULL, 0);
*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pms), "i2c"));
}
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 108800ab06..479d88e242 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -16,9 +16,9 @@
#include "qom/object.h"
#include "hw/acpi/piix4.h"
-PIIX4PMState *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-qemu_irq sci_irq, qemu_irq smi_irq,
-int smm_enabled);
+PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
+ qemu_irq sci_irq, qemu_irq smi_irq,
+ int smm_enabled);
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
--
2.20.1
[PATCH 01/12] hw/acpi/piix4: move xen_enabled() logic from piix4_pm_init() to piix4_pm_realize()
This logic can be included as part of piix4_pm_realize() and does not need to
be handled externally.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index fe5625d07a..bf20fa139b 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -525,6 +525,10 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
s->machine_ready.notify = piix4_pm_machine_ready;
qemu_add_machine_init_done_notifier(&s->machine_ready);
+if (xen_enabled()) {
+s->use_acpi_hotplug_bridge = false;
+}
+
piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
pci_get_bus(dev), s);
qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s));
@@ -551,9 +555,6 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
s->irq = sci_irq;
s->smi_irq = smi_irq;
s->smm_enabled = smm_enabled;
-if (xen_enabled()) {
-s->use_acpi_hotplug_bridge = false;
-}
pci_realize_and_unref(pci_dev, bus, &error_fatal);
--
2.20.1
[PATCH 04/12] hw/acpi/piix4: move PIIX4PMState into separate piix4.h header
This allows the QOM types in hw/acpi/piix4.c to be used elsewhere by simply
including
hw/acpi/piix4.h.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 43 +---
hw/i386/acpi-build.c | 1 +
include/hw/acpi/piix4.h | 75 +++
include/hw/southbridge/piix.h | 2 -
4 files changed, 78 insertions(+), 43 deletions(-)
create mode 100644 include/hw/acpi/piix4.h
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 2735ff375e..7ee65b1bff 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -28,6 +28,8 @@
#include "hw/pci/pci.h"
#include "hw/qdev-properties.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/pcihp.h"
+#include "hw/acpi/piix4.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
#include "sysemu/xen.h"
@@ -56,47 +58,6 @@ struct pci_status {
uint32_t down;
};
-struct PIIX4PMState {
-/*< private >*/
-PCIDevice parent_obj;
-/*< public >*/
-
-MemoryRegion io;
-uint32_t io_base;
-
-MemoryRegion io_gpe;
-ACPIREGS ar;
-
-APMState apm;
-
-PMSMBus smb;
-uint32_t smb_io_base;
-
-qemu_irq irq;
-qemu_irq smi_irq;
-bool smm_enabled;
-bool smm_compat;
-Notifier machine_ready;
-Notifier powerdown_notifier;
-
-AcpiPciHpState acpi_pci_hotplug;
-bool use_acpi_hotplug_bridge;
-bool use_acpi_root_pci_hotplug;
-bool not_migrate_acpi_index;
-
-uint8_t disable_s3;
-uint8_t disable_s4;
-uint8_t s4_val;
-
-bool cpu_hotplug_legacy;
-AcpiCpuHotplug gpe_cpu;
-CPUHotplugState cpuhp_state;
-
-MemHotplugState acpi_memory_hotplug;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4PMState, PIIX4_PM)
-
static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
PCIBus *bus, PIIX4PMState *s);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c125939ed6..89ac326d7f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -46,6 +46,7 @@
#include "hw/acpi/tpm.h"
#include "hw/acpi/vmgenid.h"
#include "hw/acpi/erst.h"
+#include "hw/acpi/piix4.h"
#include "sysemu/tpm_backend.h"
#include "hw/rtc/mc146818rtc_regs.h"
#include "migration/vmstate.h"
diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
new file mode 100644
index 00..32686a75c5
--- /dev/null
+++ b/include/hw/acpi/piix4.h
@@ -0,0 +1,75 @@
+/*
+ * ACPI implementation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
[PATCH 08/12] hw/acpi/piix4: use qdev gpio to wire up sci_irq
The sci_irq can now be wired up directly using a qdev gpio instead of having to
set the IRQ externally in piix4_pm_initfn().
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 4 +---
hw/i386/pc_piix.c | 4 ++--
hw/isa/piix4.c| 6 +++---
include/hw/southbridge/piix.h | 3 +--
4 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 454fa34df1..141852b7d4 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -506,8 +506,7 @@ static void piix4_pm_init(Object *obj)
}
PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq, qemu_irq smi_irq,
- int smm_enabled)
+ qemu_irq smi_irq, int smm_enabled)
{
PCIDevice *pci_dev;
DeviceState *dev;
@@ -519,7 +518,6 @@ PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn,
uint32_t smb_io_base,
qdev_prop_set_bit(dev, "smm-enabled", smm_enabled);
s = PIIX4_PM(dev);
-s->irq = sci_irq;
s->smi_irq = smi_irq;
pci_realize_and_unref(pci_dev, bus, &error_fatal);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ae21946981..21e6159166 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -285,9 +285,9 @@ static void pc_init1(MachineState *machine,
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
/* TODO: Populate SPD eeprom data. */
-piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100,
- x86ms->gsi[9], smi_irq,
+piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100, smi_irq,
x86_machine_is_smm_enabled(x86ms));
+qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 33a7015ea3..0b6ea22143 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -311,9 +311,9 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
if (smbus) {
-pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100,
- qdev_get_gpio_in_named(dev, "isa", 9),
- NULL, 0);
+pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100, NULL, 0);
+qdev_connect_gpio_out(DEVICE(pms), 0,
+ qdev_get_gpio_in_named(dev, "isa", 9));
*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pms), "i2c"));
}
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 479d88e242..aefdaebc3f 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -17,8 +17,7 @@
#include "hw/acpi/piix4.h"
PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq, qemu_irq smi_irq,
- int smm_enabled);
+ qemu_irq smi_irq, int smm_enabled);
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
--
2.20.1
[PATCH 02/12] hw/acpi/piix4: change smm_enabled from int to bool
This is in preparation for conversion to a qdev property.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index bf20fa139b..fcfaafc175 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -74,7 +74,7 @@ struct PIIX4PMState {
qemu_irq irq;
qemu_irq smi_irq;
-int smm_enabled;
+bool smm_enabled;
bool smm_compat;
Notifier machine_ready;
Notifier powerdown_notifier;
--
2.20.1
[PATCH 05/12] hw/acpi/piix4: alter piix4_pm_init() to return PIIX4PMState
This exposes the PIIX4_PM device to the caller to allow any qdev gpios to be
mapped outside of piix4_pm_init().
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 11 ---
hw/i386/pc_piix.c | 10 +-
hw/isa/piix4.c| 8 +---
include/hw/southbridge/piix.h | 7 ---
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 7ee65b1bff..05c129b6af 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -497,9 +497,9 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
piix4_pm_add_properties(s);
}
-I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq, qemu_irq smi_irq,
- int smm_enabled, DeviceState **piix4_pm)
+PIIX4PMState *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
+qemu_irq sci_irq, qemu_irq smi_irq,
+int smm_enabled)
{
PCIDevice *pci_dev;
DeviceState *dev;
@@ -509,9 +509,6 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
dev = DEVICE(pci_dev);
qdev_prop_set_uint32(dev, "smb_io_base", smb_io_base);
qdev_prop_set_bit(dev, "smm-enabled", smm_enabled);
-if (piix4_pm) {
-*piix4_pm = dev;
-}
s = PIIX4_PM(dev);
s->irq = sci_irq;
@@ -519,7 +516,7 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
pci_realize_and_unref(pci_dev, bus, &error_fatal);
-return s->smb.smbus;
+return s;
}
static uint64_t gpe_readb(void *opaque, hwaddr addr, unsigned width)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 578e537b35..0300be5ef4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -281,14 +281,14 @@ static void pc_init1(MachineState *machine,
}
if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
-DeviceState *piix4_pm;
+PIIX4PMState *piix4_pm;
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
/* TODO: Populate SPD eeprom data. */
-pcms->smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
-x86ms->gsi[9], smi_irq,
-x86_machine_is_smm_enabled(x86ms),
-&piix4_pm);
+piix4_pm = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
+ x86ms->gsi[9], smi_irq,
+ x86_machine_is_smm_enabled(x86ms));
+pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 8607e0ac36..7d9bedd1bb 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -293,6 +293,7 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int
irq_num)
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
PIIX4State *s;
+PIIX4PMState *pms;
PCIDevice *pci;
DeviceState *dev;
int devfn = PCI_DEVFN(10, 0);
@@ -310,9 +311,10 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
if (smbus) {
-*smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
- qdev_get_gpio_in_named(dev, "isa", 9),
- NULL, 0, NULL);
+pms = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
+qdev_get_gpio_in_named(dev, "isa", 9),
+NULL, 0);
+*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pms), "i2c"));
}
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index c5b842b45d..108800ab06 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,10 +14,11 @@
#include "hw/pci/pci.h"
#include "qom/object.h"
+#include "hw/acpi/piix4.h"
-I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq, qemu_irq smi_irq,
- int smm_enabled, DeviceState **piix4_pm);
+PIIX4PMState *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
+qemu_irq sci_irq, qemu_irq smi_irq,
+int smm_enabled);
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
--
2.20.1
[PATCH 10/12] hw/i386/pc_piix: create PIIX4_PM device directly instead of using piix4_pm_initfn()
Now that all external logic has been removed from piix4_pm_initfn() the PIIX4_PM
device can be instantiated directly.
Signed-off-by: Mark Cave-Ayland
---
hw/i386/pc_piix.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 82696fc707..4120fd52e5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -47,6 +47,7 @@
#include "hw/xen/xen-x86.h"
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/piix4.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/xen.h"
@@ -281,12 +282,16 @@ static void pc_init1(MachineState *machine,
}
if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
-PIIX4PMState *piix4_pm;
+PCIDevice *piix4_pm;
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
/* TODO: Populate SPD eeprom data. */
-piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100,
- x86_machine_is_smm_enabled(x86ms));
+piix4_pm = pci_new(piix3_devfn + 3, TYPE_PIIX4_PM);
+qdev_prop_set_uint32(DEVICE(piix4_pm), "smb_io_base", 0xb100);
+qdev_prop_set_bit(DEVICE(piix4_pm), "smm-enabled",
+ x86_machine_is_smm_enabled(x86ms));
+pci_realize_and_unref(piix4_pm, pci_bus, &error_fatal);
+
qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
--
2.20.1
[PATCH 03/12] hw/acpi/piix4: convert smm_enabled bool to qdev property
This allows the smm_enabled value to be set using a standard qdev property
instead
of being referenced directly in piix4_pm_init().
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index fcfaafc175..2735ff375e 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -547,6 +547,7 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
pci_dev = pci_new(devfn, TYPE_PIIX4_PM);
dev = DEVICE(pci_dev);
qdev_prop_set_uint32(dev, "smb_io_base", smb_io_base);
+qdev_prop_set_bit(dev, "smm-enabled", smm_enabled);
if (piix4_pm) {
*piix4_pm = dev;
}
@@ -554,7 +555,6 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
s = PIIX4_PM(dev);
s->irq = sci_irq;
s->smi_irq = smi_irq;
-s->smm_enabled = smm_enabled;
pci_realize_and_unref(pci_dev, bus, &error_fatal);
@@ -664,6 +664,7 @@ static Property piix4_pm_properties[] = {
DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
acpi_memory_hotplug.is_enabled, true),
DEFINE_PROP_BOOL("smm-compat", PIIX4PMState, smm_compat, false),
+DEFINE_PROP_BOOL("smm-enabled", PIIX4PMState, smm_enabled, false),
DEFINE_PROP_BOOL("x-not-migrate-acpi-index", PIIX4PMState,
not_migrate_acpi_index, false),
DEFINE_PROP_END_OF_LIST(),
--
2.20.1
[PATCH 09/12] hw/acpi/piix4: use qdev gpio to wire up smi_irq
The smi_irq can now be wired up directly using a qdev gpio instead of having to
set the IRQ externally in piix4_pm_initfn().
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 3 +--
hw/i386/pc_piix.c | 3 ++-
hw/isa/piix4.c| 2 +-
include/hw/southbridge/piix.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 141852b7d4..fd20e1eccc 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -506,7 +506,7 @@ static void piix4_pm_init(Object *obj)
}
PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq smi_irq, int smm_enabled)
+ int smm_enabled)
{
PCIDevice *pci_dev;
DeviceState *dev;
@@ -518,7 +518,6 @@ PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn,
uint32_t smb_io_base,
qdev_prop_set_bit(dev, "smm-enabled", smm_enabled);
s = PIIX4_PM(dev);
-s->smi_irq = smi_irq;
pci_realize_and_unref(pci_dev, bus, &error_fatal);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 21e6159166..82696fc707 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -285,9 +285,10 @@ static void pc_init1(MachineState *machine,
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
/* TODO: Populate SPD eeprom data. */
-piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100, smi_irq,
+piix4_pm = piix4_pm_initfn(pci_bus, piix3_devfn + 3, 0xb100,
x86_machine_is_smm_enabled(x86ms));
qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
+qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0b6ea22143..775e15eb20 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -311,7 +311,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
if (smbus) {
-pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100, NULL, 0);
+pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100, 0);
qdev_connect_gpio_out(DEVICE(pms), 0,
qdev_get_gpio_in_named(dev, "isa", 9));
*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pms), "i2c"));
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index aefdaebc3f..d284475f07 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -17,7 +17,7 @@
#include "hw/acpi/piix4.h"
PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq smi_irq, int smm_enabled);
+ int smm_enabled);
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
--
2.20.1
[PATCH 11/12] hw/isa/piix4.c: create PIIX4_PM device directly instead of using piix4_pm_initfn()
Now that all external logic has been removed from piix4_pm_initfn() the PIIX4_PM
device can be instantiated directly.
Signed-off-by: Mark Cave-Ayland
---
hw/isa/piix4.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 775e15eb20..9a6d981037 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -34,6 +34,7 @@
#include "hw/timer/i8254.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/ide/pci.h"
+#include "hw/acpi/piix4.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
#include "sysemu/runstate.h"
@@ -293,7 +294,6 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int
irq_num)
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
PIIX4State *s;
-PIIX4PMState *pms;
PCIDevice *pci;
DeviceState *dev;
int devfn = PCI_DEVFN(10, 0);
@@ -311,10 +311,13 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
if (smbus) {
-pms = piix4_pm_initfn(pci_bus, devfn + 3, 0x1100, 0);
-qdev_connect_gpio_out(DEVICE(pms), 0,
+pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
+qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
+qdev_prop_set_bit(DEVICE(pci), "smm-enabled", 0);
+pci_realize_and_unref(pci, pci_bus, &error_fatal);
+qdev_connect_gpio_out(DEVICE(pci), 0,
qdev_get_gpio_in_named(dev, "isa", 9));
-*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pms), "i2c"));
+*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pci), "i2c"));
}
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
--
2.20.1
[PATCH v2 2/3] hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa
The hppa_hardware.h header file holds many constants for addresses and offsets which are needed while building the firmware (SeaBIOS-hppa) and while setting up the virtual machine in QEMU. That's why this header file needs to be in sync between both source code repositories. This patch adds a comment mentioning this dependency at the top of this file and restores some DINO relevant offsets. Signed-off-by: Helge Deller --- hw/hppa/hppa_hardware.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h index 8b6b9222cb..37bafff1ed 100644 --- a/hw/hppa/hppa_hardware.h +++ b/hw/hppa/hppa_hardware.h @@ -1,4 +1,5 @@ /* HPPA cores and system support chips. */ +/* Be aware: QEMU and seabios-hppa repositories share this files as-is. */ #ifndef HW_HPPA_HPPA_HARDWARE_H #define HW_HPPA_HPPA_HARDWARE_H @@ -30,6 +31,11 @@ #define PCI_HPA DINO_HPA/* PCI bus */ #define IDE_HPA 0xf900 /* Boot disc controller */ +/* offsets to DINO HPA: */ +#define DINO_PCI_ADDR 0x064 +#define DINO_CONFIG_DATA0x068 +#define DINO_IO_DATA0x06c + #define PORT_PCI_CMD(PCI_HPA + DINO_PCI_ADDR) #define PORT_PCI_DATA (PCI_HPA + DINO_CONFIG_DATA) -- 2.35.3
[PATCH 07/12] hw/acpi/piix4: introduce piix4_pm_init() instance init function
Use the new piix4_pm_init() instance init function to initialise 2 separate qdev
gpios for the SCI and SMI IRQs.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index d897d2dee6..454fa34df1 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -497,6 +497,14 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
piix4_pm_add_properties(s);
}
+static void piix4_pm_init(Object *obj)
+{
+PIIX4PMState *s = PIIX4_PM(obj);
+
+qdev_init_gpio_out(DEVICE(obj), &s->irq, 1);
+qdev_init_gpio_out_named(DEVICE(obj), &s->smi_irq, "smi-irq", 1);
+}
+
PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
qemu_irq sci_irq, qemu_irq smi_irq,
int smm_enabled)
@@ -663,6 +671,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void
*data)
static const TypeInfo piix4_pm_info = {
.name = TYPE_PIIX4_PM,
.parent= TYPE_PCI_DEVICE,
+.instance_init = piix4_pm_init,
.instance_size = sizeof(PIIX4PMState),
.class_init= piix4_pm_class_init,
.interfaces = (InterfaceInfo[]) {
--
2.20.1
[PATCH v2 3/3] hppa: Fix serial port assignments and pass-through
This fixes the serial ports in the emulation to behave as on original
hardware.
On the real hardware, the LASI UART is serial port #0 and the DINO UART
is serial port #1. This is fixed in SeaBIOS-hppa firmware v6, which is
why at least this firmware version is required.
The serial port addresses in hppa/hppa_hardware.h have to be swapped,
and when creating the virtual serial ports the correct port addresses
are used.
This patch now for example allows to specify on the qemu command line:
-serial mon:stdio -serial /dev/ttyS4
to use the emulated ttyS0 in the guest for console output, and pass
ttyS4 from the host to ttyS1 in the guest.
Signed-off-by: Helge Deller
---
hw/hppa/hppa_hardware.h | 4 ++--
hw/hppa/machine.c | 22 --
2 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h
index 37bafff1ed..e4b0b142d9 100644
--- a/hw/hppa/hppa_hardware.h
+++ b/hw/hppa/hppa_hardware.h
@@ -41,8 +41,8 @@
#define FW_CFG_IO_BASE 0xfffa
-#define PORT_SERIAL1(DINO_UART_HPA + 0x800)
-#define PORT_SERIAL2(LASI_UART_HPA + 0x800)
+#define PORT_SERIAL1(LASI_UART_HPA + 0x800)
+#define PORT_SERIAL2(DINO_UART_HPA + 0x800)
#define HPPA_MAX_CPUS 16 /* max. number of SMP CPUs */
#define CPU_CLOCK_MHZ 250 /* emulate a 250 MHz CPU */
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index d1e174b1f4..63b9dd2396 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -32,7 +32,7 @@
#define MAX_IDE_BUS 2
-#define MIN_SEABIOS_HPPA_VERSION 1 /* require at least this fw version */
+#define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */
#define HPA_POWER_BUTTON (FIRMWARE_END - 0x10)
@@ -236,20 +236,14 @@ static void machine_hppa_init(MachineState *machine)
/* Realtime clock, used by firmware for PDC_TOD call. */
mc146818_rtc_init(isa_bus, 2000, NULL);
-/* Serial code setup. */
-if (serial_hd(0)) {
-uint32_t addr = DINO_UART_HPA + 0x800;
-serial_mm_init(addr_space, addr, 0,
- qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT),
- 115200, serial_hd(0), DEVICE_BIG_ENDIAN);
-}
+/* Serial ports: Lasi and Dino use a 7.272727 MHz clock. */
+serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
+qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 7272727 / 16,
+serial_hd(0), DEVICE_BIG_ENDIAN);
-if (serial_hd(1)) {
-/* Serial port */
-serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
-qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 800 / 16,
-serial_hd(1), DEVICE_BIG_ENDIAN);
-}
+serial_mm_init(addr_space, DINO_UART_HPA + 0x800, 0,
+qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT), 7272727 / 16,
+serial_hd(1), DEVICE_BIG_ENDIAN);
/* Parallel port */
parallel_mm_init(addr_space, LASI_LPT_HPA + 0x800, 0,
--
2.35.3
[PATCH 12/12] hw/acpi/piix4: remove unused piix4_pm_initfn() function
This function is now unused and so can be completely removed.
Signed-off-by: Mark Cave-Ayland
---
hw/acpi/piix4.c | 19 ---
include/hw/southbridge/piix.h | 4
2 files changed, 23 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index fd20e1eccc..0a81f1ad93 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -505,25 +505,6 @@ static void piix4_pm_init(Object *obj)
qdev_init_gpio_out_named(DEVICE(obj), &s->smi_irq, "smi-irq", 1);
}
-PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- int smm_enabled)
-{
-PCIDevice *pci_dev;
-DeviceState *dev;
-PIIX4PMState *s;
-
-pci_dev = pci_new(devfn, TYPE_PIIX4_PM);
-dev = DEVICE(pci_dev);
-qdev_prop_set_uint32(dev, "smb_io_base", smb_io_base);
-qdev_prop_set_bit(dev, "smm-enabled", smm_enabled);
-
-s = PIIX4_PM(dev);
-
-pci_realize_and_unref(pci_dev, bus, &error_fatal);
-
-return s;
-}
-
static uint64_t gpe_readb(void *opaque, hwaddr addr, unsigned width)
{
PIIX4PMState *s = opaque;
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index d284475f07..976b4da582 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,10 +14,6 @@
#include "hw/pci/pci.h"
#include "qom/object.h"
-#include "hw/acpi/piix4.h"
-
-PIIX4PMState *piix4_pm_initfn(PCIBus *bus, int devfn, uint32_t smb_io_base,
- int smm_enabled);
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
--
2.20.1
[PATCH v2 0/3] hppa: Fix serial port pass-through
This series fixes the SeaBIOS-hppa firmware and the serial ports setup code in qemu so that it reflects the real hardware and allows serial port pass-through from the host to guests. Tested with Linux guests. v2: Changes suggested by: Mark Cave-Ayland - Split out hppa_hardware.h restoration to an own patch - Drop unneccesary checks for serial_hd(x) Signed-off-by: Helge Deller Helge Deller (3): New SeaBIOS-hppa version 6 hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa hppa: Fix serial port assignments and pass-through hw/hppa/hppa_hardware.h | 10 -- hw/hppa/machine.c | 22 -- pc-bios/hppa-firmware.img | Bin 719040 -> 719368 bytes roms/seabios-hppa | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) -- 2.35.3
Re: [PATCH v2 5/6] hw/isa/piix4: QOM'ify PIIX4 PM creation
On 25/05/2022 19:09, Mark Cave-Ayland wrote:
On 22/05/2022 22:24, Bernhard Beschow wrote:
Just like the real hardware, create the PIIX4 ACPI controller as part of
the PIIX4 southbridge. This also mirrors how the IDE and USB functions
are already created.
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 14 +++---
hw/mips/malta.c | 3 ++-
include/hw/southbridge/piix.h | 2 +-
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 4968c69da9..1645f63450 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -206,6 +206,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
PIIX4State *s = PIIX4_PCI_DEVICE(dev);
PCIDevice *pci;
PCIBus *pci_bus = pci_get_bus(dev);
+ I2CBus *smbus;
ISABus *isa_bus;
qemu_irq *i8259_out_irq;
@@ -252,6 +253,11 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
/* USB */
pci_create_simple(pci_bus, dev->devfn + 2, "piix4-usb-uhci");
+ /* ACPI controller */
+ smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100, s->isa[9],
+ NULL, 0, NULL);
+ object_property_add_const_link(OBJECT(s), "smbus", OBJECT(smbus));
+
Interesting hack here to expose the smbus so it is available to qdev_get_child_bus(),
but really this is still really working around the fact that piix4_pm_init() itself
should be removed first. Once that is done, you can then use a standard QOM pattern
to initialise the "internal" PCI devices via object_initialize_child() and realize
them in piix4_realize() instead of using pci_create_simple().
Is that something you could take a look at? If not, I may be able to put something
together towards the end of the week. Other than that I think the rest of the series
looks good.
Hi Bernhard,
I've just sent through the series for eliminating the piix4_pm_init() which should
allow you to improve the QOMifcation done as part of this series.
A bit of background as to why this is necessary: when building a container device
such as the PIIX southbridge, there should still be 2 distinct init and realize
phases. In effect this needs to be done depth-first so when you init the PIIX4
southbridge, the instance init function should also init the contained PCI devices
such as IDE and USB. Similarly when the PIIX4 device is realized, its realize
function should then realize the contained PCI devices using qdev_realize().
This is one of the main reasons why legacy global device init functions aren't
recommended, since they both init *and* realize the device before returning it which
immediately breaks this contract.
The normal way to initialise a contained device is to use object_initialize_child()
in the container's instance init function and to store the the child device instance
within the container. But what this also means is that you shouldn't use any _new()
functions like pci_new() or pci_create_simple() to instantiated contained devices in
a container realize function either. So the next question: how is this done?
Fortunately there is an existing example of this: take a look at how this is
currently done in hw/pci-host/q35.c's q35_host_initfn() and q35_host_realize() for
the MCH_PCI_DEVICE device.
I hope this gives you all the information you need to produce a v3 of the series: if
you have any further questions, do let me know and I will do my best to answer them.
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s,
PIIX_NUM_PIRQS);
}
@@ -301,7 +307,7 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
-DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
+DeviceState *piix4_create(PCIBus *pci_bus)
{
PCIDevice *pci;
DeviceState *dev;
@@ -311,11 +317,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
- if (smbus) {
- *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
- qdev_get_gpio_in_named(dev, "isa", 9),
- NULL, 0, NULL);
- }
-
return dev;
}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index e446b25ad0..b0fc84ccbb 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1399,8 +1399,9 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
- dev = piix4_create(pci_bus, &smbus);
+ dev = piix4_create(pci_bus);
isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+ smbus = I2C_BUS(qdev_get_child_bus(dev, "smbus"));
/* Interrupt controller */
qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bec7f8ca3..2c21359efa 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -76,6 +76,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, P
Re: [PATCH v2 0/3] hppa: Fix serial port pass-through
On 28/05/2022 10:41, Helge Deller wrote: This series fixes the SeaBIOS-hppa firmware and the serial ports setup code in qemu so that it reflects the real hardware and allows serial port pass-through from the host to guests. Tested with Linux guests. v2: Changes suggested by: Mark Cave-Ayland - Split out hppa_hardware.h restoration to an own patch - Drop unneccesary checks for serial_hd(x) Signed-off-by: Helge Deller Helge Deller (3): New SeaBIOS-hppa version 6 hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa hppa: Fix serial port assignments and pass-through hw/hppa/hppa_hardware.h | 10 -- hw/hppa/machine.c | 22 -- pc-bios/hppa-firmware.img | Bin 719040 -> 719368 bytes roms/seabios-hppa | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) Looks good to me :) Reviewed-by: Mark Cave-Ayland ATB, Mark.
[PULL 2/3] hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa
The hppa_hardware.h header file holds many constants for addresses and offsets which are needed while building the firmware (SeaBIOS-hppa) and while setting up the virtual machine in QEMU. That's why this header file needs to be in sync between both source code repositories. This patch adds a comment mentioning this dependency at the top of this file and restores some DINO relevant offsets. Signed-off-by: Helge Deller Reviewed-by: Mark Cave-Ayland --- hw/hppa/hppa_hardware.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h index 8b6b9222cb..3f7627b98f 100644 --- a/hw/hppa/hppa_hardware.h +++ b/hw/hppa/hppa_hardware.h @@ -1,4 +1,5 @@ /* HPPA cores and system support chips. */ +/* Be aware: QEMU and seabios-hppa repositories share this file as-is. */ #ifndef HW_HPPA_HPPA_HARDWARE_H #define HW_HPPA_HPPA_HARDWARE_H @@ -30,6 +31,11 @@ #define PCI_HPA DINO_HPA/* PCI bus */ #define IDE_HPA 0xf900 /* Boot disc controller */ +/* offsets to DINO HPA: */ +#define DINO_PCI_ADDR 0x064 +#define DINO_CONFIG_DATA0x068 +#define DINO_IO_DATA0x06c + #define PORT_PCI_CMD(PCI_HPA + DINO_PCI_ADDR) #define PORT_PCI_DATA (PCI_HPA + DINO_CONFIG_DATA) -- 2.35.3
[PULL 3/3] hppa: Fix serial port assignments and pass-through
This fixes the serial ports in the emulation to behave as on original
hardware.
On the real hardware, the LASI UART is serial port #0 and the DINO UART
is serial port #1. This is fixed in SeaBIOS-hppa firmware v6, which is
why at least this firmware version is required.
The serial port addresses in hppa/hppa_hardware.h have to be swapped,
and when creating the virtual serial ports the correct port addresses
are used.
This patch now for example allows to specify on the qemu command line:
-serial mon:stdio -serial /dev/ttyS4
to use the emulated ttyS0 in the guest for console output, and pass
ttyS4 from the host to ttyS1 in the guest.
Signed-off-by: Helge Deller
Reviewed-by: Mark Cave-Ayland
---
hw/hppa/hppa_hardware.h | 4 ++--
hw/hppa/machine.c | 22 --
2 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h
index 3f7627b98f..a5ac3dd0fd 100644
--- a/hw/hppa/hppa_hardware.h
+++ b/hw/hppa/hppa_hardware.h
@@ -41,8 +41,8 @@
#define FW_CFG_IO_BASE 0xfffa
-#define PORT_SERIAL1(DINO_UART_HPA + 0x800)
-#define PORT_SERIAL2(LASI_UART_HPA + 0x800)
+#define PORT_SERIAL1(LASI_UART_HPA + 0x800)
+#define PORT_SERIAL2(DINO_UART_HPA + 0x800)
#define HPPA_MAX_CPUS 16 /* max. number of SMP CPUs */
#define CPU_CLOCK_MHZ 250 /* emulate a 250 MHz CPU */
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index d1e174b1f4..63b9dd2396 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -32,7 +32,7 @@
#define MAX_IDE_BUS 2
-#define MIN_SEABIOS_HPPA_VERSION 1 /* require at least this fw version */
+#define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */
#define HPA_POWER_BUTTON (FIRMWARE_END - 0x10)
@@ -236,20 +236,14 @@ static void machine_hppa_init(MachineState *machine)
/* Realtime clock, used by firmware for PDC_TOD call. */
mc146818_rtc_init(isa_bus, 2000, NULL);
-/* Serial code setup. */
-if (serial_hd(0)) {
-uint32_t addr = DINO_UART_HPA + 0x800;
-serial_mm_init(addr_space, addr, 0,
- qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT),
- 115200, serial_hd(0), DEVICE_BIG_ENDIAN);
-}
+/* Serial ports: Lasi and Dino use a 7.272727 MHz clock. */
+serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
+qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 7272727 / 16,
+serial_hd(0), DEVICE_BIG_ENDIAN);
-if (serial_hd(1)) {
-/* Serial port */
-serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
-qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 800 / 16,
-serial_hd(1), DEVICE_BIG_ENDIAN);
-}
+serial_mm_init(addr_space, DINO_UART_HPA + 0x800, 0,
+qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT), 7272727 / 16,
+serial_hd(1), DEVICE_BIG_ENDIAN);
/* Parallel port */
parallel_mm_init(addr_space, LASI_LPT_HPA + 0x800, 0,
--
2.35.3
[PULL 0/3] Hppa serial fix patches
The following changes since commit 58b53669e87fed0d70903e05cd42079fbbdbc195: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-05-25 13:46:29 -0700) are available in the Git repository at: https://github.com/hdeller/qemu-hppa.git tags/hppa-serial-fix-pull-request for you to fetch changes up to 5079892df5f113c7f2b77f53bf7663f6c7bc6be9: hppa: Fix serial port assignments and pass-through (2022-05-28 12:25:42 +0200) hppa: Fix serial port pass-through This series fixes the SeaBIOS-hppa firmware and the serial ports setup code in qemu so that it reflects the real hardware and allows serial port pass-through from the host to guests. Tested with Linux guests. v2: Changes suggested by: Mark Cave-Ayland - Split out hppa_hardware.h restoration to an own patch - Drop unneccesary checks for serial_hd(x) Signed-off-by: Helge Deller Reviewed-by: Mark Cave-Ayland Helge Deller (3): New SeaBIOS-hppa version 6 hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa hppa: Fix serial port assignments and pass-through hw/hppa/hppa_hardware.h | 10 -- hw/hppa/machine.c | 22 -- pc-bios/hppa-firmware.img | Bin 719040 -> 719368 bytes roms/seabios-hppa | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) -- 2.35.3
[PATCH v3] linux-user: Adjust child_tidptr on set_tid_address() syscall
Keep track of the new child tidptr given by a set_tid_address() syscall.
Do not call the host set_tid_address() syscall because we are emulating
the behaviour of writing to child_tidptr in the exit() path.
Signed-off-by: Helge Deller
Reviewed-by: Richard Henderson
--
v3:
- Respin of the patch because the v2 version was mungled in-between the
mail of the v1 version. Now it's possible to get correct patch with b4
- Rephrased commit message
- Added Richard's Reviewed-by
v2:
- was mungled in v1 mail thread
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f55cdebee5..1166e9f014 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -320,9 +320,6 @@ _syscall3(int,sys_syslog,int,type,char*,bufp,int,len)
#ifdef __NR_exit_group
_syscall1(int,exit_group,int,error_code)
#endif
-#if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address)
-_syscall1(int,set_tid_address,int *,tidptr)
-#endif
#if defined(__NR_futex)
_syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
const struct timespec *,timeout,int *,uaddr2,int,val3)
@@ -12200,9 +12197,14 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
}
#endif
-#if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address)
+#if defined(TARGET_NR_set_tid_address)
case TARGET_NR_set_tid_address:
-return get_errno(set_tid_address((int *)g2h(cpu, arg1)));
+{
+TaskState *ts = cpu->opaque;
+ts->child_tidptr = arg1;
+/* do not call host set_tid_address() syscall, instead return tid() */
+return get_errno(sys_gettid());
+}
#endif
case TARGET_NR_tkill:
Re: [PULL 0/9] Block patches
On Thu, May 12, 2022 at 12:29 AM Philippe Mathieu-Daudé wrote: > > Hi Stefan, Nicolas, > > > Nicolas Saenz Julienne (3): > > Introduce event-loop-base abstract class > > util/main-loop: Introduce the main loop into QOM > > util/event-loop-base: Introduce options to set the thread pool size > > I'm getting this warning on Darwin: > > ... > [379/1097] Linking static target libevent-loop-base.a > warning: > /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: > archive library: libevent-loop-base.a the table of contents is empty > (no object file members in the library define global symbols) > ... Opened https://gitlab.com/qemu-project/qemu/-/issues/1044 to track this.
[PATCH] linux-user: fix memory leak when threads exit
From: kkHAIKE when call do_fork->cpu_copy->cpu_create, the return new cpu was not parent so refby '/unattached', so need add more object_unparent call to unref. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/866 Signed-off-by: kkHAIKE --- linux-user/syscall.c | 1 + 1 file changed, 1 insertion(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index f55cdebee5..c653897d32 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8567,6 +8567,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, TaskState *ts = cpu->opaque; object_property_set_bool(OBJECT(cpu), "realized", false, NULL); +object_unparent(OBJECT(cpu)); object_unref(OBJECT(cpu)); /* * At this point the CPU should be unrealized and removed -- 2.32.1 (Apple Git-133)
Re: [PATCH 2/2] hppa: Fix serial port pass-through
On Thu, 26 May 2022 at 12:52, Helge Deller wrote:
>
> This fixes the serial ports in the emulation to behave as on original
> hardware.
>
> On the real hardware, the LASI UART is serial port #0 and the DINO UART
> is serial port #1. This is fixed in SEABIOS_HPPA_VERSION >= 6, which is
> why the latest firmware is required.
>
> The serial port addresses in hppa/hppa_hardware.h are swapped and the
> file is synced with the version in SeaBIOS-hppa. Please note that this
> file is shared between qemu and SeaBIOS-hppa, which is why a comment was
> added at the top of the file.
>
> When creating the virtual serial ports the correct port addresses are
> now used.
>
> Finally, this patch now allows the user to give:
> -serial mon:stdio -serial /dev/ttyS4
> to use emulated ttyS0 in the guest for console output, and pass ttyS4
> from the host to ttyS1 in the guest.
>
> Signed-off-by: Helge Deller
> @@ -236,18 +236,15 @@ static void machine_hppa_init(MachineState *machine)
> /* Realtime clock, used by firmware for PDC_TOD call. */
> mc146818_rtc_init(isa_bus, 2000, NULL);
>
> -/* Serial code setup. */
> +/* Serial ports - Lasi and Dino use a 7.272727 MHz clock. */
> if (serial_hd(0)) {
> -uint32_t addr = DINO_UART_HPA + 0x800;
> -serial_mm_init(addr_space, addr, 0,
> - qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT),
> - 115200, serial_hd(0), DEVICE_BIG_ENDIAN);
> +serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
> +qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 7272727 / 16,
> +serial_hd(0), DEVICE_BIG_ENDIAN);
> }
> -
> if (serial_hd(1)) {
> -/* Serial port */
> -serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
> -qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 800 / 16,
> +serial_mm_init(addr_space, DINO_UART_HPA + 0x800, 0,
> +qdev_get_gpio_in(dino_dev, DINO_IRQ_RS232INT), 7272727 / 16,
> serial_hd(1), DEVICE_BIG_ENDIAN);
> }
Not related to this change, but you should consider removing these
"if (serial_hd(n))" conditionals. They used to be necessary because
the chardev backend infrastructure couldn't cope with being handed
a NULL pointer, but these days that is guaranteed to work (with
a NULL pointer being equivalent to the 'throw everything away'
null chardev), so the if()s are no longer necessary. They also
mean that the guest sees the hardware it expects, ie the UARTs
are always there and programmable whether there's an RS232 cable
plugged in the back of the machine or not.
thanks
-- PMM
Re: [PATCH 2/2] hppa: Fix serial port pass-through
On Sat, 28 May 2022 at 15:24, Peter Maydell wrote: > Not related to this change, but you should consider removing these > "if (serial_hd(n))" conditionals. ...oops, I just saw the 3/3 patch in your pullreq which does exactly that, so ignore my email :-) -- PMM
Re: [PATCH 2/2] hppa: Fix serial port pass-through
On 5/28/22 16:25, Peter Maydell wrote: > On Sat, 28 May 2022 at 15:24, Peter Maydell wrote: >> Not related to this change, but you should consider removing these >> "if (serial_hd(n))" conditionals. > > ...oops, I just saw the 3/3 patch in your pullreq which does > exactly that, so ignore my email :-) Yes, in v3 (and the pull request) I fixed that since Mark had the same suggestion as you. Helge
Re: [PATCH v5 04/17] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15
Le 27/05/2022 à 18:47, Richard Henderson a écrit : These are raised by guest instructions, and should not fall through into the default abort case. Signed-off-by: Richard Henderson --- linux-user/m68k/cpu_loop.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c index 56417f7401..12e5d9cd53 100644 --- a/linux-user/m68k/cpu_loop.c +++ b/linux-user/m68k/cpu_loop.c @@ -75,7 +75,11 @@ void cpu_loop(CPUM68KState *env) case EXCP_INTERRUPT: /* just indicate that signals should be handled asap */ break; +case EXCP_TRAP0 + 1 ... EXCP_TRAP0 + 14: +force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP, env->pc); +break; case EXCP_DEBUG: +case EXCP_TRAP15: force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->pc); break; case EXCP_ATOMIC: Reviewed-by: Laurent Vivier
Re: [PATCH v5 15/17] linux-user/strace: Use is_error in print_syscall_err
Le 27/05/2022 à 18:48, Richard Henderson a écrit :
Errors are not all negative numbers: use is_error.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
linux-user/strace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 9fa681dea9..7d882526da 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -689,7 +689,7 @@ print_syscall_err(abi_long ret)
const char *errstr;
qemu_log(" = ");
-if (ret < 0) {
+if (is_error(ret)) {
errstr = target_strerror(-ret);
if (errstr) {
qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
Reviewed-by: Laurent Vivier
Re: [PATCH v5 00/17] target/m68k: Conditional traps + trap cleanup
Le 27/05/2022 à 18:47, Richard Henderson a écrit : Changes for v4: - Use ILLTRP for TRAP1-TRAP14. - Use is_error for print_syscall_err. r~ v1: https://lore.kernel.org/qemu-devel/20211130103752.72099-1-richard.hender...@linaro.org/ v2: https://lore.kernel.org/qemu-devel/20211202204900.50973-1-richard.hender...@linaro.org/ v3: https://lore.kernel.org/qemu-devel/20220316055840.727571-1-richard.hender...@linaro.org/ v4: https://lore.kernel.org/qemu-devel/20220430175342.370628-1-richard.hender...@linaro.org/ Richard Henderson (17): target/m68k: Raise the TRAPn exception with the correct pc target/m68k: Switch over exception type in m68k_interrupt_all target/m68k: Fix coding style in m68k_interrupt_all linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15 target/m68k: Remove retaddr in m68k_interrupt_all target/m68k: Fix address argument for EXCP_CHK target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0 target/m68k: Fix address argument for EXCP_TRACE target/m68k: Fix stack frame for EXCP_ILLEGAL target/m68k: Implement TRAPcc target/m68k: Implement TPF in terms of TRAPcc target/m68k: Implement TRAPV target/m68k: Implement FTRAPcc tests/tcg/m68k: Add trap.c linux-user/strace: Use is_error in print_syscall_err linux-user/strace: Adjust get_thread_area for m68k target/m68k: Mark helper_raise_exception as noreturn target/m68k/cpu.h | 8 ++ target/m68k/helper.h | 14 +-- linux-user/m68k/cpu_loop.c | 13 ++- linux-user/strace.c| 2 +- target/m68k/cpu.c | 1 + target/m68k/op_helper.c| 173 -- target/m68k/translate.c| 190 - tests/tcg/m68k/trap.c | 129 ++ linux-user/strace.list | 5 + tests/tcg/m68k/Makefile.target | 3 + 10 files changed, 395 insertions(+), 143 deletions(-) create mode 100644 tests/tcg/m68k/trap.c Series applied to my m68k-for-7.1 branch Thanks, Laurent
Re: [PATCH 0/2] backend/tpm: Resolve issue with TPM 2 DA lockout
On 5/27/22 15:31, Stefan Berger wrote: On 5/27/22 15:24, Marc-André Lureau wrote: Hi On Fri, May 27, 2022 at 7:36 PM Stefan Berger wrote: This series of patches resolves an issue with a TPM 2's dictionary attack lockout logic being triggered upon well-timed VM resets. Normally, the OS TPM driver sends a TPM2_Shutdown to the TPM 2 upon reboot and before a VM is reset. However, the OS driver cannot do this when the user resets a VM. In this case QEMU must send the command because otherwise several well- timed VM resets will trigger the TPM 2's dictionary attack (DA) logic and it will then refuse to do certain key-related operations until the DA logic has timed out. How does real hardware deal with that situation? Shouldn't this "shutdown"/reset logic be implemented on swtpm side instead, when CMD_INIT is received? (when the VM is restarted) I don't know what real hardware can actually do when the machine is reset, presumably via some reset line, or the power is removed. Probably it has no way to react to this. Typically the OS driver has to send the command and since it cannot do this I would defer it to the TPM emulator reset handler code, so the next layer down. Also, when this is done in QEMU we don't need to do a data channel operation (run TPM2_Shutdown) from within the control channel (upon CMD_INIT) inside of swtpm. This way we can deal with it properly. The usage model for the TPM 2 prescribes that a TPM2_Shutdown must be sent before a shutdown or reset of the system, so let's let QEMU do it if the OS cannot do it. Regards, Stefan Stefan Berger (2): backends/tpm: Record the last command sent to the TPM backends/tpm: Send TPM2_Shutdown upon VM reset backends/tpm/tpm_emulator.c | 44 + backends/tpm/tpm_int.h | 3 +++ backends/tpm/tpm_util.c | 9 backends/tpm/trace-events | 1 + include/sysemu/tpm_util.h | 3 +++ 5 files changed, 60 insertions(+) -- 2.35.3
Re: [PULL 0/3] Hppa serial fix patches
On 5/28/22 03:28, Helge Deller wrote: The following changes since commit 58b53669e87fed0d70903e05cd42079fbbdbc195: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-05-25 13:46:29 -0700) are available in the Git repository at: https://github.com/hdeller/qemu-hppa.git tags/hppa-serial-fix-pull-request for you to fetch changes up to 5079892df5f113c7f2b77f53bf7663f6c7bc6be9: hppa: Fix serial port assignments and pass-through (2022-05-28 12:25:42 +0200) hppa: Fix serial port pass-through This series fixes the SeaBIOS-hppa firmware and the serial ports setup code in qemu so that it reflects the real hardware and allows serial port pass-through from the host to guests. Tested with Linux guests. v2: Changes suggested by: Mark Cave-Ayland - Split out hppa_hardware.h restoration to an own patch - Drop unneccesary checks for serial_hd(x) Signed-off-by: Helge Deller Reviewed-by: Mark Cave-Ayland Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Helge Deller (3): New SeaBIOS-hppa version 6 hppa: Sync contents of hppa_hardware.h header file with SeaBIOS-hppa hppa: Fix serial port assignments and pass-through hw/hppa/hppa_hardware.h | 10 -- hw/hppa/machine.c | 22 -- pc-bios/hppa-firmware.img | Bin 719040 -> 719368 bytes roms/seabios-hppa | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) -- 2.35.3
[PATCH v3 0/7] QOM'ify PIIX southbridge creation
v3:
* Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
* Use embedded structs for touched PCI devices (Mark)
* Fix piix4's rtc embedded struct to be initialized by
object_initialize_child() (Peter) [2]
Testing done:
1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.
2)
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.
v2:
* Preserve `DeviceState *` as return value of piix4_create() (Mark)
* Aggregate all type name movements into first commit (Mark)
* Have piix4 southbridge rather than malta board instantiate piix4 pm (me)
Testing done:
1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.
2)
Modify pci_piix3_realize() to start with
error_setg(errp, "This is a test");
Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
archlinux-2022.05.01-x86_64.iso`.
Result: qemu-system-x86_64 aborts with: "This is a test"
v1:
The piix3 and piix4 southbridge devices still rely on create() functions which
are deprecated. This series resolves these functions piece by piece to
modernize the code.
Both devices are modified in lockstep where possible to provide more context.
Testing done:
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.
[1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
Bernhard Beschow (7):
include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
hw/isa/piix4: Use object_initialize_child() for embedded struct
hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
hw/isa/piix4: QOM'ify PIIX4 PM creation
hw/isa/piix{3,4}: Inline and remove create() functions
hw/i386/pc_piix.c | 7 +-
hw/isa/piix3.c| 98 ++-
hw/isa/piix4.c| 120 +-
hw/mips/malta.c | 7 +-
include/hw/isa/isa.h | 2 -
include/hw/southbridge/piix.h | 6 +-
6 files changed, 127 insertions(+), 113 deletions(-)
--
2.36.1
[PATCH v3 2/7] hw/isa/piix4: Use object_initialize_child() for embedded struct
Found-by: Peter Maydell
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9a6d981037..1d04fb6a55 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -224,7 +224,7 @@ static void piix4_init(Object *obj)
{
PIIX4State *s = PIIX4_PCI_DEVICE(obj);
-object_initialize(&s->rtc, sizeof(s->rtc), TYPE_MC146818_RTC);
+object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
}
static void piix4_class_init(ObjectClass *klass, void *data)
--
2.36.1
[PATCH v3 7/7] hw/isa/piix{3, 4}: Inline and remove create() functions
During the previous changesets the create() functions became trivial
wrappers around more generic functions. Modernize the code.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
---
hw/i386/pc_piix.c | 6 +-
hw/isa/piix3.c| 13 -
hw/isa/piix4.c| 13 -
hw/mips/malta.c | 5 -
include/hw/southbridge/piix.h | 4
5 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9d2076a7ca..107efa1ca6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -197,6 +197,9 @@ static void pc_init1(MachineState *machine,
if (pcmc->pci_enabled) {
PIIX3State *piix3;
+PCIDevice *pci_dev;
+const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
+ : TYPE_PIIX3_DEVICE;
pci_bus = i440fx_init(host_type,
pci_type,
@@ -207,7 +210,8 @@ static void pc_init1(MachineState *machine,
pci_memory, ram_memory);
pcms->bus = pci_bus;
-piix3 = piix3_create(pci_bus);
+pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+piix3 = PIIX3_PCI_DEVICE(pci_dev);
piix3->pic = x86ms->gsi;
piix3_devfn = piix3->dev.devfn;
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c6ff7795f4..01c376b39a 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -399,16 +399,3 @@ static void piix3_register_types(void)
}
type_init(piix3_register_types)
-
-PIIX3State *piix3_create(PCIBus *pci_bus)
-{
-PIIX3State *piix3;
-PCIDevice *pci_dev;
-const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
- : TYPE_PIIX3_DEVICE;
-
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-
-return piix3;
-}
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 217989227d..677bc2f658 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -324,16 +324,3 @@ static void piix4_register_types(void)
}
type_init(piix4_register_types)
-
-DeviceState *piix4_create(PCIBus *pci_bus)
-{
-PCIDevice *pci;
-DeviceState *dev;
-int devfn = PCI_DEVFN(10, 0);
-
-pci = pci_create_simple_multifunction(pci_bus, devfn, true,
- TYPE_PIIX4_PCI_DEVICE);
-dev = DEVICE(pci);
-
-return dev;
-}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index b0fc84ccbb..7afc4bac9a 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1238,6 +1238,7 @@ void mips_malta_init(MachineState *machine)
int be;
MaltaState *s;
DeviceState *dev;
+PCIDevice *piix4;
s = MIPS_MALTA(qdev_new(TYPE_MIPS_MALTA));
sysbus_realize_and_unref(SYS_BUS_DEVICE(s), &error_fatal);
@@ -1399,7 +1400,9 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
-dev = piix4_create(pci_bus);
+piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+TYPE_PIIX4_PCI_DEVICE);
+dev = DEVICE(piix4);
isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
smbus = I2C_BUS(qdev_get_child_bus(dev, "smbus"));
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index e1f5d6d5c8..2693778b23 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -68,8 +68,4 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-PIIX3State *piix3_create(PCIBus *pci_bus);
-
-DeviceState *piix4_create(PCIBus *pci_bus);
-
#endif
--
2.36.1
[PATCH v3 4/7] hw/isa/piix{3, 4}: QOM'ify PCI device creation and wiring
PCI interrupt wiring and device creation (piix4 only) were performed
in create() functions which are obsolete. Move these tasks into QOM
functions to modernize the code.
In order to avoid duplicate checking for xen_enabled() the piix3 realize
methods are now split.
Signed-off-by: Bernhard Beschow
---
hw/isa/piix3.c | 67 +-
hw/isa/piix4.c | 30 --
2 files changed, 67 insertions(+), 30 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c7a9014c3f..de532cc692 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -24,6 +24,7 @@
#include "qemu/osdep.h"
#include "qemu/range.h"
+#include "qapi/error.h"
#include "hw/southbridge/piix.h"
#include "hw/irq.h"
#include "hw/isa/isa.h"
@@ -277,7 +278,7 @@ static const MemoryRegionOps rcr_ops = {
.endianness = DEVICE_LITTLE_ENDIAN
};
-static void piix3_realize(PCIDevice *dev, Error **errp)
+static void pci_piix3_realize(PCIDevice *dev, Error **errp)
{
PIIX3State *d = PIIX3_PCI_DEVICE(dev);
@@ -302,7 +303,6 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
dc->desc= "ISA bridge";
dc->vmsd= &vmstate_piix3;
dc->hotpluggable = false;
-k->realize = piix3_realize;
k->vendor_id= PCI_VENDOR_ID_INTEL;
/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
k->device_id= PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -326,11 +326,28 @@ static const TypeInfo piix3_pci_type_info = {
},
};
+static void piix3_realize(PCIDevice *dev, Error **errp)
+{
+ERRP_GUARD();
+PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
+
+pci_piix3_realize(dev, errp);
+if (*errp) {
+return;
+}
+
+pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
+ piix3, PIIX_NUM_PIRQS);
+pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
+};
+
static void piix3_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
k->config_write = piix3_write_config;
+k->realize = piix3_realize;
}
static const TypeInfo piix3_info = {
@@ -339,11 +356,33 @@ static const TypeInfo piix3_info = {
.class_init= piix3_class_init,
};
+static void piix3_xen_realize(PCIDevice *dev, Error **errp)
+{
+ERRP_GUARD();
+PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
+
+pci_piix3_realize(dev, errp);
+if (*errp) {
+return;
+}
+
+/*
+ * Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI.
+ */
+pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
+ piix3, XEN_PIIX_NUM_PIRQS);
+};
+
static void piix3_xen_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
k->config_write = piix3_write_config_xen;
+k->realize = piix3_xen_realize;
};
static const TypeInfo piix3_xen_info = {
@@ -365,27 +404,11 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus
**isa_bus)
{
PIIX3State *piix3;
PCIDevice *pci_dev;
+const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
+ : TYPE_PIIX3_DEVICE;
-/*
- * Xen supports additional interrupt routes from the PCI devices to
- * the IOAPIC: the four pins of each PCI device on the bus are also
- * connected to the IOAPIC directly.
- * These additional routes can be discovered through ACPI.
- */
-if (xen_enabled()) {
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
- TYPE_PIIX3_XEN_DEVICE);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
- piix3, XEN_PIIX_NUM_PIRQS);
-} else {
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
- TYPE_PIIX3_DEVICE);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
- piix3, PIIX_NUM_PIRQS);
-pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
-}
+pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+piix3 = PIIX3_PCI_DEVICE(pci_dev);
*isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
return piix3;
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 18aa24424f..058bebb5e2 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -35,6 +35,7 @@
#include "hw/rtc/mc146818rtc.h"
#include "hw/ide/pci.h"
#include "hw/acpi/piix4.h"
+#include "hw/usb/hcd-uhci.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
#include "sysemu/runstate.h"
@@ -46,6 +47,8 @@ s
[PATCH v3 1/7] include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
TYPE_PIIX3_PCI_DEVICE resides there as already, so add the remaining
ones, too.
Signed-off-by: Bernhard Beschow
Reviewed-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix3.c| 3 ---
include/hw/isa/isa.h | 2 --
include/hw/southbridge/piix.h | 4
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index dab901c9ad..d96ce2b788 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -35,9 +35,6 @@
#define XEN_PIIX_NUM_PIRQS 128ULL
-#define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
-
static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
{
qemu_set_irq(piix3->pic[pic_irq],
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 034d706ba1..e9fa2f5cea 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -144,6 +144,4 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
return ISA_BUS(qdev_get_parent_bus(DEVICE(d)));
}
-#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-
#endif
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 976b4da582..3b97186f75 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -64,6 +64,10 @@ typedef struct PIIXState PIIX3State;
DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
TYPE_PIIX3_PCI_DEVICE)
+#define TYPE_PIIX3_DEVICE "PIIX3"
+#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
+#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
+
PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
--
2.36.1
[PATCH v3 5/7] hw/isa/piix{3, 4}: Factor out ISABus retrieval from create() functions
Modernizes the code and even saves a few lines.
Signed-off-by: Bernhard Beschow
Reviewed-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/i386/pc_piix.c | 3 ++-
hw/isa/piix3.c| 3 +--
hw/isa/piix4.c| 6 +-
hw/mips/malta.c | 3 ++-
include/hw/southbridge/piix.h | 4 ++--
5 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4120fd52e5..9d2076a7ca 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -207,9 +207,10 @@ static void pc_init1(MachineState *machine,
pci_memory, ram_memory);
pcms->bus = pci_bus;
-piix3 = piix3_create(pci_bus, &isa_bus);
+piix3 = piix3_create(pci_bus);
piix3->pic = x86ms->gsi;
piix3_devfn = piix3->dev.devfn;
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
} else {
pci_bus = NULL;
i440fx_state = NULL;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index de532cc692..c6ff7795f4 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -400,7 +400,7 @@ static void piix3_register_types(void)
type_init(piix3_register_types)
-PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
+PIIX3State *piix3_create(PCIBus *pci_bus)
{
PIIX3State *piix3;
PCIDevice *pci_dev;
@@ -409,7 +409,6 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
piix3 = PIIX3_PCI_DEVICE(pci_dev);
-*isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
return piix3;
}
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 058bebb5e2..96df21a610 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -312,7 +312,7 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
+DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
{
PCIDevice *pci;
DeviceState *dev;
@@ -322,10 +322,6 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
-if (isa_bus) {
-*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
-}
-
if (smbus) {
pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9ffdc5b8f1..e446b25ad0 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1399,7 +1399,8 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
-dev = piix4_create(pci_bus, &isa_bus, &smbus);
+dev = piix4_create(pci_bus, &smbus);
+isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
/* Interrupt controller */
qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 3b97186f75..0a2ef0c7b6 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -68,8 +68,8 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
+PIIX3State *piix3_create(PCIBus *pci_bus);
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
+DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus);
#endif
--
2.36.1
[PATCH v3 3/7] hw/isa/piix{3, 4}: Move pci_map_irq_fn's near pci_set_irq_fn's
The pci_map_irq_fn's were implemented below type_init() which made them
inaccessible to QOM functions. So move them up.
Signed-off-by: Bernhard Beschow
Reviewed-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix3.c | 22 +++---
hw/isa/piix4.c | 50 +-
2 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index d96ce2b788..c7a9014c3f 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -78,6 +78,17 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
piix3_set_irq_level(piix3, pirq, level);
}
+/*
+ * Return the global irq number corresponding to a given device irq
+ * pin. We could also use the bus number to have a more precise mapping.
+ */
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+{
+int slot_addend;
+slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
+return (pci_intx + slot_addend) & 3;
+}
+
static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
{
PIIX3State *piix3 = opaque;
@@ -350,17 +361,6 @@ static void piix3_register_types(void)
type_init(piix3_register_types)
-/*
- * Return the global irq number corresponding to a given device irq
- * pin. We could also use the bus number to have a more precise mapping.
- */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
-{
-int slot_addend;
-slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
-return (pci_intx + slot_addend) & 3;
-}
-
PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
{
PIIX3State *piix3;
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 1d04fb6a55..18aa24424f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -74,6 +74,31 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
}
}
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
static void piix4_isa_reset(DeviceState *dev)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -266,31 +291,6 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
-int slot;
-
-slot = PCI_SLOT(pci_dev->devfn);
-
-switch (slot) {
-/* PIIX4 USB */
-case 10:
-return 3;
-/* AMD 79C973 Ethernet */
-case 11:
-return 1;
-/* Crystal 4281 Sound */
-case 12:
-return 2;
-/* PCI slot 1 to 4 */
-case 18 ... 21:
-return ((slot - 18) + irq_num) & 0x03;
-/* Unknown device, don't do any translation */
-default:
-return irq_num;
-}
-}
-
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
PIIX4State *s;
--
2.36.1
[PATCH v3 6/7] hw/isa/piix4: QOM'ify PIIX4 PM creation
Just like the real hardware, create the PIIX4 ACPI controller as part of
the PIIX4 southbridge. This also mirrors how the IDE and USB functions
are already created.
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c| 25 ++---
hw/mips/malta.c | 3 ++-
include/hw/southbridge/piix.h | 2 +-
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 96df21a610..217989227d 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -49,6 +49,7 @@ struct PIIX4State {
RTCState rtc;
PCIIDEState ide;
UHCIState uhci;
+PIIX4PMState pm;
/* Reset Control Register */
MemoryRegion rcr_mem;
uint8_t rcr;
@@ -261,6 +262,14 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
return;
}
+/* ACPI controller */
+qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
+if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+return;
+}
+qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
+object_property_add_alias(OBJECT(s), "smbus", OBJECT(&s->pm), "i2c");
+
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
}
@@ -271,6 +280,10 @@ static void piix4_init(Object *obj)
object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
+
+object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM);
+qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100);
+qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", 0);
}
static void piix4_class_init(ObjectClass *klass, void *data)
@@ -312,7 +325,7 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
-DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
+DeviceState *piix4_create(PCIBus *pci_bus)
{
PCIDevice *pci;
DeviceState *dev;
@@ -322,15 +335,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
-if (smbus) {
-pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
-qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
-qdev_prop_set_bit(DEVICE(pci), "smm-enabled", 0);
-pci_realize_and_unref(pci, pci_bus, &error_fatal);
-qdev_connect_gpio_out(DEVICE(pci), 0,
- qdev_get_gpio_in_named(dev, "isa", 9));
-*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pci), "i2c"));
-}
-
return dev;
}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index e446b25ad0..b0fc84ccbb 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1399,8 +1399,9 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
-dev = piix4_create(pci_bus, &smbus);
+dev = piix4_create(pci_bus);
isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+smbus = I2C_BUS(qdev_get_child_bus(dev, "smbus"));
/* Interrupt controller */
qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0a2ef0c7b6..e1f5d6d5c8 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,6 +70,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
PIIX3State *piix3_create(PCIBus *pci_bus);
-DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus);
+DeviceState *piix4_create(PCIBus *pci_bus);
#endif
--
2.36.1
[PATCH] Updating gdbstub to allow safe multithreading in usermode emulation
I was testing some multi-threaded code in qemu's usermode and ran into
issues with the gdbstub because the user mode qemu emulation spawns new
threads when the process tries to make a new thread but the gdbstub does
not handle the threads well. The current gdbstub has a single global
struct which contains all the state data, and multiple threads can write
to this struct simultaneously, causing gdb packets to be corrupted. The
different threads can also try to read off the gdb socket at the same
time causing the packet to be devided between threads. This patch is
designed to add a single separate thread for the usermode gdbstub which
will handle all the gdb comms and avoid the multithreading issues.
To demonstrate that the mutlithreading was not working properly before
and that it hopefully works properlly now, I wrote a small test program
with some gdb scripts that can be found here:
http://url6163.thefarbeyond.com/ls/click?upn=dUoerjbXT3NK9PiRMHEMD5Fm1RmJf0eUWTDkIDLODGZSRXu94ynuiGwQ-2FCFcimw5rndUfJbozecGKoOE5zbdfRVgadbVSeCrgP5IlB2UqPU-3DUBT-_E8SO-2FEypfU855L0ybQoiQY4Xaj8Z6NYzBoBK89OH-2BiIJOE8-2BoeErelzsKKZyRONZN5M7Gzw0Zs4K0tnG4gxJSOWW89OLEjRW7ISHPWO2lT6fJJUM88-2BLL6sh4BfexcL-2FKt7KhnEpzqyb9bd5UZ-2FR2iPVIjp7zfshwPtjJEHAHaIGeNZbI4nFw81hhs0N1tt9sAcC1ALVazbgnC5E-2F5ZChA-3D-3D
Signed-off-by: Ben Cohen
---
gdbstub.c | 105 +
include/exec/gdbstub.h | 13 +
linux-user/signal.c| 4 ++
3 files changed, 122 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index a3ff8702ce..11a76da575 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -29,6 +29,7 @@
#include "qemu/ctype.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
+#include "qemu/thread.h"
#include "trace/trace-root.h"
#include "exec/gdbstub.h"
#ifdef CONFIG_USER_ONLY
@@ -370,8 +371,103 @@ typedef struct GDBState {
int sstep_flags;
int supported_sstep_flags;
} GDBState;
+typedef struct GDBSignal {
+CPUState *cpu;
+int sig;
+} GDBSignal;
static GDBState gdbserver_state;
+#ifdef CONFIG_USER_ONLY
+static GDBSignal *waiting_signal;
+static QemuMutex signal_wait_mutex;
+static QemuMutex signal_done_mutex;
+static QemuMutex another_thread_is_stepping;
+static int signal_result;
+#endif
+
+static void *gdb_signal_handler_loop(void* args)
+{
+while (TRUE) {
+if (NULL != waiting_signal) {
+signal_result = gdb_handlesig(waiting_signal->cpu,
+waiting_signal->sig);
+waiting_signal = NULL;
+qemu_mutex_unlock(&signal_done_mutex);
+}
+}
+/* Should never return */
+return NULL;
+}
+
+int gdb_thread_handle_signal(CPUState *cpu, int sig)
+{
+GDBSignal signal = {
+.cpu = cpu,
+.sig = sig
+};
+if (!cpu->singlestep_enabled) {
+/*
+ * This mutex is locked by all threads that are not stepping to allow
+ * only the thread that is currently stepping to handle signals until
+ * it finished stepping. Without this, pending signals that are queued
+ * up while the stepping thread handles its signal will race with the
+ * stepping thread to get the next signal handled. This is bad, because
+ * the gdb client expects the stepping thread to be the first response
+ * back to the step command that was sent.
+ */
+qemu_mutex_lock(&another_thread_is_stepping);
+}
+/*
+ * This mutex is locked to allow only one thread at a time to be
+ * handling a signal. This is necessary, because otherwise multiple
+ * threads will try to talk to the gdb client simultaneously and can
+ * race each other sending and receiving packets, potentially going
+ * out of order or simulatenously reading off of the same socket.
+ */
+qemu_mutex_lock(&signal_wait_mutex);
+{
+/*
+ * This mutex is first locked here to ensure that it is in a locked
+ * state before the gdb_signal_handler_loop handles the next signal
+ * and unlocks it.
+ */
+qemu_mutex_lock(&signal_done_mutex);
+waiting_signal = &signal;
+/*
+ * The thread locks this mutex again to wait until the
+ * gdb_signal_handler_loop is finished handling the signal and has
+ * unlocked the mutex.
+ */
+qemu_mutex_lock(&signal_done_mutex);
+/*
+ * Finally, unlock this mutex in preparation for the next call to
+ * this function
+ */
+qemu_mutex_unlock(&signal_done_mutex);
+sig = signal_result;
+if (!cpu->singlestep_enabled) {
+/*
+ * If this thread is not stepping and is handling its signal
+ * then it can always safely unlock this mutex.
+ */
+qemu_mutex_unlock(&another_thread_is_stepping);
+} else {
+/*
+ * If this thread was already stepping it will already be holding
+ * this mutex so here
Re: [PATCH] Updating gdbstub to allow safe multithreading in usermode emulation
Sorry, my email service mangled the link I ment to send. It should be:https://github.com/odinssecrets/qemu_gdbstub_multithread_testingOn May 28, 2022 3:53 PM, Ben Cohen wrote:I was testing some multi-threaded code in qemu's usermode and ran into
issues with the gdbstub because the user mode qemu emulation spawns new
threads when the process tries to make a new thread but the gdbstub does
not handle the threads well. The current gdbstub has a single global
struct which contains all the state data, and multiple threads can write
to this struct simultaneously, causing gdb packets to be corrupted. The
different threads can also try to read off the gdb socket at the same
time causing the packet to be devided between threads. This patch is
designed to add a single separate thread for the usermode gdbstub which
will handle all the gdb comms and avoid the multithreading issues.
To demonstrate that the mutlithreading was not working properly before
and that it hopefully works properlly now, I wrote a small test program
with some gdb scripts that can be found here:
https://github.com/odinssecrets/qemu_gdbstub_multithread_testing
Signed-off-by: Ben Cohen
---
gdbstub.c | 105 +
include/exec/gdbstub.h | 13 +
linux-user/signal.c | 4 ++
3 files changed, 122 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index a3ff8702ce..11a76da575 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -29,6 +29,7 @@
#include "qemu/ctype.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
+#include "qemu/thread.h"
#include "trace/trace-root.h"
#include "exec/gdbstub.h"
#ifdef CONFIG_USER_ONLY
@@ -370,8 +371,103 @@ typedef struct GDBState {
int sstep_flags;
int supported_sstep_flags;
} GDBState;
+typedef struct GDBSignal {
+ CPUState *cpu;
+ int sig;
+} GDBSignal;
static GDBState gdbserver_state;
+#ifdef CONFIG_USER_ONLY
+static GDBSignal *waiting_signal;
+static QemuMutex signal_wait_mutex;
+static QemuMutex signal_done_mutex;
+static QemuMutex another_thread_is_stepping;
+static int signal_result;
+#endif
+
+static void *gdb_signal_handler_loop(void* args)
+{
+ while (TRUE) {
+ if (NULL != waiting_signal) {
+ signal_result = gdb_handlesig(waiting_signal->cpu,
+ waiting_signal->sig);
+ waiting_signal = NULL;
+ qemu_mutex_unlock(&signal_done_mutex);
+ }
+ }
+ /* Should never return */
+ return NULL;
+}
+
+int gdb_thread_handle_signal(CPUState *cpu, int sig)
+{
+ GDBSignal signal = {
+ .cpu = cpu,
+ .sig = sig
+ };
+ if (!cpu->singlestep_enabled) {
+ /*
+ * This mutex is locked by all threads that are not stepping to allow
+ * only the thread that is currently stepping to handle signals until
+ * it finished stepping. Without this, pending signals that are queued
+ * up while the stepping thread handles its signal will race with the
+ * stepping thread to get the next signal handled. This is bad, because
+ * the gdb client expects the stepping thread to be the first response
+ * back to the step command that was sent.
+ */
+ qemu_mutex_lock(&another_thread_is_stepping);
+ }
+ /*
+ * This mutex is locked to allow only one thread at a time to be
+ * handling a signal. This is necessary, because otherwise multiple
+ * threads will try to talk to the gdb client simultaneously and can
+ * race each other sending and receiving packets, potentially going
+ * out of order or simulatenously reading off of the same socket.
+ */
+ qemu_mutex_lock(&signal_wait_mutex);
+ {
+ /*
+ * This mutex is first locked here to ensure that it is in a locked
+ * state before the gdb_signal_handler_loop handles the next signal
+ * and unlocks it.
+ */
+ qemu_mutex_lock(&signal_done_mutex);
+ waiting_signal = &signal;
+ /*
+ * The thread locks this mutex again to wait until the
+ * gdb_signal_handler_loop is finished handling the signal and has
+ * unlocked the mutex.
+ */
+ qemu_mutex_lock(&signal_done_mutex);
+ /*
+ * Finally, unlock this mutex in preparation for the next call to
+ * this function
+ */
+ qemu_mutex_unlock(&signal_done_mutex);
+ sig = signal_result;
+ if (!cpu->singlestep_enabled) {
+ /*
+ * If this thread is not stepping and is handling its signal
+ * then it can always safely unlock this mutex.
+ */
+ qemu_mutex_unlock(&another_thread_is_stepping);
+ } else {
+ /*
+ * If this thread was already stepping it will already be holding
+ * this mutex so here try to lock instead of waiting on
[PATCH 1/4] hw/ide/atapi.c: Correct typos (CD-CDROM -> CD-ROM)
Signed-off-by: Lev Kujawski
---
hw/ide/atapi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index b626199e3d..88b2890faf 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -318,7 +318,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int
max_size)
}
}
-/* start a CD-CDROM read command */
+/* start a CD-ROM read command */
static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors,
int sector_size)
{
@@ -417,7 +417,7 @@ eot:
ide_set_inactive(s, false);
}
-/* start a CD-CDROM read command with DMA */
+/* start a CD-ROM read command with DMA */
/* XXX: test if DMA is available */
static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors,
int sector_size)
--
2.34.1
[PATCH 2/4] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC
Prior to this patch, cmd_exec_dev_diagnostic relied upon
ide_set_signature to clear the device register. While the
preservation of the drive bit by ide_set_signature is necessary for
the DEVICE RESET, IDENTIFY DEVICE, and READ SECTOR commands,
ATA/ATAPI-6 specifies that "DEV shall be cleared to zero" for EXECUTE
DEVICE DIAGNOSTIC.
This deviation was uncovered by the ATACT Device Testing Program
written by Hale Landis.
Signed-off-by: Lev Kujawski
---
hw/ide/core.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index c2caa54285..5a24547e49 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1704,8 +1704,14 @@ static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
return false;
}
+/* EXECUTE DEVICE DIAGNOSTIC */
static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
{
+/*
+ * Clear the device register per the ATA (v6) specification,
+ * because ide_set_signature does not clear LBA or drive bits.
+ */
+s->select = (ATA_DEV_ALWAYS_ON);
ide_set_signature(s);
if (s->drive_kind == IDE_CD) {
--
2.34.1
[PATCH 4/4] hw/ide/piix: Ignore writes of hardwired PCI command register bits
One method to enable PCI bus mastering for IDE controllers, often used
by x86 firmware, is to write 0x7 to the PCI command register. Neither
the PIIX3 specification nor actual hardware (a Tyan S1686D system)
permit modification of the Memory Space Enable (MSE) bit, 1, and thus
the command register would be left in an unspecified state without
this patch.
Signed-off-by: Lev Kujawski
---
hw/ide/piix.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 76ea8fd9f6..f1d1168ecd 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -25,6 +25,8 @@
* References:
* [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
* 290550-002, Intel Corporation, April 1997.
+ * [2] 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4), 290562-001,
+ * Intel Corporation, April 1997.
*/
#include "qemu/osdep.h"
@@ -32,6 +34,7 @@
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "qemu/range.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "sysemu/dma.h"
@@ -220,6 +223,26 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static void piix_pci_config_write(PCIDevice *d, uint32_t addr,
+ uint32_t val, int l)
+{
+/*
+ * Mask all IDE PCI command register bits except for Bus Master
+ * Function Enable (bit 2) and I/O Space Enable (bit 1), as the
+ * remainder are hardwired to 0 [1, p.48] [2, p.89-90].
+ *
+ * NOTE: According to the PIIX3 datasheet [1], the Memory Space
+ * Enable (MSE bit) is hardwired to 1, but this is contradicted by
+ * actual PIIX3 hardware, the datasheet itself (viz., Default
+ * Value: h), and the PIIX4 datasheet [2].
+ */
+if (range_covers_byte(addr, l, PCI_COMMAND)) {
+val &= ~(0xfffa << ((PCI_COMMAND - addr) << 3));
+}
+
+pci_default_write_config(d, addr, val, l);
+}
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -232,6 +255,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
}
@@ -260,6 +284,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void
*data)
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
}
--
2.34.1
[PATCH 3/4] piix_ide_reset: Use pci_set_* functions instead of direct access
Eliminates the remaining TODOs in hw/ide/piix.c by:
- Using pci_set_{size} functions to write the PIIX PCI configuration
space instead of manipulating it directly as an array; and
- Documenting default register values by reference to the controlling
specification.
Signed-off-by: Lev Kujawski
---
hw/ide/piix.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ce89fd0aa3..76ea8fd9f6 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -21,6 +21,10 @@
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
+ *
+ * References:
+ * [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
+ * 290550-002, Intel Corporation, April 1997.
*/
#include "qemu/osdep.h"
@@ -114,14 +118,11 @@ static void piix_ide_reset(DeviceState *dev)
ide_bus_reset(&d->bus[i]);
}
-/* TODO: this is the default. do not override. */
-pci_conf[PCI_COMMAND] = 0x00;
-/* TODO: this is the default. do not override. */
-pci_conf[PCI_COMMAND + 1] = 0x00;
-/* TODO: use pci_set_word */
-pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
-pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
-pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+/* PCI command register default value (h) per [1, p.48]. */
+pci_set_word(pci_conf + PCI_COMMAND, 0x);
+pci_set_word(pci_conf + PCI_STATUS,
+ PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK);
+pci_set_byte(pci_conf + 0x20, 0x01); /* BMIBA: 20-23h */
}
static int pci_piix_init_ports(PCIIDEState *d)
--
2.34.1
Re: [PATCH v2 0/3] PIIX3-IDE XEN cleanup
Am 13. Mai 2022 18:09:54 UTC schrieb Bernhard Beschow : >v2: >* Have pci_xen_ide_unplug() return void (Paul Durrant) >* CC Xen maintainers (Michael S. Tsirkin) > >v1: >This patch series first removes the redundant "piix3-ide-xen" device class and >then moves a XEN-specific helper function from PIIX3 code to XEN code. The idea >is to decouple PIIX3-IDE and XEN and to compile XEN-specific bits only if XEN >support is enabled. > >Testing done: >'qemu-system-x86_64 -M pc -m 1G -cdrom archlinux-2022.05.01-x86_64.iso" boots >successfully and a 'poweroff' inside the VM also shuts it down correctly. > >XEN mode wasn't tested for the time being since its setup procedure seems quite >sophisticated. Please let me know in case this is an obstacle. > >Bernhard Beschow (3): > hw/ide/piix: Remove redundant "piix3-ide-xen" device class > hw/ide/piix: Add some documentation to pci_piix3_xen_ide_unplug() > include/hw/ide: Unexport pci_piix3_xen_ide_unplug() > > hw/i386/pc_piix.c | 3 +-- > hw/i386/xen/xen_platform.c | 48 +- > hw/ide/piix.c | 42 - > include/hw/ide.h | 3 --- > 4 files changed, 48 insertions(+), 48 deletions(-) > Ping Whole series is reviewed/acked.
