[PATCH 0/2] PC: Create RTC controllers in south bridges

[PATCH 0/2] PC: Create RTC controllers in south bridges

[Bernhard Beschow]

This series contains two patches originating from the PIIX consolidation series
[1]. Since the two patches represent general PC code modernization (ICH9 is
modernized as well) and since the PIIX consolidation series is still under
discussion it made sense to split off these patches.

Testing done:
* `make check`
* `make check-avocado`

[1] https://lore.kernel.org/qemu-devel/20230518172747-mutt-send-email-mst@kernel.org/

Bernhard Beschow (2):
  hw/i386/pc: Create RTC controllers in south bridges
  hw/i386/pc: No need for rtc_state to be an out-parameter

 include/hw/i386/pc.h          |  2 +-
 include/hw/southbridge/ich9.h |  2 ++
 include/hw/southbridge/piix.h |  3 +++
 hw/i386/pc.c                  | 16 +++++++++++++---
 hw/i386/pc_piix.c             | 10 +++++++++-
 hw/i386/pc_q35.c              |  4 +++-
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 hw/isa/Kconfig                |  2 ++
 9 files changed, 56 insertions(+), 6 deletions(-)

-- 
2.40.1

[PATCH 1/2] hw/i386/pc: Create RTC controllers in south bridges

[Bernhard Beschow]

Just like in the real hardware (and in PIIX4), create the RTC
controllers in the south bridges.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20221022150508.26830-11-shentey@gmail.com>
---
 include/hw/southbridge/ich9.h |  2 ++
 include/hw/southbridge/piix.h |  3 +++
 hw/i386/pc.c                  | 12 +++++++++++-
 hw/i386/pc_piix.c             |  8 ++++++++
 hw/i386/pc_q35.c              |  2 ++
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 hw/isa/Kconfig                |  2 ++
 8 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
index 7004eecbf9..fd01649d04 100644
--- a/include/hw/southbridge/ich9.h
+++ b/include/hw/southbridge/ich9.h
@@ -6,6 +6,7 @@
 #include "hw/intc/ioapic.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_device.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "exec/memory.h"
 #include "qemu/notify.h"
 #include "qom/object.h"
@@ -30,6 +31,7 @@ struct ICH9LPCState {
     */
     uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
 
+    MC146818RtcState rtc;
     APMState apm;
     ICH9LPCPMRegs pm;
     uint32_t sci_level; /* track sci level */
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bf48e936d..a840340308 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -13,6 +13,7 @@
 #define HW_SOUTHBRIDGE_PIIX_H
 
 #include "hw/pci/pci_device.h"
+#include "hw/rtc/mc146818rtc.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -51,6 +52,8 @@ struct PIIXState {
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    MC146818RtcState rtc;
+
     /* Reset Control Register contents */
     uint8_t rcr;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d761c8c775..2bf7de694e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1318,7 +1318,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
     }
-    *rtc_state = ISA_DEVICE(mc146818_rtc_init(isa_bus, 2000, rtc_irq));
+
+    if (rtc_irq) {
+        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+    } else {
+        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+                                                "irq",
+                                                &error_fatal);
+        isa_connect_gpio_out(*rtc_state, 0, irq);
+    }
+    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+                              "date");
 
 #ifdef CONFIG_XEN_EMU
     if (xen_mode == XEN_EMULATE) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 66a849d279..682f51577c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -32,6 +32,7 @@
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
 #include "hw/pci-host/i440fx.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "hw/southbridge/piix.h"
 #include "hw/display/ramfb.h"
 #include "hw/firmware/smbios.h"
@@ -240,10 +241,17 @@ static void pc_init1(MachineState *machine,
         piix3->pic = x86ms->gsi;
         piix3_devfn = piix3->dev.devfn;
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+        rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+                                                             "rtc"));
     } else {
         pci_bus = NULL;
         isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
                               &error_abort);
+
+        rtc_state = isa_new(TYPE_MC146818_RTC);
+        qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
+        isa_realize_and_unref(rtc_state, isa_bus, &error_fatal);
+
         i8257_dma_init(isa_bus, 0);
         pcms->hpet_enabled = false;
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f02919d92c..2bbd0d02df 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -242,6 +242,8 @@ static void pc_q35_init(MachineState *machine)
                       x86_machine_is_smm_enabled(x86ms));
     pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
+    rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
+
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                              TYPE_HOTPLUG_HANDLER,
                              (Object **)&x86ms->acpi_dev,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 9714b0001e..9c47a2f6c7 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -658,6 +658,8 @@ static void ich9_lpc_initfn(Object *obj)
     static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
     static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
 
+    object_initialize_child(obj, "rtc", &lpc->rtc, TYPE_MC146818_RTC);
+
     object_property_add_uint8_ptr(obj, ACPI_PM_PROP_SCI_INT,
                                   &lpc->sci_gsi, OBJ_PROP_FLAG_READ);
     object_property_add_uint8_ptr(OBJECT(lpc), ACPI_PM_PROP_ACPI_ENABLE_CMD,
@@ -723,6 +725,12 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
 
     i8257_dma_init(isa_bus, 0);
 
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&lpc->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
+
     pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
     pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
     pci_bus_set_route_irq_fn(pci_bus, ich9_route_intx_pin_to_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a9cb39bf21..f9103ea45a 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -28,6 +28,7 @@
 #include "hw/dma/i8257.h"
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
+#include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
@@ -301,6 +302,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
                                         PIIX_RCR_IOPORT, &d->rcr_mem, 1);
 
     i8257_dma_init(isa_bus, 0);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
 }
 
 static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -324,6 +331,13 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
     qbus_build_aml(bus, scope);
 }
 
+static void pci_piix3_init(Object *obj)
+{
+    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+
+    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
+}
+
 static void pci_piix3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -350,6 +364,7 @@ static const TypeInfo piix3_pci_type_info = {
     .name = TYPE_PIIX3_PCI_DEVICE,
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIX3State),
+    .instance_init = pci_piix3_init,
     .abstract = true,
     .class_init = pci_piix3_class_init,
     .interfaces = (InterfaceInfo[]) {
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 0156a66889..c10cbc5fc1 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
     bool
     select I8257
     select ISA_BUS
+    select MC146818RTC
 
 config PIIX4
     bool
@@ -79,3 +80,4 @@ config LPC_ICH9
     select I8257
     select ISA_BUS
     select ACPI_ICH9
+    select MC146818RTC
-- 
2.40.1

[PATCH 2/2] hw/i386/pc: No need for rtc_state to be an out-parameter

[Bernhard Beschow]

Now that the RTC is created as part of the southbridges it doesn't need
to be an out-parameter any longer.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221022150508.26830-12-shentey@gmail.com>
---
 include/hw/i386/pc.h |  2 +-
 hw/i386/pc.c         | 12 ++++++------
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 84935fc958..377e8ebdfb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -168,7 +168,7 @@ uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
+                          ISADevice *rtc_state,
                           bool create_fdctrl,
                           uint32_t hpet_irqs);
 void pc_cmos_init(PCMachineState *pcms,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2bf7de694e..1c77bc872e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1265,7 +1265,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
 
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
+                          ISADevice *rtc_state,
                           bool create_fdctrl,
                           uint32_t hpet_irqs)
 {
@@ -1320,14 +1320,14 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     }
 
     if (rtc_irq) {
-        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+        qdev_connect_gpio_out(DEVICE(rtc_state), 0, rtc_irq);
     } else {
-        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+        uint32_t irq = object_property_get_uint(OBJECT(rtc_state),
                                                 "irq",
                                                 &error_fatal);
-        isa_connect_gpio_out(*rtc_state, 0, irq);
+        isa_connect_gpio_out(rtc_state, 0, irq);
     }
-    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(rtc_state),
                               "date");
 
 #ifdef CONFIG_XEN_EMU
@@ -1341,7 +1341,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     }
 #endif
 
-    qemu_register_boot_set(pc_boot_set, *rtc_state);
+    qemu_register_boot_set(pc_boot_set, rtc_state);
 
     if (!xen_enabled() &&
         (x86ms->pit == ON_OFF_AUTO_AUTO || x86ms->pit == ON_OFF_AUTO_ON)) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 682f51577c..d7913887dd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -277,7 +277,7 @@ static void pc_init1(MachineState *machine,
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, &rtc_state, true,
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true,
                          0x4);
 
     pc_nic_init(pcmc, isa_bus, pci_bus);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2bbd0d02df..0d6720270e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,7 +293,7 @@ static void pc_q35_init(MachineState *machine)
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, &rtc_state, !mc->no_floppy,
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, !mc->no_floppy,
                          0xff0104);
 
     if (pcms->sata_enabled) {
-- 
2.40.1

Re: [PATCH 1/2] hw/i386/pc: Create RTC controllers in south bridges

[Michael S. Tsirkin]

On Fri, May 19, 2023 at 10:47:33AM +0200, Bernhard Beschow wrote:
> Just like in the real hardware (and in PIIX4), create the RTC
> controllers in the south bridges.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Message-Id: <20221022150508.26830-11-shentey@gmail.com>

Please do not include this Message-Id field when you post.

> ---
>  include/hw/southbridge/ich9.h |  2 ++
>  include/hw/southbridge/piix.h |  3 +++
>  hw/i386/pc.c                  | 12 +++++++++++-
>  hw/i386/pc_piix.c             |  8 ++++++++
>  hw/i386/pc_q35.c              |  2 ++
>  hw/isa/lpc_ich9.c             |  8 ++++++++
>  hw/isa/piix3.c                | 15 +++++++++++++++
>  hw/isa/Kconfig                |  2 ++
>  8 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
> index 7004eecbf9..fd01649d04 100644
> --- a/include/hw/southbridge/ich9.h
> +++ b/include/hw/southbridge/ich9.h
> @@ -6,6 +6,7 @@
>  #include "hw/intc/ioapic.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_device.h"
> +#include "hw/rtc/mc146818rtc.h"
>  #include "exec/memory.h"
>  #include "qemu/notify.h"
>  #include "qom/object.h"
> @@ -30,6 +31,7 @@ struct ICH9LPCState {
>      */
>      uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
>  
> +    MC146818RtcState rtc;
>      APMState apm;
>      ICH9LPCPMRegs pm;
>      uint32_t sci_level; /* track sci level */
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index 0bf48e936d..a840340308 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -13,6 +13,7 @@
>  #define HW_SOUTHBRIDGE_PIIX_H
>  
>  #include "hw/pci/pci_device.h"
> +#include "hw/rtc/mc146818rtc.h"
>  
>  /* PIRQRC[A:D]: PIRQx Route Control Registers */
>  #define PIIX_PIRQCA 0x60
> @@ -51,6 +52,8 @@ struct PIIXState {
>      /* This member isn't used. Just for save/load compatibility */
>      int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>  
> +    MC146818RtcState rtc;
> +
>      /* Reset Control Register contents */
>      uint8_t rcr;
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d761c8c775..2bf7de694e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1318,7 +1318,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>          pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
>          rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
>      }
> -    *rtc_state = ISA_DEVICE(mc146818_rtc_init(isa_bus, 2000, rtc_irq));
> +
> +    if (rtc_irq) {
> +        qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
> +    } else {
> +        uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
> +                                                "irq",
> +                                                &error_fatal);
> +        isa_connect_gpio_out(*rtc_state, 0, irq);
> +    }
> +    object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
> +                              "date");
>  
>  #ifdef CONFIG_XEN_EMU
>      if (xen_mode == XEN_EMULATE) {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 66a849d279..682f51577c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -32,6 +32,7 @@
>  #include "hw/i386/pc.h"
>  #include "hw/i386/apic.h"
>  #include "hw/pci-host/i440fx.h"
> +#include "hw/rtc/mc146818rtc.h"
>  #include "hw/southbridge/piix.h"
>  #include "hw/display/ramfb.h"
>  #include "hw/firmware/smbios.h"
> @@ -240,10 +241,17 @@ static void pc_init1(MachineState *machine,
>          piix3->pic = x86ms->gsi;
>          piix3_devfn = piix3->dev.devfn;
>          isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> +        rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
> +                                                             "rtc"));
>      } else {
>          pci_bus = NULL;
>          isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
>                                &error_abort);
> +
> +        rtc_state = isa_new(TYPE_MC146818_RTC);
> +        qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
> +        isa_realize_and_unref(rtc_state, isa_bus, &error_fatal);
> +
>          i8257_dma_init(isa_bus, 0);
>          pcms->hpet_enabled = false;
>      }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f02919d92c..2bbd0d02df 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -242,6 +242,8 @@ static void pc_q35_init(MachineState *machine)
>                        x86_machine_is_smm_enabled(x86ms));
>      pci_realize_and_unref(lpc, host_bus, &error_fatal);
>  
> +    rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
> +
>      object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
>                               TYPE_HOTPLUG_HANDLER,
>                               (Object **)&x86ms->acpi_dev,
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 9714b0001e..9c47a2f6c7 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -658,6 +658,8 @@ static void ich9_lpc_initfn(Object *obj)
>      static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
>      static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
>  
> +    object_initialize_child(obj, "rtc", &lpc->rtc, TYPE_MC146818_RTC);
> +
>      object_property_add_uint8_ptr(obj, ACPI_PM_PROP_SCI_INT,
>                                    &lpc->sci_gsi, OBJ_PROP_FLAG_READ);
>      object_property_add_uint8_ptr(OBJECT(lpc), ACPI_PM_PROP_ACPI_ENABLE_CMD,
> @@ -723,6 +725,12 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
>  
>      i8257_dma_init(isa_bus, 0);
>  
> +    /* RTC */
> +    qdev_prop_set_int32(DEVICE(&lpc->rtc), "base_year", 2000);
> +    if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
> +        return;
> +    }
> +
>      pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
>      pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
>      pci_bus_set_route_irq_fn(pci_bus, ich9_route_intx_pin_to_irq);
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index a9cb39bf21..f9103ea45a 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -28,6 +28,7 @@
>  #include "hw/dma/i8257.h"
>  #include "hw/southbridge/piix.h"
>  #include "hw/irq.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/isa/isa.h"
>  #include "hw/xen/xen.h"
>  #include "sysemu/runstate.h"
> @@ -301,6 +302,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
>                                          PIIX_RCR_IOPORT, &d->rcr_mem, 1);
>  
>      i8257_dma_init(isa_bus, 0);
> +
> +    /* RTC */
> +    qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
> +    if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
> +        return;
> +    }
>  }
>  
>  static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
> @@ -324,6 +331,13 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
>      qbus_build_aml(bus, scope);
>  }
>  
> +static void pci_piix3_init(Object *obj)
> +{
> +    PIIX3State *d = PIIX3_PCI_DEVICE(obj);
> +
> +    object_initialize_child(obj, "rtc", &d->rtc, TYPE_MC146818_RTC);
> +}
> +
>  static void pci_piix3_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -350,6 +364,7 @@ static const TypeInfo piix3_pci_type_info = {
>      .name = TYPE_PIIX3_PCI_DEVICE,
>      .parent = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(PIIX3State),
> +    .instance_init = pci_piix3_init,
>      .abstract = true,
>      .class_init = pci_piix3_class_init,
>      .interfaces = (InterfaceInfo[]) {
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index 0156a66889..c10cbc5fc1 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -35,6 +35,7 @@ config PIIX3
>      bool
>      select I8257
>      select ISA_BUS
> +    select MC146818RTC
>  
>  config PIIX4
>      bool
> @@ -79,3 +80,4 @@ config LPC_ICH9
>      select I8257
>      select ISA_BUS
>      select ACPI_ICH9
> +    select MC146818RTC
> -- 
> 2.40.1

Re: [PATCH 0/2] PC: Create RTC controllers in south bridges

[Mark Cave-Ayland]

On 19/05/2023 09:47, Bernhard Beschow wrote:

> This series contains two patches originating from the PIIX consolidation series
> [1]. Since the two patches represent general PC code modernization (ICH9 is
> modernized as well) and since the PIIX consolidation series is still under
> discussion it made sense to split off these patches.
> 
> Testing done:
> * `make check`
> * `make check-avocado`
> 
> [1] https://lore.kernel.org/qemu-devel/20230518172747-mutt-send-email-mst@kernel.org/
> 
> Bernhard Beschow (2):
>    hw/i386/pc: Create RTC controllers in south bridges
>    hw/i386/pc: No need for rtc_state to be an out-parameter
> 
>   include/hw/i386/pc.h          |  2 +-
>   include/hw/southbridge/ich9.h |  2 ++
>   include/hw/southbridge/piix.h |  3 +++
>   hw/i386/pc.c                  | 16 +++++++++++++---
>   hw/i386/pc_piix.c             | 10 +++++++++-
>   hw/i386/pc_q35.c              |  4 +++-
>   hw/isa/lpc_ich9.c             |  8 ++++++++
>   hw/isa/piix3.c                | 15 +++++++++++++++
>   hw/isa/Kconfig                |  2 ++
>   9 files changed, 56 insertions(+), 6 deletions(-)

These look good to me:

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


ATB,

Mark.