| Message ID | 20220520174537.5827-3-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QOM improvements for rtc/mc146818rtc | expand |
On 20/05/2022 18:45, Bernhard Beschow wrote: > Exposing the io_base offset as a QOM property not only allows it to be > configurable but also to be displayed in HMP: > > Before: > > (qemu) info qtree > ... > dev: mc146818rtc, id "" > gpio-out "" 1 > base_year = 0 (0x0) > irq = 8 (0x8) > lost_tick_policy = "discard" > > After: > > dev: mc146818rtc, id "" > gpio-out "" 1 > base_year = 0 (0x0) > iobase = 112 (0x70) > irq = 8 (0x8) > lost_tick_policy = "discard" > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/i386/microvm-dt.c | 2 +- > hw/rtc/mc146818rtc.c | 7 ++++--- > include/hw/rtc/mc146818rtc.h | 2 +- > 3 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/microvm-dt.c b/hw/i386/microvm-dt.c > index a5db9e4e5a..39fe425b26 100644 > --- a/hw/i386/microvm-dt.c > +++ b/hw/i386/microvm-dt.c > @@ -209,7 +209,7 @@ static void dt_add_isa_rtc(MicrovmMachineState *mms, ISADevice *dev) > { > const char compat[] = "motorola,mc146818"; > uint32_t irq = object_property_get_uint(OBJECT(dev), "irq", NULL); > - hwaddr base = RTC_ISA_BASE; > + hwaddr base = object_property_get_int(OBJECT(dev), "iobase", NULL); Same comment here re: &error_abort. > hwaddr size = 8; > char *nodename; > > diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c > index f235c2ddbe..484f91b6f8 100644 > --- a/hw/rtc/mc146818rtc.c > +++ b/hw/rtc/mc146818rtc.c > @@ -941,7 +941,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) > qemu_register_suspend_notifier(&s->suspend_notifier); > > memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2); > - isa_register_ioport(isadev, &s->io, RTC_ISA_BASE); > + isa_register_ioport(isadev, &s->io, s->io_base); > > /* register rtc 0x70 port for coalesced_pio */ > memory_region_set_flush_coalesced(&s->io); > @@ -950,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) > memory_region_add_subregion(&s->io, 0, &s->coalesced_io); > memory_region_add_coalescing(&s->coalesced_io, 0, 1); > > - qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3); > + qdev_set_legacy_instance_id(dev, s->io_base, 3); > > object_property_add_tm(OBJECT(s), "date", rtc_get_date); > > @@ -983,6 +983,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq) > > static Property mc146818rtc_properties[] = { > DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), > + DEFINE_PROP_UINT32("iobase", RTCState, io_base, 0x70), I think this should be RTC_ISA_BASE? > DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ), > DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState, > lost_tick_policy, LOST_TICK_POLICY_DISCARD), > @@ -1028,7 +1029,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope) > * does, even though qemu only responds to the first two ports. > */ > crs = aml_resource_template(); > - aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE, > + aml_append(crs, aml_io(AML_DECODE16, s->io_base, s->io_base, > 0x01, 0x08)); > aml_append(crs, aml_irq_no_flags(s->isairq)); > > diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h > index 33d85753c0..1f7942a9f8 100644 > --- a/include/hw/rtc/mc146818rtc.h > +++ b/include/hw/rtc/mc146818rtc.h > @@ -26,6 +26,7 @@ struct RTCState { > uint8_t cmos_data[128]; > uint8_t cmos_index; > uint8_t isairq; > + uint32_t io_base; > int32_t base_year; > uint64_t base_rtc; > uint64_t last_update; > @@ -49,7 +50,6 @@ struct RTCState { > }; > > #define RTC_ISA_IRQ 8 > -#define RTC_ISA_BASE 0x70 ... and so you'll need to keep this. > ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, > qemu_irq intercept_irq); Otherwise: Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On Sat, May 21, 2022 at 11:24 AM Mark Cave-Ayland < mark.cave-ayland@ilande.co.uk> wrote: > On 20/05/2022 18:45, Bernhard Beschow wrote: > > > Exposing the io_base offset as a QOM property not only allows it to be > > configurable but also to be displayed in HMP: > > > > Before: > > > > (qemu) info qtree > > ... > > dev: mc146818rtc, id "" > > gpio-out "" 1 > > base_year = 0 (0x0) > > irq = 8 (0x8) > > lost_tick_policy = "discard" > > > > After: > > > > dev: mc146818rtc, id "" > > gpio-out "" 1 > > base_year = 0 (0x0) > > iobase = 112 (0x70) > > irq = 8 (0x8) > > lost_tick_policy = "discard" > > > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > > --- > > hw/i386/microvm-dt.c | 2 +- > > hw/rtc/mc146818rtc.c | 7 ++++--- > > include/hw/rtc/mc146818rtc.h | 2 +- > > 3 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/hw/i386/microvm-dt.c b/hw/i386/microvm-dt.c > > index a5db9e4e5a..39fe425b26 100644 > > --- a/hw/i386/microvm-dt.c > > +++ b/hw/i386/microvm-dt.c > > @@ -209,7 +209,7 @@ static void dt_add_isa_rtc(MicrovmMachineState *mms, > ISADevice *dev) > > { > > const char compat[] = "motorola,mc146818"; > > uint32_t irq = object_property_get_uint(OBJECT(dev), "irq", NULL); > > - hwaddr base = RTC_ISA_BASE; > > + hwaddr base = object_property_get_int(OBJECT(dev), "iobase", NULL); > > Same comment here re: &error_abort. > Ack. > > > hwaddr size = 8; > > char *nodename; > > > > diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c > > index f235c2ddbe..484f91b6f8 100644 > > --- a/hw/rtc/mc146818rtc.c > > +++ b/hw/rtc/mc146818rtc.c > > @@ -941,7 +941,7 @@ static void rtc_realizefn(DeviceState *dev, Error > **errp) > > qemu_register_suspend_notifier(&s->suspend_notifier); > > > > memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2); > > - isa_register_ioport(isadev, &s->io, RTC_ISA_BASE); > > + isa_register_ioport(isadev, &s->io, s->io_base); > > > > /* register rtc 0x70 port for coalesced_pio */ > > memory_region_set_flush_coalesced(&s->io); > > @@ -950,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error > **errp) > > memory_region_add_subregion(&s->io, 0, &s->coalesced_io); > > memory_region_add_coalescing(&s->coalesced_io, 0, 1); > > > > - qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3); > > + qdev_set_legacy_instance_id(dev, s->io_base, 3); > > > > object_property_add_tm(OBJECT(s), "date", rtc_get_date); > > > > @@ -983,6 +983,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int > base_year, qemu_irq intercept_irq) > > > > static Property mc146818rtc_properties[] = { > > DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), > > + DEFINE_PROP_UINT32("iobase", RTCState, io_base, 0x70), > > I think this should be RTC_ISA_BASE? > > > DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ), > > DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState, > > lost_tick_policy, > LOST_TICK_POLICY_DISCARD), > > @@ -1028,7 +1029,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml > *scope) > > * does, even though qemu only responds to the first two ports. > > */ > > crs = aml_resource_template(); > > - aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE, > > + aml_append(crs, aml_io(AML_DECODE16, s->io_base, s->io_base, > > 0x01, 0x08)); > > aml_append(crs, aml_irq_no_flags(s->isairq)); > > > > diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h > > index 33d85753c0..1f7942a9f8 100644 > > --- a/include/hw/rtc/mc146818rtc.h > > +++ b/include/hw/rtc/mc146818rtc.h > > @@ -26,6 +26,7 @@ struct RTCState { > > uint8_t cmos_data[128]; > > uint8_t cmos_index; > > uint8_t isairq; > > + uint32_t io_base; > > int32_t base_year; > > uint64_t base_rtc; > > uint64_t last_update; > > @@ -49,7 +50,6 @@ struct RTCState { > > }; > > > > #define RTC_ISA_IRQ 8 > > -#define RTC_ISA_BASE 0x70 > > ... and so you'll need to keep this. > My intention was indeed to remove it since it is now redundant. Keeping the constant public has the risk of using it instead of the user-configurable QOM property which could cause bugs. > > ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, > > qemu_irq intercept_irq); > > Otherwise: > > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > > ATB, > > Mark. >
On 22/05/2022 10:07, Bernhard Beschow wrote: > On Sat, May 21, 2022 at 11:24 AM Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk > <mailto:mark.cave-ayland@ilande.co.uk>> wrote: > > On 20/05/2022 18:45, Bernhard Beschow wrote: > > > Exposing the io_base offset as a QOM property not only allows it to be > > configurable but also to be displayed in HMP: > > > > Before: > > > > (qemu) info qtree > > ... > > dev: mc146818rtc, id "" > > gpio-out "" 1 > > base_year = 0 (0x0) > > irq = 8 (0x8) > > lost_tick_policy = "discard" > > > > After: > > > > dev: mc146818rtc, id "" > > gpio-out "" 1 > > base_year = 0 (0x0) > > iobase = 112 (0x70) > > irq = 8 (0x8) > > lost_tick_policy = "discard" > > > > Signed-off-by: Bernhard Beschow <shentey@gmail.com <mailto:shentey@gmail.com>> > > --- > > hw/i386/microvm-dt.c | 2 +- > > hw/rtc/mc146818rtc.c | 7 ++++--- > > include/hw/rtc/mc146818rtc.h | 2 +- > > 3 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/hw/i386/microvm-dt.c b/hw/i386/microvm-dt.c > > index a5db9e4e5a..39fe425b26 100644 > > --- a/hw/i386/microvm-dt.c > > +++ b/hw/i386/microvm-dt.c > > @@ -209,7 +209,7 @@ static void dt_add_isa_rtc(MicrovmMachineState *mms, > ISADevice *dev) > > { > > const char compat[] = "motorola,mc146818"; > > uint32_t irq = object_property_get_uint(OBJECT(dev), "irq", NULL); > > - hwaddr base = RTC_ISA_BASE; > > + hwaddr base = object_property_get_int(OBJECT(dev), "iobase", NULL); > > Same comment here re: &error_abort. > > > Ack. > > > > hwaddr size = 8; > > char *nodename; > > > > diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c > > index f235c2ddbe..484f91b6f8 100644 > > --- a/hw/rtc/mc146818rtc.c > > +++ b/hw/rtc/mc146818rtc.c > > @@ -941,7 +941,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) > > qemu_register_suspend_notifier(&s->suspend_notifier); > > > > memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2); > > - isa_register_ioport(isadev, &s->io, RTC_ISA_BASE); > > + isa_register_ioport(isadev, &s->io, s->io_base); > > > > /* register rtc 0x70 port for coalesced_pio */ > > memory_region_set_flush_coalesced(&s->io); > > @@ -950,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) > > memory_region_add_subregion(&s->io, 0, &s->coalesced_io); > > memory_region_add_coalescing(&s->coalesced_io, 0, 1); > > > > - qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3); > > + qdev_set_legacy_instance_id(dev, s->io_base, 3); > > > > object_property_add_tm(OBJECT(s), "date", rtc_get_date); > > > > @@ -983,6 +983,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, > qemu_irq intercept_irq) > > > > static Property mc146818rtc_properties[] = { > > DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), > > + DEFINE_PROP_UINT32("iobase", RTCState, io_base, 0x70), > > I think this should be RTC_ISA_BASE? > > > DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ), > > DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState, > > lost_tick_policy, LOST_TICK_POLICY_DISCARD), > > @@ -1028,7 +1029,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope) > > * does, even though qemu only responds to the first two ports. > > */ > > crs = aml_resource_template(); > > - aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE, > > + aml_append(crs, aml_io(AML_DECODE16, s->io_base, s->io_base, > > 0x01, 0x08)); > > aml_append(crs, aml_irq_no_flags(s->isairq)); > > > > diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h > > index 33d85753c0..1f7942a9f8 100644 > > --- a/include/hw/rtc/mc146818rtc.h > > +++ b/include/hw/rtc/mc146818rtc.h > > @@ -26,6 +26,7 @@ struct RTCState { > > uint8_t cmos_data[128]; > > uint8_t cmos_index; > > uint8_t isairq; > > + uint32_t io_base; > > int32_t base_year; > > uint64_t base_rtc; > > uint64_t last_update; > > @@ -49,7 +50,6 @@ struct RTCState { > > }; > > > > #define RTC_ISA_IRQ 8 > > -#define RTC_ISA_BASE 0x70 > > ... and so you'll need to keep this. > > > My intention was indeed to remove it since it is now redundant. Keeping the constant > public has the risk of using it instead of the user-configurable QOM property which > could cause bugs. True, that's not a completely unreasonable argument. In that case how about moving the RTC_ISA_BASE define to somewhere around the top of hw/rtc/mc146818rtc.c so that the origin of the 0x70 address is preserved? > > ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, > > qemu_irq intercept_irq); > > Otherwise: > > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk > <mailto:mark.cave-ayland@ilande.co.uk>> > > > ATB, > > Mark. ATB, Mark.
Am 22. Mai 2022 12:13:48 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: >On 22/05/2022 10:07, Bernhard Beschow wrote: > >> On Sat, May 21, 2022 at 11:24 AM Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk <mailto:mark.cave-ayland@ilande.co.uk>> wrote: >> >> On 20/05/2022 18:45, Bernhard Beschow wrote: >> >> > Exposing the io_base offset as a QOM property not only allows it to be >> > configurable but also to be displayed in HMP: >> > >> > Before: >> > >> > (qemu) info qtree >> > ... >> > dev: mc146818rtc, id "" >> > gpio-out "" 1 >> > base_year = 0 (0x0) >> > irq = 8 (0x8) >> > lost_tick_policy = "discard" >> > >> > After: >> > >> > dev: mc146818rtc, id "" >> > gpio-out "" 1 >> > base_year = 0 (0x0) >> > iobase = 112 (0x70) >> > irq = 8 (0x8) >> > lost_tick_policy = "discard" >> > >> > Signed-off-by: Bernhard Beschow <shentey@gmail.com <mailto:shentey@gmail.com>> >> > --- >> > hw/i386/microvm-dt.c | 2 +- >> > hw/rtc/mc146818rtc.c | 7 ++++--- >> > include/hw/rtc/mc146818rtc.h | 2 +- >> > 3 files changed, 6 insertions(+), 5 deletions(-) >> > >> > diff --git a/hw/i386/microvm-dt.c b/hw/i386/microvm-dt.c >> > index a5db9e4e5a..39fe425b26 100644 >> > --- a/hw/i386/microvm-dt.c >> > +++ b/hw/i386/microvm-dt.c >> > @@ -209,7 +209,7 @@ static void dt_add_isa_rtc(MicrovmMachineState *mms, >> ISADevice *dev) >> > { >> > const char compat[] = "motorola,mc146818"; >> > uint32_t irq = object_property_get_uint(OBJECT(dev), "irq", NULL); >> > - hwaddr base = RTC_ISA_BASE; >> > + hwaddr base = object_property_get_int(OBJECT(dev), "iobase", NULL); >> >> Same comment here re: &error_abort. >> >> >> Ack. >> >> >> > hwaddr size = 8; >> > char *nodename; >> > >> > diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c >> > index f235c2ddbe..484f91b6f8 100644 >> > --- a/hw/rtc/mc146818rtc.c >> > +++ b/hw/rtc/mc146818rtc.c >> > @@ -941,7 +941,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) >> > qemu_register_suspend_notifier(&s->suspend_notifier); >> > >> > memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2); >> > - isa_register_ioport(isadev, &s->io, RTC_ISA_BASE); >> > + isa_register_ioport(isadev, &s->io, s->io_base); >> > >> > /* register rtc 0x70 port for coalesced_pio */ >> > memory_region_set_flush_coalesced(&s->io); >> > @@ -950,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) >> > memory_region_add_subregion(&s->io, 0, &s->coalesced_io); >> > memory_region_add_coalescing(&s->coalesced_io, 0, 1); >> > >> > - qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3); >> > + qdev_set_legacy_instance_id(dev, s->io_base, 3); >> > >> > object_property_add_tm(OBJECT(s), "date", rtc_get_date); >> > >> > @@ -983,6 +983,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, >> qemu_irq intercept_irq) >> > >> > static Property mc146818rtc_properties[] = { >> > DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), >> > + DEFINE_PROP_UINT32("iobase", RTCState, io_base, 0x70), >> >> I think this should be RTC_ISA_BASE? >> >> > DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ), >> > DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState, >> > lost_tick_policy, LOST_TICK_POLICY_DISCARD), >> > @@ -1028,7 +1029,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope) >> > * does, even though qemu only responds to the first two ports. >> > */ >> > crs = aml_resource_template(); >> > - aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE, >> > + aml_append(crs, aml_io(AML_DECODE16, s->io_base, s->io_base, >> > 0x01, 0x08)); >> > aml_append(crs, aml_irq_no_flags(s->isairq)); >> > >> > diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h >> > index 33d85753c0..1f7942a9f8 100644 >> > --- a/include/hw/rtc/mc146818rtc.h >> > +++ b/include/hw/rtc/mc146818rtc.h >> > @@ -26,6 +26,7 @@ struct RTCState { >> > uint8_t cmos_data[128]; >> > uint8_t cmos_index; >> > uint8_t isairq; >> > + uint32_t io_base; >> > int32_t base_year; >> > uint64_t base_rtc; >> > uint64_t last_update; >> > @@ -49,7 +50,6 @@ struct RTCState { >> > }; >> > >> > #define RTC_ISA_IRQ 8 >> > -#define RTC_ISA_BASE 0x70 >> >> ... and so you'll need to keep this. >> >> >> My intention was indeed to remove it since it is now redundant. Keeping the constant public has the risk of using it instead of the user-configurable QOM property which could cause bugs. > >True, that's not a completely unreasonable argument. In that case how about moving the RTC_ISA_BASE define to somewhere around the top of hw/rtc/mc146818rtc.c so that the origin of the 0x70 address is preserved? Okay, I'll move it there. > >> > ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, >> > qemu_irq intercept_irq); >> >> Otherwise: >> >> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk >> <mailto:mark.cave-ayland@ilande.co.uk>> >> >> >> ATB, >> >> Mark. > > >ATB, > >Mark.
diff --git a/hw/i386/microvm-dt.c b/hw/i386/microvm-dt.c index a5db9e4e5a..39fe425b26 100644 --- a/hw/i386/microvm-dt.c +++ b/hw/i386/microvm-dt.c @@ -209,7 +209,7 @@ static void dt_add_isa_rtc(MicrovmMachineState *mms, ISADevice *dev) { const char compat[] = "motorola,mc146818"; uint32_t irq = object_property_get_uint(OBJECT(dev), "irq", NULL); - hwaddr base = RTC_ISA_BASE; + hwaddr base = object_property_get_int(OBJECT(dev), "iobase", NULL); hwaddr size = 8; char *nodename; diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c index f235c2ddbe..484f91b6f8 100644 --- a/hw/rtc/mc146818rtc.c +++ b/hw/rtc/mc146818rtc.c @@ -941,7 +941,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) qemu_register_suspend_notifier(&s->suspend_notifier); memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2); - isa_register_ioport(isadev, &s->io, RTC_ISA_BASE); + isa_register_ioport(isadev, &s->io, s->io_base); /* register rtc 0x70 port for coalesced_pio */ memory_region_set_flush_coalesced(&s->io); @@ -950,7 +950,7 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->io, 0, &s->coalesced_io); memory_region_add_coalescing(&s->coalesced_io, 0, 1); - qdev_set_legacy_instance_id(dev, RTC_ISA_BASE, 3); + qdev_set_legacy_instance_id(dev, s->io_base, 3); object_property_add_tm(OBJECT(s), "date", rtc_get_date); @@ -983,6 +983,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq) static Property mc146818rtc_properties[] = { DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), + DEFINE_PROP_UINT32("iobase", RTCState, io_base, 0x70), DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ), DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState, lost_tick_policy, LOST_TICK_POLICY_DISCARD), @@ -1028,7 +1029,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope) * does, even though qemu only responds to the first two ports. */ crs = aml_resource_template(); - aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE, + aml_append(crs, aml_io(AML_DECODE16, s->io_base, s->io_base, 0x01, 0x08)); aml_append(crs, aml_irq_no_flags(s->isairq)); diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h index 33d85753c0..1f7942a9f8 100644 --- a/include/hw/rtc/mc146818rtc.h +++ b/include/hw/rtc/mc146818rtc.h @@ -26,6 +26,7 @@ struct RTCState { uint8_t cmos_data[128]; uint8_t cmos_index; uint8_t isairq; + uint32_t io_base; int32_t base_year; uint64_t base_rtc; uint64_t last_update; @@ -49,7 +50,6 @@ struct RTCState { }; #define RTC_ISA_IRQ 8 -#define RTC_ISA_BASE 0x70 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
Exposing the io_base offset as a QOM property not only allows it to be configurable but also to be displayed in HMP: Before: (qemu) info qtree ... dev: mc146818rtc, id "" gpio-out "" 1 base_year = 0 (0x0) irq = 8 (0x8) lost_tick_policy = "discard" After: dev: mc146818rtc, id "" gpio-out "" 1 base_year = 0 (0x0) iobase = 112 (0x70) irq = 8 (0x8) lost_tick_policy = "discard" Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/i386/microvm-dt.c | 2 +- hw/rtc/mc146818rtc.c | 7 ++++--- include/hw/rtc/mc146818rtc.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-)