Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
On 2/22/22 14:06, Peter Maydell wrote: On Tue, 22 Feb 2022 at 12:56, Philippe Mathieu-Daudé wrote: On 22/2/22 13:02, Markus Armbruster wrote: Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and the RTC supports the event). What if there's more than one RTC? w.r.t. RTC, a machine having multiple RTC devices is silly... I don't think we have any examples in the tree currently, but I bet real hardware like that does exist: the most plausible thing would be a board where there's an RTC built into the SoC but the board designers put an external RTC on the board (perhaps because it was better/more accurate/easier to make battery-backed). Yes. like Aspeed machines. C. In fact, here's an old bug report from a user trying to get their Debian system to use the battery-backed RTC as the "real" one rather than the non-battery-backed RTC device that's also part of the arm board they're using: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785445 -- PMM
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
Markus Armbruster writes: > Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and > the RTC supports the event). What if there's more than one RTC? > Which one changed? New @qom-path identifies it. > > Signed-off-by: Markus Armbruster > --- > RFC because it's compile-tested only. Worthwhile? Let me know what you > think. Passes manual testing with mc146818rtc.
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
On 22/2/22 14:06, Peter Maydell wrote: On Tue, 22 Feb 2022 at 12:56, Philippe Mathieu-Daudé wrote: On 22/2/22 13:02, Markus Armbruster wrote: Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and the RTC supports the event). What if there's more than one RTC? w.r.t. RTC, a machine having multiple RTC devices is silly... I don't think we have any examples in the tree currently, but I bet real hardware like that does exist: the most plausible thing would be a board where there's an RTC built into the SoC but the board designers put an external RTC on the board (perhaps because it was better/more accurate/easier to make battery-backed). In fact, here's an old bug report from a user trying to get their Debian system to use the battery-backed RTC as the "real" one rather than the non-battery-backed RTC device that's also part of the arm board they're using: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785445 OK, thanks for this pointer.
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
Philippe Mathieu-Daudé writes: > Hi Markus, > > On 22/2/22 13:02, Markus Armbruster wrote: >> Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and >> the RTC supports the event). What if there's more than one RTC? > > w.r.t. RTC, a machine having multiple RTC devices is silly... > > Assuming one wants to emulate that; shouldn't all QMP events have a > qom-path by default? Or have a generic "event-from-multiple-sources" > flag which automatically add this field? Not all events originate from a device, or even a QOM object. The ones that do could all use a qom-path member, I guess. [...]
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
On 22/2/22 16:04, Markus Armbruster wrote: Philippe Mathieu-Daudé writes: Hi Markus, On 22/2/22 13:02, Markus Armbruster wrote: Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and the RTC supports the event). What if there's more than one RTC? w.r.t. RTC, a machine having multiple RTC devices is silly... Assuming one wants to emulate that; shouldn't all QMP events have a qom-path by default? Or have a generic "event-from-multiple-sources" flag which automatically add this field? Not all events originate from a device, or even a QOM object. OK. Reviewed-by: Philippe Mathieu-Daudé The ones that do could all use a qom-path member, I guess. [...]
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
On Tue, 22 Feb 2022 at 12:56, Philippe Mathieu-Daudé wrote: > On 22/2/22 13:02, Markus Armbruster wrote: > > Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and > > the RTC supports the event). What if there's more than one RTC? > > w.r.t. RTC, a machine having multiple RTC devices is silly... I don't think we have any examples in the tree currently, but I bet real hardware like that does exist: the most plausible thing would be a board where there's an RTC built into the SoC but the board designers put an external RTC on the board (perhaps because it was better/more accurate/easier to make battery-backed). In fact, here's an old bug report from a user trying to get their Debian system to use the battery-backed RTC as the "real" one rather than the non-battery-backed RTC device that's also part of the arm board they're using: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785445 -- PMM
Re: [PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
Hi Markus, On 22/2/22 13:02, Markus Armbruster wrote: Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and the RTC supports the event). What if there's more than one RTC? w.r.t. RTC, a machine having multiple RTC devices is silly... Assuming one wants to emulate that; shouldn't all QMP events have a qom-path by default? Or have a generic "event-from-multiple-sources" flag which automatically add this field? Which one changed? New @qom-path identifies it. Signed-off-by: Markus Armbruster --- RFC because it's compile-tested only. Worthwhile? Let me know what you think. qapi/misc.json | 4 +++- hw/ppc/spapr_rtc.c | 4 +++- hw/rtc/mc146818rtc.c | 3 ++- hw/rtc/pl031.c | 3 ++- 4 files changed, 10 insertions(+), 4 deletions(-)
[PATCH RFC 4/4] rtc: Have event RTC_CHANGE identify the RTC by QOM path
Event RTC_CHANGE is "emitted when the guest changes the RTC time" (and
the RTC supports the event). What if there's more than one RTC?
Which one changed? New @qom-path identifies it.
Signed-off-by: Markus Armbruster
---
RFC because it's compile-tested only. Worthwhile? Let me know what you
think.
qapi/misc.json | 4 +++-
hw/ppc/spapr_rtc.c | 4 +++-
hw/rtc/mc146818rtc.c | 3 ++-
hw/rtc/pl031.c | 3 ++-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index 0ab235e41f..b83cc39029 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -536,6 +536,8 @@
# @offset: offset in seconds between base RTC clock (as specified
# by -rtc base), and new RTC clock value
#
+# @qom-path: path to the RTC object in the QOM tree
+#
# Note: This event is rate-limited.
# It is not guaranteed that the RTC in the system implements
# this event, or even that the system has an RTC at all.
@@ -550,4 +552,4 @@
#
##
{ 'event': 'RTC_CHANGE',
- 'data': { 'offset': 'int' } }
+ 'data': { 'offset': 'int', 'qom-path': 'str' } }
diff --git a/hw/ppc/spapr_rtc.c b/hw/ppc/spapr_rtc.c
index 79677cf550..d55b4b0c50 100644
--- a/hw/ppc/spapr_rtc.c
+++ b/hw/ppc/spapr_rtc.c
@@ -97,6 +97,7 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu,
SpaprMachineState *spapr,
uint32_t nret, target_ulong rets)
{
SpaprRtcState *rtc = &spapr->rtc;
+g_autofree const char *qom_path = NULL;
struct tm tm;
time_t new_s;
int64_t host_ns;
@@ -120,7 +121,8 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu,
SpaprMachineState *spapr,
}
/* Generate a monitor event for the change */
-qapi_event_send_rtc_change(qemu_timedate_diff(&tm));
+qom_path = object_get_canonical_path(OBJECT(rtc));
+qapi_event_send_rtc_change(qemu_timedate_diff(&tm), qom_path);
host_ns = qemu_clock_get_ns(rtc_clock);
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 57c514e15c..ac9a60c90e 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -611,12 +611,13 @@ static void rtc_get_time(RTCState *s, struct tm *tm)
static void rtc_set_time(RTCState *s)
{
struct tm tm;
+g_autofree const char *qom_path = object_get_canonical_path(OBJECT(s));
rtc_get_time(s, &tm);
s->base_rtc = mktimegm(&tm);
s->last_update = qemu_clock_get_ns(rtc_clock);
-qapi_event_send_rtc_change(qemu_timedate_diff(&tm));
+qapi_event_send_rtc_change(qemu_timedate_diff(&tm), qom_path);
}
static void rtc_set_cmos(RTCState *s, const struct tm *tm)
diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
index 60167c778f..b01d0e75d1 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -138,12 +138,13 @@ static void pl031_write(void * opaque, hwaddr offset,
switch (offset) {
case RTC_LR: {
+g_autofree const char *qom_path = object_get_canonical_path(opaque);
struct tm tm;
s->tick_offset += value - pl031_get_count(s);
qemu_get_timedate(&tm, s->tick_offset);
-qapi_event_send_rtc_change(qemu_timedate_diff(&tm));
+qapi_event_send_rtc_change(qemu_timedate_diff(&tm), qom_path);
pl031_set_alarm(s);
break;
--
2.35.1
