diff mbox series

[v4,08/30] hw/i386/pc: Create RTC controllers in south bridges

Message ID 20221221170003.2929-9-shentey@gmail.com (mailing list archive)
State New, archived
Headers show
Series This series consolidates the implementations of the PIIX3 and PIIX4 south | expand

Commit Message

Bernhard Beschow Dec. 21, 2022, 4:59 p.m. UTC
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>
Message-Id: <20221022150508.26830-11-shentey@gmail.com>
---
 hw/i386/pc.c                  | 12 +++++++++++-
 hw/i386/pc_piix.c             |  8 ++++++++
 hw/i386/pc_q35.c              |  1 +
 hw/isa/Kconfig                |  2 ++
 hw/isa/lpc_ich9.c             |  8 ++++++++
 hw/isa/piix3.c                | 15 +++++++++++++++
 include/hw/i386/ich9.h        |  2 ++
 include/hw/southbridge/piix.h |  3 +++
 8 files changed, 50 insertions(+), 1 deletion(-)

Comments

Thomas Huth Jan. 2, 2023, 5:03 p.m. UTC | #1
On 21/12/2022 17.59, 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>
> Message-Id: <20221022150508.26830-11-shentey@gmail.com>
> ---
>   hw/i386/pc.c                  | 12 +++++++++++-
>   hw/i386/pc_piix.c             |  8 ++++++++
>   hw/i386/pc_q35.c              |  1 +
>   hw/isa/Kconfig                |  2 ++
>   hw/isa/lpc_ich9.c             |  8 ++++++++
>   hw/isa/piix3.c                | 15 +++++++++++++++
>   include/hw/i386/ich9.h        |  2 ++
>   include/hw/southbridge/piix.h |  3 +++
>   8 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index fa69b6f43e..d154eedcb3 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1299,7 +1299,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 = 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");
I think you could turn now the "ISADevice **rtc_state" parameter of this 
function into a normal "ISADevice *rtc_state" since the pointer is not a 
return value anymore.

  Thomas
Bernhard Beschow Jan. 2, 2023, 6:25 p.m. UTC | #2
Am 2. Januar 2023 17:03:29 UTC schrieb Thomas Huth <thuth@redhat.com>:
>On 21/12/2022 17.59, 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>
>> Message-Id: <20221022150508.26830-11-shentey@gmail.com>
>> ---
>>   hw/i386/pc.c                  | 12 +++++++++++-
>>   hw/i386/pc_piix.c             |  8 ++++++++
>>   hw/i386/pc_q35.c              |  1 +
>>   hw/isa/Kconfig                |  2 ++
>>   hw/isa/lpc_ich9.c             |  8 ++++++++
>>   hw/isa/piix3.c                | 15 +++++++++++++++
>>   include/hw/i386/ich9.h        |  2 ++
>>   include/hw/southbridge/piix.h |  3 +++
>>   8 files changed, 50 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index fa69b6f43e..d154eedcb3 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1299,7 +1299,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 = 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");
>I think you could turn now the "ISADevice **rtc_state" parameter of this function into a normal "ISADevice *rtc_state" since the pointer is not a return value anymore.

This is done in patch 9/30: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03799.html

Best regards,
Bernhard

>
> Thomas
>
Thomas Huth Jan. 3, 2023, 8:51 a.m. UTC | #3
On 02/01/2023 19.25, Bernhard Beschow wrote:
> 
> 
> Am 2. Januar 2023 17:03:29 UTC schrieb Thomas Huth <thuth@redhat.com>:
>> On 21/12/2022 17.59, 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>
>>> Message-Id: <20221022150508.26830-11-shentey@gmail.com>
>>> ---
>>>    hw/i386/pc.c                  | 12 +++++++++++-
>>>    hw/i386/pc_piix.c             |  8 ++++++++
>>>    hw/i386/pc_q35.c              |  1 +
>>>    hw/isa/Kconfig                |  2 ++
>>>    hw/isa/lpc_ich9.c             |  8 ++++++++
>>>    hw/isa/piix3.c                | 15 +++++++++++++++
>>>    include/hw/i386/ich9.h        |  2 ++
>>>    include/hw/southbridge/piix.h |  3 +++
>>>    8 files changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index fa69b6f43e..d154eedcb3 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1299,7 +1299,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 = 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");
>> I think you could turn now the "ISADevice **rtc_state" parameter of this function into a normal "ISADevice *rtc_state" since the pointer is not a return value anymore.
> 
> This is done in patch 9/30: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03799.html

Ah, right, sorry for my shortsightness, then this is fine here:

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fa69b6f43e..d154eedcb3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1299,7 +1299,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 = 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");
 
     qemu_register_boot_set(pc_boot_set, *rtc_state);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0689b7d3f7..d4a9c79713 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 ed541102f4..92817a9ebd 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -239,6 +239,7 @@  static void pc_q35_init(MachineState *machine)
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,
                                           TYPE_ICH9_LPC_DEVICE);
+    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,
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 18b5c6bf3f..af5ec9cd61 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 ISA_BUS
     select ACPI_SMBUS
     select ACPI_X86_ICH
+    select MC146818RTC
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 6c44cc9767..eb230a1a23 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -660,6 +660,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,
@@ -725,6 +727,12 @@  static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     isa_bus_irqs(isa_bus, lpc->gsi);
 
     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;
+    }
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 283b971ec4..e8ddb6a602 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)
@@ -327,6 +334,13 @@  static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *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);
@@ -353,6 +367,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/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 23ee8e371b..672efc6bce 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -11,6 +11,7 @@ 
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
 #include "hw/pci/pci_bus.h"
+#include "hw/rtc/mc146818rtc.h"
 #include "qom/object.h"
 
 void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
@@ -39,6 +40,7 @@  struct ICH9LPCState {
     */
     uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
 
+    RTCState 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 2693778b23..b1fa08dd2b 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,6 +14,7 @@ 
 
 #include "hw/pci/pci.h"
 #include "qom/object.h"
+#include "hw/rtc/mc146818rtc.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -52,6 +53,8 @@  struct PIIXState {
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
+    RTCState rtc;
+
     /* Reset Control Register contents */
     uint8_t rcr;