All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs
@ 2023-02-15 17:43 Philippe Mathieu-Daudé
  2023-02-15 17:43 ` [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header Philippe Mathieu-Daudé
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé

i8254_pit_init() uses a odd pattern of "use this IRQ output
line if non-NULL, otherwise use the ISA IRQ #number as output".

Rework as simply "Use this IRQ output".

Un-inline/rename/document functions.

Based-on: <20230215161641.32663-1-philmd@linaro.org>
          "hw/ide: Untangle ISA/PCI abuses of ide_init_ioport" v2
https://lore.kernel.org/qemu-devel/20230215161641.32663-1-philmd@linaro.org/

Philippe Mathieu-Daudé (5):
  hw/timer/hpet: Include missing 'hw/qdev-properties.h' header
  hw/timer/i8254: Factor i8254_pit_create() out and document
  hw/i386/pc: Un-inline i8254_pit_init()
  hw/timer/i8254: Really inline i8254_pit_init()
  hw/i386/kvm: Factor i8254_pit_create_try_kvm() out

 hw/i386/kvm/i8254.c        | 18 ++++++++++++++
 hw/i386/microvm.c          |  6 +----
 hw/i386/pc.c               | 15 +++++-------
 hw/isa/i82378.c            |  2 +-
 hw/isa/piix4.c             |  4 ++--
 hw/isa/vt82c686.c          |  2 +-
 hw/mips/jazz.c             |  2 +-
 hw/timer/hpet.c            |  1 +
 hw/timer/i8254.c           | 16 +++++++++++++
 include/hw/timer/i8254.h   | 48 +++++++++++++-------------------------
 target/i386/kvm/kvm-stub.c |  6 +++++
 11 files changed, 69 insertions(+), 51 deletions(-)

-- 
2.38.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
@ 2023-02-15 17:43 ` Philippe Mathieu-Daudé
  2023-02-15 18:55   ` Richard Henderson
  2023-02-15 17:43 ` [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé

Avoid when refactoring unrelated headers:

  hw/timer/hpet.c:776:39: error: array has incomplete element type 'Property' (aka 'struct Property')
  static Property hpet_device_properties[] = {
                                        ^
  hw/timer/hpet.c:777:5: error: implicit declaration of function 'DEFINE_PROP_UINT8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
      DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS),
      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/timer/hpet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 9520471be2..214d6a0501 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -30,6 +30,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
+#include "hw/qdev-properties.h"
 #include "hw/timer/hpet.h"
 #include "hw/sysbus.h"
 #include "hw/rtc/mc146818rtc.h"
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
  2023-02-15 17:43 ` [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header Philippe Mathieu-Daudé
@ 2023-02-15 17:43 ` Philippe Mathieu-Daudé
  2023-02-15 18:56   ` Richard Henderson
  2023-02-15 17:43 ` [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/timer/i8254.c         | 16 ++++++++++++++++
 include/hw/timer/i8254.h | 24 +++++++++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
index c8388ea432..9b6932ab08 100644
--- a/hw/timer/i8254.c
+++ b/hw/timer/i8254.c
@@ -26,9 +26,11 @@
 #include "hw/irq.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
+#include "hw/qdev-properties.h"
 #include "hw/timer/i8254.h"
 #include "hw/timer/i8254_internal.h"
 #include "qom/object.h"
+#include "qapi/error.h"
 
 //#define DEBUG_PIT
 
@@ -47,6 +49,20 @@ struct PITClass {
     DeviceRealize parent_realize;
 };
 
+ISADevice *i8254_pit_create(ISABus *bus, int iobase, qemu_irq irq_in)
+{
+    DeviceState *dev;
+    ISADevice *d;
+
+    d = isa_new(TYPE_I8254);
+    dev = DEVICE(d);
+    qdev_prop_set_uint32(dev, "iobase", iobase);
+    isa_realize_and_unref(d, bus, &error_fatal);
+    qdev_connect_gpio_out(dev, 0, irq_in);
+
+    return d;
+}
+
 static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
 
 static int pit_get_count(PITChannelState *s)
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index 8402caad30..a0843cae07 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -45,21 +45,23 @@ OBJECT_DECLARE_TYPE(PITCommonState, PITCommonClass, PIT_COMMON)
 #define TYPE_I8254 "isa-pit"
 #define TYPE_KVM_I8254 "kvm-pit"
 
+/**
+ * Create and realize a I8254 PIT device on the heap.
+ * @bus: the #ISABus to put it on.
+ * @iobase: the base I/O port.
+ * @irq_in: qemu_irq to connect the PIT output IRQ to.
+ *
+ * Create the device state structure, initialize it, put it on the
+ * specified ISA @bus, and drop the reference to it (the device is realized).
+ */
+ISADevice *i8254_pit_create(ISABus *bus, int iobase, qemu_irq irq_in);
+
 static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
                                         qemu_irq alt_irq)
 {
-    DeviceState *dev;
-    ISADevice *d;
-
-    d = isa_new(TYPE_I8254);
-    dev = DEVICE(d);
-    qdev_prop_set_uint32(dev, "iobase", base);
-    isa_realize_and_unref(d, bus, &error_fatal);
-    qdev_connect_gpio_out(dev, 0,
-                          isa_irq >= 0 ? isa_bus_get_irq(bus, isa_irq)
+    return i8254_pit_create(bus, base, isa_irq >= 0
+                                       ? isa_bus_get_irq(bus, isa_irq)
                                        : alt_irq);
-
-    return d;
 }
 
 static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init()
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
  2023-02-15 17:43 ` [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header Philippe Mathieu-Daudé
  2023-02-15 17:43 ` [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document Philippe Mathieu-Daudé
@ 2023-02-15 17:43 ` Philippe Mathieu-Daudé
  2023-02-15 18:58   ` Richard Henderson
  2023-02-15 17:43 ` [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé

pc_basic_device_init() is the single caller of i8254_pit_init()
with a non-NULL 'alt_irq' argument. Open-code i8254_pit_init()
by direclty calling i8254_pit_create().

To confirm all other callers pass a NULL 'alt_irq', add an
assertion in i8254_pit_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c             | 10 +++++-----
 include/hw/timer/i8254.h |  5 ++---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 79297a6ecd..fe95f6e9f2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1258,8 +1258,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
 {
     int i;
     DeviceState *hpet = NULL;
-    int pit_isa_irq = 0;
-    qemu_irq pit_alt_irq = NULL;
+    qemu_irq pit_irq;
     qemu_irq rtc_irq = NULL;
     ISADevice *pit = NULL;
     MemoryRegion *ioport80_io = g_new(MemoryRegion, 1);
@@ -1301,9 +1300,10 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         for (i = 0; i < GSI_NUM_PINS; i++) {
             sysbus_connect_irq(SYS_BUS_DEVICE(hpet), i, gsi[i]);
         }
-        pit_isa_irq = -1;
-        pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
+        pit_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
         rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
+    } else {
+        pit_irq = isa_bus_get_irq(isa_bus, 0);
     }
     *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
 
@@ -1314,7 +1314,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         if (kvm_pit_in_kernel()) {
             pit = kvm_pit_init(isa_bus, 0x40);
         } else {
-            pit = i8254_pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq);
+            pit = i8254_pit_create(isa_bus, 0x40, pit_irq);
         }
         if (hpet) {
             /* connect PIT to output control line of the HPET */
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index a0843cae07..0d837f3f41 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -59,9 +59,8 @@ ISADevice *i8254_pit_create(ISABus *bus, int iobase, qemu_irq irq_in);
 static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
                                         qemu_irq alt_irq)
 {
-    return i8254_pit_create(bus, base, isa_irq >= 0
-                                       ? isa_bus_get_irq(bus, isa_irq)
-                                       : alt_irq);
+    assert(isa_irq == 0 && alt_irq == NULL);
+    return i8254_pit_create(bus, base, isa_bus_get_irq(bus, 0));
 }
 
 static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init()
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2023-02-15 17:43 ` [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init() Philippe Mathieu-Daudé
@ 2023-02-15 17:43 ` Philippe Mathieu-Daudé
  2023-02-15 19:00   ` Richard Henderson
  2023-02-15 17:43 ` [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out Philippe Mathieu-Daudé
  2023-03-01 21:54 ` [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Michael S. Tsirkin
  5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé,
	Hervé Poussineau, Aurelien Jarno, Huacai Chen, Jiaxun Yang,
	Aleksandar Rikalo

In-line the one-line i8254_pit_create() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/microvm.c        | 2 +-
 hw/isa/i82378.c          | 2 +-
 hw/isa/piix4.c           | 4 ++--
 hw/isa/vt82c686.c        | 2 +-
 hw/mips/jazz.c           | 2 +-
 include/hw/timer/i8254.h | 7 -------
 6 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 29f30dd6d3..9204bb4ff2 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -261,7 +261,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
         if (kvm_pit_in_kernel()) {
             kvm_pit_init(isa_bus, 0x40);
         } else {
-            i8254_pit_init(isa_bus, 0x40, 0, NULL);
+            i8254_pit_create(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
         }
     }
 
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index e3322e03bf..f75f5a23ba 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -99,7 +99,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     isa_bus_irqs(isabus, s->i8259);
 
     /* 1 82C54 (pit) */
-    pit = i8254_pit_init(isabus, 0x40, 0, NULL);
+    pit = i8254_pit_create(isabus, 0x40, isa_bus_get_irq(isabus, 0));
 
     /* speaker */
     pcspk_init(isa_new(TYPE_PC_SPEAKER), isabus, pit);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 94e5dc7825..8ff118445f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -214,8 +214,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
     /* initialize ISA irqs */
     isa_bus_irqs(isa_bus, s->isa);
 
-    /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    /* PIT */
+    i8254_pit_create(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
 
     /* DMA */
     i8257_dma_init(isa_bus, 0);
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 3f9bd0c04d..297bcda420 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -624,7 +624,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
 
     s->isa_irqs = i8259_init(isa_bus, *isa_irq);
     isa_bus_irqs(isa_bus, s->isa_irqs);
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    i8254_pit_create(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
     i8257_dma_init(isa_bus, 0);
 
     /* RTC */
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 6aefe9a61b..856acfe2a7 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -251,7 +251,7 @@ static void mips_jazz_init(MachineState *machine,
     i8259 = i8259_init(isa_bus, env->irq[4]);
     isa_bus_irqs(isa_bus, i8259);
     i8257_dma_init(isa_bus, 0);
-    pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    pit = i8254_pit_create(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
     pcspk_init(isa_new(TYPE_PC_SPEAKER), isa_bus, pit);
 
     /* Video card */
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index 0d837f3f41..aa48c44d39 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -56,13 +56,6 @@ OBJECT_DECLARE_TYPE(PITCommonState, PITCommonClass, PIT_COMMON)
  */
 ISADevice *i8254_pit_create(ISABus *bus, int iobase, qemu_irq irq_in);
 
-static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq,
-                                        qemu_irq alt_irq)
-{
-    assert(isa_irq == 0 && alt_irq == NULL);
-    return i8254_pit_create(bus, base, isa_bus_get_irq(bus, 0));
-}
-
 static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
 {
     DeviceState *dev;
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2023-02-15 17:43 ` [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init() Philippe Mathieu-Daudé
@ 2023-02-15 17:43 ` Philippe Mathieu-Daudé
  2023-02-15 19:02   ` Richard Henderson
  2023-03-01 21:54 ` [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Michael S. Tsirkin
  5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-15 17:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, Richard Henderson,
	kvm, Michael S. Tsirkin, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum, Philippe Mathieu-Daudé

Factor a new i8254_pit_create_try_kvm() helper out of the
following patter:

  if (kvm_pit_in_kernel()) {
      kvm_pit_init(...);
  } else }
    i8254_pit_create(...);
  }

(adding a stub for non-KVM builds).

Since kvm_pit_init() is only used once, un-inline it and
remove the now unused headers from "hw/timer/i8254.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/kvm/i8254.c        | 18 ++++++++++++++++++
 hw/i386/microvm.c          |  6 +-----
 hw/i386/pc.c               |  7 ++-----
 include/hw/timer/i8254.h   | 22 ++++++----------------
 target/i386/kvm/kvm-stub.c |  6 ++++++
 5 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index 191a26fa57..9520c98c76 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -35,6 +35,7 @@
 #include "hw/qdev-properties-system.h"
 #include "sysemu/kvm.h"
 #include "qom/object.h"
+#include "kvm/kvm_i386.h"
 
 #define KVM_PIT_REINJECT_BIT 0
 
@@ -59,6 +60,23 @@ struct KVMPITClass {
     DeviceRealize parent_realize;
 };
 
+ISADevice *i8254_pit_create_try_kvm(ISABus *bus, int iobase, qemu_irq irq_in)
+{
+    DeviceState *dev;
+    ISADevice *d;
+
+    if (!kvm_pit_in_kernel()) {
+        return i8254_pit_create(bus, iobase, irq_in);
+    }
+
+    d = isa_new(TYPE_KVM_I8254);
+    dev = DEVICE(d);
+    qdev_prop_set_uint32(dev, "iobase", iobase);
+    isa_realize_and_unref(d, bus, &error_fatal);
+
+    return d;
+}
+
 static void kvm_pit_update_clock_offset(KVMPITState *s)
 {
     int64_t offset, clock_offset;
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 9204bb4ff2..d93dc8a263 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -258,11 +258,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
     }
 
     if (x86ms->pit == ON_OFF_AUTO_ON || x86ms->pit == ON_OFF_AUTO_AUTO) {
-        if (kvm_pit_in_kernel()) {
-            kvm_pit_init(isa_bus, 0x40);
-        } else {
-            i8254_pit_create(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
-        }
+        i8254_pit_create_try_kvm(isa_bus, 0x40, isa_bus_get_irq(isa_bus, 0));
     }
 
     if (mms->rtc == ON_OFF_AUTO_ON ||
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fe95f6e9f2..7d9f57c695 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1311,11 +1311,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
 
     if (!xen_enabled() &&
         (x86ms->pit == ON_OFF_AUTO_AUTO || x86ms->pit == ON_OFF_AUTO_ON)) {
-        if (kvm_pit_in_kernel()) {
-            pit = kvm_pit_init(isa_bus, 0x40);
-        } else {
-            pit = i8254_pit_create(isa_bus, 0x40, pit_irq);
-        }
+        pit = i8254_pit_create_try_kvm(isa_bus, 0x40, pit_irq);
+
         if (hpet) {
             /* connect PIT to output control line of the HPET */
             qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(DEVICE(pit), 0));
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index aa48c44d39..9fb18c4ff4 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -25,9 +25,6 @@
 #ifndef HW_I8254_H
 #define HW_I8254_H
 
-#include "hw/qdev-properties.h"
-#include "hw/isa/isa.h"
-#include "qapi/error.h"
 #include "qom/object.h"
 
 #define PIT_FREQ 1193182
@@ -55,19 +52,12 @@ OBJECT_DECLARE_TYPE(PITCommonState, PITCommonClass, PIT_COMMON)
  * specified ISA @bus, and drop the reference to it (the device is realized).
  */
 ISADevice *i8254_pit_create(ISABus *bus, int iobase, qemu_irq irq_in);
-
-static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
-{
-    DeviceState *dev;
-    ISADevice *d;
-
-    d = isa_new(TYPE_KVM_I8254);
-    dev = DEVICE(d);
-    qdev_prop_set_uint32(dev, "iobase", base);
-    isa_realize_and_unref(d, bus, &error_fatal);
-
-    return d;
-}
+/**
+ * Try to create and realize a in-kernel I8254 PIT device on the heap.
+ * If KVM is not available or doesn't have in-kernel PIT support, a
+ * emulated PIT is used. See i8254_pit_create.
+ */
+ISADevice *i8254_pit_create_try_kvm(ISABus *bus, int iobase, qemu_irq irq_in);
 
 void pit_set_gate(ISADevice *dev, int channel, int val);
 void pit_get_channel_info(ISADevice *dev, int channel, PITChannelInfo *info);
diff --git a/target/i386/kvm/kvm-stub.c b/target/i386/kvm/kvm-stub.c
index e052f1c7b0..396da89f55 100644
--- a/target/i386/kvm/kvm-stub.c
+++ b/target/i386/kvm/kvm-stub.c
@@ -10,6 +10,7 @@
  *
  */
 #include "qemu/osdep.h"
+#include "hw/timer/i8254.h"
 #include "cpu.h"
 #include "kvm_i386.h"
 
@@ -49,3 +50,8 @@ void kvm_set_max_apic_id(uint32_t max_apic_id)
 {
     return;
 }
+
+ISADevice *i8254_pit_create_try_kvm(ISABus *bus, int iobase, qemu_irq irq_in)
+{
+    return i8254_pit_create(bus, iobase, irq_in);
+}
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header
  2023-02-15 17:43 ` [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header Philippe Mathieu-Daudé
@ 2023-02-15 18:55   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-02-15 18:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, kvm,
	Michael S. Tsirkin, Eduardo Habkost, qemu-ppc, Marcel Apfelbaum

On 2/15/23 07:43, Philippe Mathieu-Daudé wrote:
> Avoid when refactoring unrelated headers:
> 
>    hw/timer/hpet.c:776:39: error: array has incomplete element type 'Property' (aka 'struct Property')
>    static Property hpet_device_properties[] = {
>                                          ^
>    hw/timer/hpet.c:777:5: error: implicit declaration of function 'DEFINE_PROP_UINT8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>        DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS),
>        ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/timer/hpet.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 9520471be2..214d6a0501 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -30,6 +30,7 @@
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "qemu/timer.h"
> +#include "hw/qdev-properties.h"
>   #include "hw/timer/hpet.h"
>   #include "hw/sysbus.h"
>   #include "hw/rtc/mc146818rtc.h"

Acked-by: Richard Henderson <richard.henderson@linaro.org>

r~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document
  2023-02-15 17:43 ` [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document Philippe Mathieu-Daudé
@ 2023-02-15 18:56   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-02-15 18:56 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, kvm,
	Michael S. Tsirkin, Eduardo Habkost, qemu-ppc, Marcel Apfelbaum

On 2/15/23 07:43, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/timer/i8254.c         | 16 ++++++++++++++++
>   include/hw/timer/i8254.h | 24 +++++++++++++-----------
>   2 files changed, 29 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init()
  2023-02-15 17:43 ` [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init() Philippe Mathieu-Daudé
@ 2023-02-15 18:58   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-02-15 18:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, kvm,
	Michael S. Tsirkin, Eduardo Habkost, qemu-ppc, Marcel Apfelbaum

On 2/15/23 07:43, Philippe Mathieu-Daudé wrote:
> pc_basic_device_init() is the single caller of i8254_pit_init()
> with a non-NULL 'alt_irq' argument. Open-code i8254_pit_init()
> by direclty calling i8254_pit_create().
> 
> To confirm all other callers pass a NULL 'alt_irq', add an
> assertion in i8254_pit_init().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i386/pc.c             | 10 +++++-----
>   include/hw/timer/i8254.h |  5 ++---
>   2 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init()
  2023-02-15 17:43 ` [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init() Philippe Mathieu-Daudé
@ 2023-02-15 19:00   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-02-15 19:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel

On 2/15/23 07:43, Philippe Mathieu-Daudé wrote:
> In-line the one-line i8254_pit_create() call.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i386/microvm.c        | 2 +-
>   hw/isa/i82378.c          | 2 +-
>   hw/isa/piix4.c           | 4 ++--
>   hw/isa/vt82c686.c        | 2 +-
>   hw/mips/jazz.c           | 2 +-
>   include/hw/timer/i8254.h | 7 -------
>   6 files changed, 6 i

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out
  2023-02-15 17:43 ` [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out Philippe Mathieu-Daudé
@ 2023-02-15 19:02   ` Richard Henderson
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2023-02-15 19:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Marcelo Tosatti, Sergio Lopez, Paolo Bonzini, kvm,
	Michael S. Tsirkin, Eduardo Habkost, qemu-ppc, Marcel Apfelbaum

On 2/15/23 07:43, Philippe Mathieu-Daudé wrote:
> Factor a new i8254_pit_create_try_kvm() helper out of the
> following patter:

pattern

> 
>    if (kvm_pit_in_kernel()) {
>        kvm_pit_init(...);
>    } else }
>      i8254_pit_create(...);
>    }
> 
> (adding a stub for non-KVM builds).
> 
> Since kvm_pit_init() is only used once, un-inline it and
> remove the now unused headers from "hw/timer/i8254.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i386/kvm/i8254.c        | 18 ++++++++++++++++++
>   hw/i386/microvm.c          |  6 +-----
>   hw/i386/pc.c               |  7 ++-----
>   include/hw/timer/i8254.h   | 22 ++++++----------------
>   target/i386/kvm/kvm-stub.c |  6 ++++++
>   5 files changed, 33 insertions(+), 26 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs
  2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2023-02-15 17:43 ` [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out Philippe Mathieu-Daudé
@ 2023-03-01 21:54 ` Michael S. Tsirkin
  5 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2023-03-01 21:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Marcelo Tosatti, Sergio Lopez, Paolo Bonzini,
	Richard Henderson, kvm, Eduardo Habkost, qemu-ppc,
	Marcel Apfelbaum

On Wed, Feb 15, 2023 at 06:43:48PM +0100, Philippe Mathieu-Daudé wrote:
> i8254_pit_init() uses a odd pattern of "use this IRQ output
> line if non-NULL, otherwise use the ISA IRQ #number as output".
> 
> Rework as simply "Use this IRQ output".


Acked-by: Michael S. Tsirkin <mst@redhat.com>


Given it also affects KVM I will let Paolo merge this.

> Un-inline/rename/document functions.
> 
> Based-on: <20230215161641.32663-1-philmd@linaro.org>
>           "hw/ide: Untangle ISA/PCI abuses of ide_init_ioport" v2
> https://lore.kernel.org/qemu-devel/20230215161641.32663-1-philmd@linaro.org/
> 
> Philippe Mathieu-Daudé (5):
>   hw/timer/hpet: Include missing 'hw/qdev-properties.h' header
>   hw/timer/i8254: Factor i8254_pit_create() out and document
>   hw/i386/pc: Un-inline i8254_pit_init()
>   hw/timer/i8254: Really inline i8254_pit_init()
>   hw/i386/kvm: Factor i8254_pit_create_try_kvm() out
> 
>  hw/i386/kvm/i8254.c        | 18 ++++++++++++++
>  hw/i386/microvm.c          |  6 +----
>  hw/i386/pc.c               | 15 +++++-------
>  hw/isa/i82378.c            |  2 +-
>  hw/isa/piix4.c             |  4 ++--
>  hw/isa/vt82c686.c          |  2 +-
>  hw/mips/jazz.c             |  2 +-
>  hw/timer/hpet.c            |  1 +
>  hw/timer/i8254.c           | 16 +++++++++++++
>  include/hw/timer/i8254.h   | 48 +++++++++++++-------------------------
>  target/i386/kvm/kvm-stub.c |  6 +++++
>  11 files changed, 69 insertions(+), 51 deletions(-)
> 
> -- 
> 2.38.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-01 21:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-15 17:43 [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Philippe Mathieu-Daudé
2023-02-15 17:43 ` [PATCH 1/5] hw/timer/hpet: Include missing 'hw/qdev-properties.h' header Philippe Mathieu-Daudé
2023-02-15 18:55   ` Richard Henderson
2023-02-15 17:43 ` [PATCH 2/5] hw/timer/i8254: Factor i8254_pit_create() out and document Philippe Mathieu-Daudé
2023-02-15 18:56   ` Richard Henderson
2023-02-15 17:43 ` [PATCH 3/5] hw/i386/pc: Un-inline i8254_pit_init() Philippe Mathieu-Daudé
2023-02-15 18:58   ` Richard Henderson
2023-02-15 17:43 ` [PATCH 4/5] hw/timer/i8254: Really inline i8254_pit_init() Philippe Mathieu-Daudé
2023-02-15 19:00   ` Richard Henderson
2023-02-15 17:43 ` [PATCH 5/5] hw/i386/kvm: Factor i8254_pit_create_try_kvm() out Philippe Mathieu-Daudé
2023-02-15 19:02   ` Richard Henderson
2023-03-01 21:54 ` [PATCH 0/5] hw/timer/i8254: Un-inline and simplify IRQs Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.