* [PATCH v6 1/4] hw/intc: Extract the IRQ counting functions into a separate file
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
@ 2023-01-10 9:53 ` Thomas Huth
2023-01-11 22:48 ` Bernhard Beschow
2023-01-10 9:53 ` [PATCH v6 2/4] hw/core/qdev-properties-system: Allow the 'slew' policy only on x86 Thomas Huth
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Thomas Huth @ 2023-01-10 9:53 UTC (permalink / raw)
To: Paolo Bonzini, Michael S Tsirkin, qemu-devel, Bernhard Beschow,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
These IRQ counting functions will soon be required in binaries that
do not include the APIC code, too, so let's extract them into a
separate file that can be linked independently of the APIC code.
While we're at it, change the apic_* prefix into kvm_* since the
functions are used from the i8259 PIC (i.e. not the APIC), too.
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/i386/apic.h | 2 --
include/hw/i386/apic_internal.h | 1 -
include/hw/intc/kvm_irqcount.h | 10 +++++++
hw/i386/kvm/i8259.c | 4 +--
hw/i386/kvm/ioapic.c | 4 +--
hw/intc/apic.c | 3 +-
hw/intc/apic_common.c | 30 ++------------------
hw/intc/kvm_irqcount.c | 49 +++++++++++++++++++++++++++++++++
hw/rtc/mc146818rtc.c | 6 ++--
hw/intc/meson.build | 6 ++++
hw/intc/trace-events | 9 +++---
11 files changed, 81 insertions(+), 43 deletions(-)
create mode 100644 include/hw/intc/kvm_irqcount.h
create mode 100644 hw/intc/kvm_irqcount.c
diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index da1d2fe155..bdc15a7a73 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -9,8 +9,6 @@ int apic_accept_pic_intr(DeviceState *s);
void apic_deliver_pic_intr(DeviceState *s, int level);
void apic_deliver_nmi(DeviceState *d);
int apic_get_interrupt(DeviceState *s);
-void apic_reset_irq_delivered(void);
-int apic_get_irq_delivered(void);
void cpu_set_apic_base(DeviceState *s, uint64_t val);
uint64_t cpu_get_apic_base(DeviceState *s);
void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 968b6648b3..5f2ba24bfc 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -199,7 +199,6 @@ typedef struct VAPICState {
extern bool apic_report_tpr_access;
-void apic_report_irq_delivered(int delivered);
bool apic_next_timer(APICCommonState *s, int64_t current_time);
void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
void apic_enable_vapic(DeviceState *d, hwaddr paddr);
diff --git a/include/hw/intc/kvm_irqcount.h b/include/hw/intc/kvm_irqcount.h
new file mode 100644
index 0000000000..0ed5999e49
--- /dev/null
+++ b/include/hw/intc/kvm_irqcount.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#ifndef KVM_IRQCOUNT_H
+#define KVM_IRQCOUNT_H
+
+void kvm_report_irq_delivered(int delivered);
+void kvm_reset_irq_delivered(void);
+int kvm_get_irq_delivered(void);
+
+#endif
diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c
index d61bae4dc3..3ca0e1ff03 100644
--- a/hw/i386/kvm/i8259.c
+++ b/hw/i386/kvm/i8259.c
@@ -14,7 +14,7 @@
#include "hw/isa/i8259_internal.h"
#include "hw/intc/i8259.h"
#include "qemu/module.h"
-#include "hw/i386/apic_internal.h"
+#include "hw/intc/kvm_irqcount.h"
#include "hw/irq.h"
#include "sysemu/kvm.h"
#include "qom/object.h"
@@ -117,7 +117,7 @@ static void kvm_pic_set_irq(void *opaque, int irq, int level)
pic_stat_update_irq(irq, level);
delivered = kvm_set_irq(kvm_state, irq, level);
- apic_report_irq_delivered(delivered);
+ kvm_report_irq_delivered(delivered);
}
static void kvm_pic_realize(DeviceState *dev, Error **errp)
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index ee7c8ef68b..272e26b4a2 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -15,7 +15,7 @@
#include "hw/i386/x86.h"
#include "hw/qdev-properties.h"
#include "hw/i386/ioapic_internal.h"
-#include "hw/i386/apic_internal.h"
+#include "hw/intc/kvm_irqcount.h"
#include "sysemu/kvm.h"
/* PC Utility function */
@@ -116,7 +116,7 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
ioapic_stat_update_irq(common, irq, level);
delivered = kvm_set_irq(kvm_state, s->kvm_gsi_base + irq, level);
- apic_report_irq_delivered(delivered);
+ kvm_report_irq_delivered(delivered);
}
static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 3df11c34d6..2d3e55f4e2 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -22,6 +22,7 @@
#include "hw/i386/apic.h"
#include "hw/i386/ioapic.h"
#include "hw/intc/i8259.h"
+#include "hw/intc/kvm_irqcount.h"
#include "hw/pci/msi.h"
#include "qemu/host-utils.h"
#include "sysemu/kvm.h"
@@ -399,7 +400,7 @@ void apic_poll_irq(DeviceState *dev)
static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode)
{
- apic_report_irq_delivered(!apic_get_bit(s->irr, vector_num));
+ kvm_report_irq_delivered(!apic_get_bit(s->irr, vector_num));
apic_set_bit(s->irr, vector_num);
if (trigger_mode)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 2a20982066..4a34f03047 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -25,6 +25,7 @@
#include "qapi/visitor.h"
#include "hw/i386/apic.h"
#include "hw/i386/apic_internal.h"
+#include "hw/intc/kvm_irqcount.h"
#include "trace.h"
#include "hw/boards.h"
#include "sysemu/hax.h"
@@ -33,7 +34,6 @@
#include "hw/sysbus.h"
#include "migration/vmstate.h"
-static int apic_irq_delivered;
bool apic_report_tpr_access;
void cpu_set_apic_base(DeviceState *dev, uint64_t val)
@@ -122,32 +122,6 @@ void apic_handle_tpr_access_report(DeviceState *dev, target_ulong ip,
vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
}
-void apic_report_irq_delivered(int delivered)
-{
- apic_irq_delivered += delivered;
-
- trace_apic_report_irq_delivered(apic_irq_delivered);
-}
-
-void apic_reset_irq_delivered(void)
-{
- /* Copy this into a local variable to encourage gcc to emit a plain
- * register for a sys/sdt.h marker. For details on this workaround, see:
- * https://sourceware.org/bugzilla/show_bug.cgi?id=13296
- */
- volatile int a_i_d = apic_irq_delivered;
- trace_apic_reset_irq_delivered(a_i_d);
-
- apic_irq_delivered = 0;
-}
-
-int apic_get_irq_delivered(void)
-{
- trace_apic_get_irq_delivered(apic_irq_delivered);
-
- return apic_irq_delivered;
-}
-
void apic_deliver_nmi(DeviceState *dev)
{
APICCommonState *s = APIC_COMMON(dev);
@@ -272,7 +246,7 @@ static void apic_reset_common(DeviceState *dev)
s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
s->id = s->initial_apic_id;
- apic_reset_irq_delivered();
+ kvm_reset_irq_delivered();
s->vapic_paddr = 0;
info->vapic_base_update(s);
diff --git a/hw/intc/kvm_irqcount.c b/hw/intc/kvm_irqcount.c
new file mode 100644
index 0000000000..2ef8a83a7a
--- /dev/null
+++ b/hw/intc/kvm_irqcount.c
@@ -0,0 +1,49 @@
+/*
+ * KVM PIC functions for counting the delivered IRQs.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * 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 <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "hw/intc/kvm_irqcount.h"
+#include "trace.h"
+
+static int kvm_irq_delivered;
+
+void kvm_report_irq_delivered(int delivered)
+{
+ kvm_irq_delivered += delivered;
+
+ trace_kvm_report_irq_delivered(kvm_irq_delivered);
+}
+
+void kvm_reset_irq_delivered(void)
+{
+ /*
+ * Copy this into a local variable to encourage gcc to emit a plain
+ * register for a sys/sdt.h marker. For details on this workaround, see:
+ * https://sourceware.org/bugzilla/show_bug.cgi?id=13296
+ */
+ volatile int k_i_d = kvm_irq_delivered;
+ trace_kvm_reset_irq_delivered(k_i_d);
+
+ kvm_irq_delivered = 0;
+}
+
+int kvm_get_irq_delivered(void)
+{
+ trace_kvm_get_irq_delivered(kvm_irq_delivered);
+
+ return kvm_irq_delivered;
+}
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 1ebb412479..947d68c257 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -27,6 +27,7 @@
#include "qemu/module.h"
#include "qemu/bcd.h"
#include "hw/acpi/acpi_aml_interface.h"
+#include "hw/intc/kvm_irqcount.h"
#include "hw/irq.h"
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
@@ -46,7 +47,6 @@
#ifdef TARGET_I386
#include "qapi/qapi-commands-misc-target.h"
-#include "hw/i386/apic.h"
#endif
//#define DEBUG_CMOS
@@ -124,9 +124,9 @@ void qmp_rtc_reset_reinjection(Error **errp)
static bool rtc_policy_slew_deliver_irq(RTCState *s)
{
- apic_reset_irq_delivered();
+ kvm_reset_irq_delivered();
qemu_irq_raise(s->irq);
- return apic_get_irq_delivered();
+ return kvm_get_irq_delivered();
}
static void rtc_coalesced_timer(void *opaque)
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index bcbf22ff51..cd9f1ee888 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -25,6 +25,12 @@ softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_intc.c'))
softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP', if_true: files('xlnx-zynqmp-ipi.c'))
softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: files('xlnx-pmu-iomod-intc.c'))
+if config_all_devices.has_key('CONFIG_APIC') or \
+ config_all_devices.has_key('CONFIG_I8259') or \
+ config_all_devices.has_key('CONFIG_MC146818RTC')
+ softmmu_ss.add(files('kvm_irqcount.c'))
+endif
+
specific_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: files('allwinner-a10-pic.c'))
specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c'))
specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif_common.c'))
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 6fbc2045e6..50cadfb996 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -10,10 +10,6 @@ pic_ioport_read(bool master, uint64_t addr, int val) "master %d addr 0x%"PRIx64"
# apic_common.c
cpu_set_apic_base(uint64_t val) "0x%016"PRIx64
cpu_get_apic_base(uint64_t val) "0x%016"PRIx64
-# coalescing
-apic_report_irq_delivered(int apic_irq_delivered) "coalescing %d"
-apic_reset_irq_delivered(int apic_irq_delivered) "old coalescing %d"
-apic_get_irq_delivered(int apic_irq_delivered) "returning coalescing %d"
# apic.c
apic_local_deliver(int vector, uint32_t lvt) "vector %d delivery mode %d"
@@ -30,6 +26,11 @@ ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapi
ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
ioapic_set_irq(int vector, int level) "vector: %d level: %d"
+# kvm_irqcount.c
+kvm_report_irq_delivered(int irq_delivered) "coalescing %d"
+kvm_reset_irq_delivered(int irq_delivered) "old coalescing %d"
+kvm_get_irq_delivered(int irq_delivered) "returning coalescing %d"
+
# slavio_intctl.c
slavio_intctl_mem_readl(uint32_t cpu, uint64_t addr, uint32_t ret) "read cpu %d reg 0x%"PRIx64" = 0x%x"
slavio_intctl_mem_writel(uint32_t cpu, uint64_t addr, uint32_t val) "write cpu %d reg 0x%"PRIx64" = 0x%x"
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 1/4] hw/intc: Extract the IRQ counting functions into a separate file
2023-01-10 9:53 ` [PATCH v6 1/4] hw/intc: Extract the IRQ counting functions into a separate file Thomas Huth
@ 2023-01-11 22:48 ` Bernhard Beschow
0 siblings, 0 replies; 10+ messages in thread
From: Bernhard Beschow @ 2023-01-11 22:48 UTC (permalink / raw)
To: Thomas Huth, Paolo Bonzini, Michael S Tsirkin, qemu-devel,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
Am 10. Januar 2023 09:53:48 UTC schrieb Thomas Huth <thuth@redhat.com>:
>These IRQ counting functions will soon be required in binaries that
>do not include the APIC code, too, so let's extract them into a
>separate file that can be linked independently of the APIC code.
>
>While we're at it, change the apic_* prefix into kvm_* since the
>functions are used from the i8259 PIC (i.e. not the APIC), too.
>
>Reviewed-by: Bernhard Beschow <shentey@gmail.com>
>Signed-off-by: Thomas Huth <thuth@redhat.com>
>---
> include/hw/i386/apic.h | 2 --
> include/hw/i386/apic_internal.h | 1 -
> include/hw/intc/kvm_irqcount.h | 10 +++++++
> hw/i386/kvm/i8259.c | 4 +--
> hw/i386/kvm/ioapic.c | 4 +--
> hw/intc/apic.c | 3 +-
> hw/intc/apic_common.c | 30 ++------------------
> hw/intc/kvm_irqcount.c | 49 +++++++++++++++++++++++++++++++++
> hw/rtc/mc146818rtc.c | 6 ++--
> hw/intc/meson.build | 6 ++++
> hw/intc/trace-events | 9 +++---
> 11 files changed, 81 insertions(+), 43 deletions(-)
> create mode 100644 include/hw/intc/kvm_irqcount.h
> create mode 100644 hw/intc/kvm_irqcount.c
>
>diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
>index da1d2fe155..bdc15a7a73 100644
>--- a/include/hw/i386/apic.h
>+++ b/include/hw/i386/apic.h
>@@ -9,8 +9,6 @@ int apic_accept_pic_intr(DeviceState *s);
> void apic_deliver_pic_intr(DeviceState *s, int level);
> void apic_deliver_nmi(DeviceState *d);
> int apic_get_interrupt(DeviceState *s);
>-void apic_reset_irq_delivered(void);
>-int apic_get_irq_delivered(void);
> void cpu_set_apic_base(DeviceState *s, uint64_t val);
> uint64_t cpu_get_apic_base(DeviceState *s);
> void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
>diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
>index 968b6648b3..5f2ba24bfc 100644
>--- a/include/hw/i386/apic_internal.h
>+++ b/include/hw/i386/apic_internal.h
>@@ -199,7 +199,6 @@ typedef struct VAPICState {
>
> extern bool apic_report_tpr_access;
>
>-void apic_report_irq_delivered(int delivered);
> bool apic_next_timer(APICCommonState *s, int64_t current_time);
> void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
> void apic_enable_vapic(DeviceState *d, hwaddr paddr);
>diff --git a/include/hw/intc/kvm_irqcount.h b/include/hw/intc/kvm_irqcount.h
>new file mode 100644
>index 0000000000..0ed5999e49
>--- /dev/null
>+++ b/include/hw/intc/kvm_irqcount.h
>@@ -0,0 +1,10 @@
>+/* SPDX-License-Identifier: LGPL-2.1-or-later */
>+
>+#ifndef KVM_IRQCOUNT_H
>+#define KVM_IRQCOUNT_H
>+
>+void kvm_report_irq_delivered(int delivered);
>+void kvm_reset_irq_delivered(void);
>+int kvm_get_irq_delivered(void);
>+
>+#endif
>diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c
>index d61bae4dc3..3ca0e1ff03 100644
>--- a/hw/i386/kvm/i8259.c
>+++ b/hw/i386/kvm/i8259.c
>@@ -14,7 +14,7 @@
> #include "hw/isa/i8259_internal.h"
> #include "hw/intc/i8259.h"
> #include "qemu/module.h"
>-#include "hw/i386/apic_internal.h"
>+#include "hw/intc/kvm_irqcount.h"
> #include "hw/irq.h"
> #include "sysemu/kvm.h"
> #include "qom/object.h"
>@@ -117,7 +117,7 @@ static void kvm_pic_set_irq(void *opaque, int irq, int level)
>
> pic_stat_update_irq(irq, level);
> delivered = kvm_set_irq(kvm_state, irq, level);
>- apic_report_irq_delivered(delivered);
>+ kvm_report_irq_delivered(delivered);
> }
>
> static void kvm_pic_realize(DeviceState *dev, Error **errp)
>diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
>index ee7c8ef68b..272e26b4a2 100644
>--- a/hw/i386/kvm/ioapic.c
>+++ b/hw/i386/kvm/ioapic.c
>@@ -15,7 +15,7 @@
> #include "hw/i386/x86.h"
> #include "hw/qdev-properties.h"
> #include "hw/i386/ioapic_internal.h"
>-#include "hw/i386/apic_internal.h"
>+#include "hw/intc/kvm_irqcount.h"
> #include "sysemu/kvm.h"
>
> /* PC Utility function */
>@@ -116,7 +116,7 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
>
> ioapic_stat_update_irq(common, irq, level);
> delivered = kvm_set_irq(kvm_state, s->kvm_gsi_base + irq, level);
>- apic_report_irq_delivered(delivered);
>+ kvm_report_irq_delivered(delivered);
> }
>
> static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
>diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>index 3df11c34d6..2d3e55f4e2 100644
>--- a/hw/intc/apic.c
>+++ b/hw/intc/apic.c
>@@ -22,6 +22,7 @@
> #include "hw/i386/apic.h"
> #include "hw/i386/ioapic.h"
> #include "hw/intc/i8259.h"
>+#include "hw/intc/kvm_irqcount.h"
> #include "hw/pci/msi.h"
> #include "qemu/host-utils.h"
> #include "sysemu/kvm.h"
>@@ -399,7 +400,7 @@ void apic_poll_irq(DeviceState *dev)
>
> static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode)
> {
>- apic_report_irq_delivered(!apic_get_bit(s->irr, vector_num));
>+ kvm_report_irq_delivered(!apic_get_bit(s->irr, vector_num));
>
> apic_set_bit(s->irr, vector_num);
> if (trigger_mode)
>diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>index 2a20982066..4a34f03047 100644
>--- a/hw/intc/apic_common.c
>+++ b/hw/intc/apic_common.c
>@@ -25,6 +25,7 @@
> #include "qapi/visitor.h"
> #include "hw/i386/apic.h"
> #include "hw/i386/apic_internal.h"
>+#include "hw/intc/kvm_irqcount.h"
> #include "trace.h"
> #include "hw/boards.h"
> #include "sysemu/hax.h"
>@@ -33,7 +34,6 @@
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
>
>-static int apic_irq_delivered;
> bool apic_report_tpr_access;
>
> void cpu_set_apic_base(DeviceState *dev, uint64_t val)
>@@ -122,32 +122,6 @@ void apic_handle_tpr_access_report(DeviceState *dev, target_ulong ip,
> vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
> }
>
>-void apic_report_irq_delivered(int delivered)
>-{
>- apic_irq_delivered += delivered;
>-
>- trace_apic_report_irq_delivered(apic_irq_delivered);
>-}
>-
>-void apic_reset_irq_delivered(void)
>-{
>- /* Copy this into a local variable to encourage gcc to emit a plain
>- * register for a sys/sdt.h marker. For details on this workaround, see:
>- * https://sourceware.org/bugzilla/show_bug.cgi?id=13296
>- */
>- volatile int a_i_d = apic_irq_delivered;
>- trace_apic_reset_irq_delivered(a_i_d);
>-
>- apic_irq_delivered = 0;
>-}
>-
>-int apic_get_irq_delivered(void)
>-{
>- trace_apic_get_irq_delivered(apic_irq_delivered);
>-
>- return apic_irq_delivered;
>-}
>-
> void apic_deliver_nmi(DeviceState *dev)
> {
> APICCommonState *s = APIC_COMMON(dev);
>@@ -272,7 +246,7 @@ static void apic_reset_common(DeviceState *dev)
> s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
> s->id = s->initial_apic_id;
>
>- apic_reset_irq_delivered();
>+ kvm_reset_irq_delivered();
>
> s->vapic_paddr = 0;
> info->vapic_base_update(s);
>diff --git a/hw/intc/kvm_irqcount.c b/hw/intc/kvm_irqcount.c
>new file mode 100644
>index 0000000000..2ef8a83a7a
>--- /dev/null
>+++ b/hw/intc/kvm_irqcount.c
>@@ -0,0 +1,49 @@
>+/*
>+ * KVM PIC functions for counting the delivered IRQs.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * 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 <http://www.gnu.org/licenses/>
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "hw/intc/kvm_irqcount.h"
>+#include "trace.h"
>+
>+static int kvm_irq_delivered;
>+
>+void kvm_report_irq_delivered(int delivered)
>+{
>+ kvm_irq_delivered += delivered;
>+
>+ trace_kvm_report_irq_delivered(kvm_irq_delivered);
>+}
>+
>+void kvm_reset_irq_delivered(void)
>+{
>+ /*
>+ * Copy this into a local variable to encourage gcc to emit a plain
>+ * register for a sys/sdt.h marker. For details on this workaround, see:
>+ * https://sourceware.org/bugzilla/show_bug.cgi?id=13296
>+ */
>+ volatile int k_i_d = kvm_irq_delivered;
>+ trace_kvm_reset_irq_delivered(k_i_d);
>+
>+ kvm_irq_delivered = 0;
>+}
>+
>+int kvm_get_irq_delivered(void)
>+{
>+ trace_kvm_get_irq_delivered(kvm_irq_delivered);
>+
>+ return kvm_irq_delivered;
>+}
>diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>index 1ebb412479..947d68c257 100644
>--- a/hw/rtc/mc146818rtc.c
>+++ b/hw/rtc/mc146818rtc.c
>@@ -27,6 +27,7 @@
> #include "qemu/module.h"
> #include "qemu/bcd.h"
> #include "hw/acpi/acpi_aml_interface.h"
>+#include "hw/intc/kvm_irqcount.h"
> #include "hw/irq.h"
> #include "hw/qdev-properties.h"
> #include "hw/qdev-properties-system.h"
>@@ -46,7 +47,6 @@
>
> #ifdef TARGET_I386
> #include "qapi/qapi-commands-misc-target.h"
>-#include "hw/i386/apic.h"
> #endif
>
> //#define DEBUG_CMOS
>@@ -124,9 +124,9 @@ void qmp_rtc_reset_reinjection(Error **errp)
>
> static bool rtc_policy_slew_deliver_irq(RTCState *s)
> {
>- apic_reset_irq_delivered();
>+ kvm_reset_irq_delivered();
> qemu_irq_raise(s->irq);
>- return apic_get_irq_delivered();
>+ return kvm_get_irq_delivered();
> }
>
> static void rtc_coalesced_timer(void *opaque)
>diff --git a/hw/intc/meson.build b/hw/intc/meson.build
>index bcbf22ff51..cd9f1ee888 100644
>--- a/hw/intc/meson.build
>+++ b/hw/intc/meson.build
>@@ -25,6 +25,12 @@ softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_intc.c'))
> softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP', if_true: files('xlnx-zynqmp-ipi.c'))
> softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: files('xlnx-pmu-iomod-intc.c'))
>
>+if config_all_devices.has_key('CONFIG_APIC') or \
>+ config_all_devices.has_key('CONFIG_I8259') or \
>+ config_all_devices.has_key('CONFIG_MC146818RTC')
>+ softmmu_ss.add(files('kvm_irqcount.c'))
>+endif
>+
> specific_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: files('allwinner-a10-pic.c'))
> specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c'))
> specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif_common.c'))
>diff --git a/hw/intc/trace-events b/hw/intc/trace-events
>index 6fbc2045e6..50cadfb996 100644
>--- a/hw/intc/trace-events
>+++ b/hw/intc/trace-events
>@@ -10,10 +10,6 @@ pic_ioport_read(bool master, uint64_t addr, int val) "master %d addr 0x%"PRIx64"
> # apic_common.c
> cpu_set_apic_base(uint64_t val) "0x%016"PRIx64
> cpu_get_apic_base(uint64_t val) "0x%016"PRIx64
>-# coalescing
>-apic_report_irq_delivered(int apic_irq_delivered) "coalescing %d"
>-apic_reset_irq_delivered(int apic_irq_delivered) "old coalescing %d"
>-apic_get_irq_delivered(int apic_irq_delivered) "returning coalescing %d"
>
> # apic.c
> apic_local_deliver(int vector, uint32_t lvt) "vector %d delivery mode %d"
>@@ -30,6 +26,11 @@ ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapi
> ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
> ioapic_set_irq(int vector, int level) "vector: %d level: %d"
>
>+# kvm_irqcount.c
>+kvm_report_irq_delivered(int irq_delivered) "coalescing %d"
>+kvm_reset_irq_delivered(int irq_delivered) "old coalescing %d"
>+kvm_get_irq_delivered(int irq_delivered) "returning coalescing %d"
>+
> # slavio_intctl.c
> slavio_intctl_mem_readl(uint32_t cpu, uint64_t addr, uint32_t ret) "read cpu %d reg 0x%"PRIx64" = 0x%x"
> slavio_intctl_mem_writel(uint32_t cpu, uint64_t addr, uint32_t val) "write cpu %d reg 0x%"PRIx64" = 0x%x"
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 2/4] hw/core/qdev-properties-system: Allow the 'slew' policy only on x86
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
2023-01-10 9:53 ` [PATCH v6 1/4] hw/intc: Extract the IRQ counting functions into a separate file Thomas Huth
@ 2023-01-10 9:53 ` Thomas Huth
2023-01-11 22:52 ` Bernhard Beschow
2023-01-10 9:53 ` [PATCH v6 3/4] hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent Thomas Huth
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Thomas Huth @ 2023-01-10 9:53 UTC (permalink / raw)
To: Paolo Bonzini, Michael S Tsirkin, qemu-devel, Bernhard Beschow,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
The 'slew' tick policy is currently enforced to be only available on
x86 via some "#ifdef TARGET_I386" statements in mc146818rtc.c. We
want to get rid of those #ifdefs, so we need a different way of
checking whether the policy is allowed or not. Using the setter
function in hw/core/qdev-properties-system.c seems to be a good
place, so let's add a check here.
Suggested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/core/qdev-properties-system.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 54a09fa9ac..d42493f630 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -33,6 +33,7 @@
#include "net/net.h"
#include "hw/pci/pci.h"
#include "hw/pci/pcie.h"
+#include "hw/i386/x86.h"
#include "util/block-helpers.h"
static bool check_prop_still_unset(Object *obj, const char *name,
@@ -558,13 +559,38 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
/* --- lost tick policy --- */
+static void qdev_propinfo_set_losttickpolicy(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ Property *prop = opaque;
+ int *ptr = object_field_prop_ptr(obj, prop);
+ int value;
+
+ if (!visit_type_enum(v, name, &value, prop->info->enum_table, errp)) {
+ return;
+ }
+
+ if (value == LOST_TICK_POLICY_SLEW) {
+ MachineState *ms = MACHINE(qdev_get_machine());
+
+ if (!object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
+ error_setg(errp,
+ "the 'slew' policy is only available for x86 machines");
+ return;
+ }
+ }
+
+ *ptr = value;
+}
+
QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
const PropertyInfo qdev_prop_losttickpolicy = {
.name = "LostTickPolicy",
.enum_table = &LostTickPolicy_lookup,
.get = qdev_propinfo_get_enum,
- .set = qdev_propinfo_set_enum,
+ .set = qdev_propinfo_set_losttickpolicy,
.set_default_value = qdev_propinfo_set_default_value_enum,
};
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 2/4] hw/core/qdev-properties-system: Allow the 'slew' policy only on x86
2023-01-10 9:53 ` [PATCH v6 2/4] hw/core/qdev-properties-system: Allow the 'slew' policy only on x86 Thomas Huth
@ 2023-01-11 22:52 ` Bernhard Beschow
0 siblings, 0 replies; 10+ messages in thread
From: Bernhard Beschow @ 2023-01-11 22:52 UTC (permalink / raw)
To: Thomas Huth, Paolo Bonzini, Michael S Tsirkin, qemu-devel,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
Am 10. Januar 2023 09:53:49 UTC schrieb Thomas Huth <thuth@redhat.com>:
>The 'slew' tick policy is currently enforced to be only available on
>x86 via some "#ifdef TARGET_I386" statements in mc146818rtc.c. We
>want to get rid of those #ifdefs, so we need a different way of
>checking whether the policy is allowed or not. Using the setter
>function in hw/core/qdev-properties-system.c seems to be a good
>place, so let's add a check here.
>
>Suggested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>Signed-off-by: Thomas Huth <thuth@redhat.com>
>---
> hw/core/qdev-properties-system.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
>diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
>index 54a09fa9ac..d42493f630 100644
>--- a/hw/core/qdev-properties-system.c
>+++ b/hw/core/qdev-properties-system.c
>@@ -33,6 +33,7 @@
> #include "net/net.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pcie.h"
>+#include "hw/i386/x86.h"
> #include "util/block-helpers.h"
>
> static bool check_prop_still_unset(Object *obj, const char *name,
>@@ -558,13 +559,38 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
>
> /* --- lost tick policy --- */
>
>+static void qdev_propinfo_set_losttickpolicy(Object *obj, Visitor *v,
>+ const char *name, void *opaque,
>+ Error **errp)
>+{
>+ Property *prop = opaque;
>+ int *ptr = object_field_prop_ptr(obj, prop);
>+ int value;
>+
>+ if (!visit_type_enum(v, name, &value, prop->info->enum_table, errp)) {
>+ return;
>+ }
>+
>+ if (value == LOST_TICK_POLICY_SLEW) {
>+ MachineState *ms = MACHINE(qdev_get_machine());
>+
>+ if (!object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
>+ error_setg(errp,
>+ "the 'slew' policy is only available for x86 machines");
>+ return;
>+ }
>+ }
>+
>+ *ptr = value;
>+}
>+
> QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
>
> const PropertyInfo qdev_prop_losttickpolicy = {
> .name = "LostTickPolicy",
> .enum_table = &LostTickPolicy_lookup,
> .get = qdev_propinfo_get_enum,
>- .set = qdev_propinfo_set_enum,
>+ .set = qdev_propinfo_set_losttickpolicy,
> .set_default_value = qdev_propinfo_set_default_value_enum,
> };
>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 3/4] hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
2023-01-10 9:53 ` [PATCH v6 1/4] hw/intc: Extract the IRQ counting functions into a separate file Thomas Huth
2023-01-10 9:53 ` [PATCH v6 2/4] hw/core/qdev-properties-system: Allow the 'slew' policy only on x86 Thomas Huth
@ 2023-01-10 9:53 ` Thomas Huth
2023-01-10 9:53 ` [PATCH v6 4/4] softmmu/rtc: Emit warning when using driftfix=slew on systems without mc146818 Thomas Huth
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2023-01-10 9:53 UTC (permalink / raw)
To: Paolo Bonzini, Michael S Tsirkin, qemu-devel, Bernhard Beschow,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
The only reason for this code being target dependent was the IRQ-counting
related code in rtc_policy_slew_deliver_irq(). Since these functions have
been moved into a new, separate file (kvm_irqcount.c) which is now always
compiled and linked if necessary, we can get rid of the #ifdef TARGET_I386
switches in mc146818rtc.c and declare it in the softmmu_ss instead of
specific_ss, so that the code only gets compiled once for all targets.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/rtc/mc146818rtc.h | 1 +
hw/rtc/mc146818rtc.c | 14 --------------
hw/rtc/meson.build | 3 +--
3 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
index 1db0fcee92..45bcd6f040 100644
--- a/include/hw/rtc/mc146818rtc.h
+++ b/include/hw/rtc/mc146818rtc.h
@@ -55,5 +55,6 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
qemu_irq intercept_irq);
void rtc_set_memory(ISADevice *dev, int addr, int val);
int rtc_get_memory(ISADevice *dev, int addr);
+void qmp_rtc_reset_reinjection(Error **errp);
#endif /* HW_RTC_MC146818RTC_H */
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 947d68c257..bc1192b7ae 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -45,10 +45,6 @@
#include "qapi/visitor.h"
#include "hw/rtc/mc146818rtc_regs.h"
-#ifdef TARGET_I386
-#include "qapi/qapi-commands-misc-target.h"
-#endif
-
//#define DEBUG_CMOS
//#define DEBUG_COALESCED
@@ -112,7 +108,6 @@ static void rtc_coalesced_timer_update(RTCState *s)
static QLIST_HEAD(, RTCState) rtc_devices =
QLIST_HEAD_INITIALIZER(rtc_devices);
-#ifdef TARGET_I386
void qmp_rtc_reset_reinjection(Error **errp)
{
RTCState *s;
@@ -145,13 +140,6 @@ static void rtc_coalesced_timer(void *opaque)
rtc_coalesced_timer_update(s);
}
-#else
-static bool rtc_policy_slew_deliver_irq(RTCState *s)
-{
- assert(0);
- return false;
-}
-#endif
static uint32_t rtc_periodic_clock_ticks(RTCState *s)
{
@@ -922,12 +910,10 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
rtc_set_date_from_host(isadev);
switch (s->lost_tick_policy) {
-#ifdef TARGET_I386
case LOST_TICK_POLICY_SLEW:
s->coalesced_timer =
timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
break;
-#endif
case LOST_TICK_POLICY_DISCARD:
break;
default:
diff --git a/hw/rtc/meson.build b/hw/rtc/meson.build
index dc33973384..34a4d316fa 100644
--- a/hw/rtc/meson.build
+++ b/hw/rtc/meson.build
@@ -13,5 +13,4 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_rtc.c'))
softmmu_ss.add(when: 'CONFIG_GOLDFISH_RTC', if_true: files('goldfish_rtc.c'))
softmmu_ss.add(when: 'CONFIG_LS7A_RTC', if_true: files('ls7a_rtc.c'))
softmmu_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true: files('allwinner-rtc.c'))
-
-specific_ss.add(when: 'CONFIG_MC146818RTC', if_true: files('mc146818rtc.c'))
+softmmu_ss.add(when: 'CONFIG_MC146818RTC', if_true: files('mc146818rtc.c'))
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 4/4] softmmu/rtc: Emit warning when using driftfix=slew on systems without mc146818
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
` (2 preceding siblings ...)
2023-01-10 9:53 ` [PATCH v6 3/4] hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent Thomas Huth
@ 2023-01-10 9:53 ` Thomas Huth
2023-01-10 10:21 ` [PATCH v6 0/4] Make the mc146818 RTC device target independent Philippe Mathieu-Daudé
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2023-01-10 9:53 UTC (permalink / raw)
To: Paolo Bonzini, Michael S Tsirkin, qemu-devel, Bernhard Beschow,
Mark Cave-Ayland
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
The 'slew' lost tick policy is only available on systems with a mc146818
RTC. On other systems, "-rtc driftfix=slew" is currently silently ignored.
Let's emit at least a warning in this case to make the users aware that
there is something wrong in their command line settings.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
softmmu/rtc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/softmmu/rtc.c b/softmmu/rtc.c
index 7e2956f81e..f7114bed7d 100644
--- a/softmmu/rtc.c
+++ b/softmmu/rtc.c
@@ -33,6 +33,7 @@
#include "sysemu/replay.h"
#include "sysemu/sysemu.h"
#include "sysemu/rtc.h"
+#include "hw/rtc/mc146818rtc.h"
static enum {
RTC_BASE_UTC,
@@ -177,10 +178,13 @@ void configure_rtc(QemuOpts *opts)
value = qemu_opt_get(opts, "driftfix");
if (value) {
if (!strcmp(value, "slew")) {
- object_register_sugar_prop("mc146818rtc",
+ object_register_sugar_prop(TYPE_MC146818_RTC,
"lost_tick_policy",
"slew",
false);
+ if (!object_class_by_name(TYPE_MC146818_RTC)) {
+ warn_report("driftfix 'slew' is not available with this machine");
+ }
} else if (!strcmp(value, "none")) {
/* discard is default */
} else {
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 0/4] Make the mc146818 RTC device target independent
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
` (3 preceding siblings ...)
2023-01-10 9:53 ` [PATCH v6 4/4] softmmu/rtc: Emit warning when using driftfix=slew on systems without mc146818 Thomas Huth
@ 2023-01-10 10:21 ` Philippe Mathieu-Daudé
2023-01-10 23:16 ` Mark Cave-Ayland
2023-01-13 8:54 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-10 10:21 UTC (permalink / raw)
To: Thomas Huth, Paolo Bonzini, Michael S Tsirkin, qemu-devel,
Bernhard Beschow, Mark Cave-Ayland
Cc: BALATON Zoltan, Marcel Apfelbaum, Richard Henderson,
Eduardo Habkost
On 10/1/23 10:53, Thomas Huth wrote:
> The basic idea of this patch set is to change hw/rtc/mc146818rtc.c into
> target independent code so that the file only has to be compiled once
> instead of multiple times (and that it can be used in a qemu-system-all
> binary once we get there).
> Thomas Huth (4):
> hw/intc: Extract the IRQ counting functions into a separate file
> hw/core/qdev-properties-system: Allow the 'slew' policy only on x86
> hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent
> softmmu/rtc: Emit warning when using driftfix=slew on systems without
> mc146818
I was happy enough with each iterations, but the changes requested over
each iter was worth having a v6, it is now very clean.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v6 0/4] Make the mc146818 RTC device target independent
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
` (4 preceding siblings ...)
2023-01-10 10:21 ` [PATCH v6 0/4] Make the mc146818 RTC device target independent Philippe Mathieu-Daudé
@ 2023-01-10 23:16 ` Mark Cave-Ayland
2023-01-13 8:54 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 10+ messages in thread
From: Mark Cave-Ayland @ 2023-01-10 23:16 UTC (permalink / raw)
To: Thomas Huth, Paolo Bonzini, Michael S Tsirkin, qemu-devel,
Bernhard Beschow
Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Marcel Apfelbaum,
Richard Henderson, Eduardo Habkost
On 10/01/2023 09:53, Thomas Huth wrote:
> The basic idea of this patch set is to change hw/rtc/mc146818rtc.c into
> target independent code so that the file only has to be compiled once
> instead of multiple times (and that it can be used in a qemu-system-all
> binary once we get there).
>
> The first patch extracts some functions from the APIC code that will be
> required for linking when the mc146818rtc becomes target-independent.
>
> The second patch adds a new way for checking whether the "driftfix=slew"
> policy is available or not (since the corresponding #ifdefs in the
> mc146818rtc code will be removed).
>
> The third patch then removes the "#ifdef TARGET" switches and turns
> the mc146818rtc code into a target-independent file.
>
> The fourth patch just fixes a small cosmetic nit that I discovered along
> the way: On systems without mc146818, the "-rtc driftfix=slew" simply
> got ignored silently. We should at least emit a warning in this case.
>
> Changes since last iteration:
> - Dropped the approach of using a new "slew-tick-policy-available"
> property that needs to be set by the machine code (and thus dropped
> the clean-up patches from Bernhard from this series since they are
> no longer required here now)
> - Use a new check in hw/core/qdev-properties-system.c instead
> (see the second patch)
>
> Thomas Huth (4):
> hw/intc: Extract the IRQ counting functions into a separate file
> hw/core/qdev-properties-system: Allow the 'slew' policy only on x86
> hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent
> softmmu/rtc: Emit warning when using driftfix=slew on systems without
> mc146818
>
> include/hw/i386/apic.h | 2 --
> include/hw/i386/apic_internal.h | 1 -
> include/hw/intc/kvm_irqcount.h | 10 +++++++
> include/hw/rtc/mc146818rtc.h | 1 +
> hw/core/qdev-properties-system.c | 28 +++++++++++++++++-
> hw/i386/kvm/i8259.c | 4 +--
> hw/i386/kvm/ioapic.c | 4 +--
> hw/intc/apic.c | 3 +-
> hw/intc/apic_common.c | 30 ++-----------------
> hw/intc/kvm_irqcount.c | 49 ++++++++++++++++++++++++++++++++
> hw/rtc/mc146818rtc.c | 20 ++-----------
> softmmu/rtc.c | 6 +++-
> hw/intc/meson.build | 6 ++++
> hw/intc/trace-events | 9 +++---
> hw/rtc/meson.build | 3 +-
> 15 files changed, 115 insertions(+), 61 deletions(-)
> create mode 100644 include/hw/intc/kvm_irqcount.h
> create mode 100644 hw/intc/kvm_irqcount.c
This looks much better than the previous approaches - thanks for working on this!
Looks good to me, so:
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v6 0/4] Make the mc146818 RTC device target independent
2023-01-10 9:53 [PATCH v6 0/4] Make the mc146818 RTC device target independent Thomas Huth
` (5 preceding siblings ...)
2023-01-10 23:16 ` Mark Cave-Ayland
@ 2023-01-13 8:54 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-13 8:54 UTC (permalink / raw)
To: Thomas Huth, Paolo Bonzini, Michael S Tsirkin, qemu-devel,
Bernhard Beschow, Mark Cave-Ayland
Cc: BALATON Zoltan, Marcel Apfelbaum, Richard Henderson,
Eduardo Habkost
Hi Michael,
On 10/1/23 10:53, Thomas Huth wrote:
> The basic idea of this patch set is to change hw/rtc/mc146818rtc.c into
> target independent code so that the file only has to be compiled once
> instead of multiple times (and that it can be used in a qemu-system-all
> binary once we get there).
> Thomas Huth (4):
> hw/intc: Extract the IRQ counting functions into a separate file
> hw/core/qdev-properties-system: Allow the 'slew' policy only on x86
> hw/rtc/mc146818rtc: Make the mc146818 RTC device target independent
> softmmu/rtc: Emit warning when using driftfix=slew on systems without
> mc146818
I am queuing this series via mips-next tree if you don't object
(all patches reviewed multiple times, in case there is an issue,
we are early enough in the release cycle to fix it).
Regards,
Phil.
^ permalink raw reply [flat|nested] 10+ messages in thread