qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
@ 2023-01-02 21:34 Bernhard Beschow
  2023-01-02 21:34 ` [PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it Bernhard Beschow
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
machine agnostic to the precise southbridge being used. 2/ will become
particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
the "Frankenstein" use of PIIX4_ACPI in PIIX3.

Testing done:
None, because I don't know how to conduct this properly :(

Based-on: <20221221170003.2929-1-shentey@gmail.com>
          "[PATCH v4 00/30] Consolidate PIIX south bridges"

Bernhard Beschow (6):
  include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
  hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
  hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
  hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
  hw/isa/piix: Resolve redundant k->config_write assignments
  hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE

 hw/i386/pc_piix.c             | 34 ++++++++++++++++--
 hw/i386/xen/xen-hvm.c         |  9 +++--
 hw/isa/piix.c                 | 66 +----------------------------------
 include/hw/southbridge/piix.h |  1 -
 include/hw/xen/xen.h          |  2 +-
 stubs/xen-hw-stub.c           |  2 +-
 6 files changed, 40 insertions(+), 74 deletions(-)

-- 
2.39.0



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

* [PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
@ 2023-01-02 21:34 ` Bernhard Beschow
  2023-01-02 21:35 ` [PATCH 2/6] hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize() Bernhard Beschow
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

xen_piix3_set_irq() hardcoded the number of PCI IRQ lines. Get it from
the PCI bus instead.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/xen/xen-hvm.c | 9 ++++++---
 hw/isa/piix.c         | 2 +-
 include/hw/xen/xen.h  | 2 +-
 stubs/xen-hw-stub.c   | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index e4293d6d66..59e8246a48 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -142,10 +142,13 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return irq_num + (PCI_SLOT(pci_dev->devfn) << 2);
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
-    xen_set_pci_intx_level(xen_domid, 0, 0, irq_num >> 2,
-                           irq_num & 3, level);
+    PCIDevice *pci_dev = opaque;
+    PCIBus *pci_bus = pci_get_bus(pci_dev);
+
+    xen_set_pci_intx_level(xen_domid, 0, 0, irq_num / pci_bus->nirq,
+                           irq_num % pci_bus->nirq, level);
 }
 
 int xen_set_pci_link_route(uint8_t link, uint8_t irq)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index ae8a27c53c..dc6014a4e4 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -504,7 +504,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
      * connected to the IOAPIC directly.
      * These additional routes can be discovered through ACPI.
      */
-    pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
+    pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
 }
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index afdf9c436a..7c83ecf6b9 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -22,7 +22,7 @@ extern bool xen_domid_restrict;
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 int xen_set_pci_link_route(uint8_t link, uint8_t irq);
-void xen_piix3_set_irq(void *opaque, int irq_num, int level);
+void xen_intx_set_irq(void *opaque, int irq_num, int level);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
 int xen_is_pirq_msi(uint32_t msi_data);
 
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 34a22f2ad7..7d7ffe83a9 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -15,7 +15,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
     return -1;
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
 }
 
-- 
2.39.0



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

* [PATCH 2/6] hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
  2023-01-02 21:34 ` [PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it Bernhard Beschow
@ 2023-01-02 21:35 ` Bernhard Beschow
  2023-01-02 21:35 ` [PATCH 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3 Bernhard Beschow
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

This is a preparational patch for the next one to make the following
more obvious:

First, pci_bus_irqs() is now called twice in case of Xen where the
second call overrides the pci_set_irq_fn with the Xen variant.

Second, pci_bus_set_route_irq_fn() is now also called in Xen mode.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index dc6014a4e4..a1281c2d77 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -493,7 +493,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
     PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
     PCIBus *pci_bus = pci_get_bus(dev);
 
-    pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
+    piix3_realize(dev, errp);
     if (*errp) {
         return;
     }
-- 
2.39.0



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

* [PATCH 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
  2023-01-02 21:34 ` [PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it Bernhard Beschow
  2023-01-02 21:35 ` [PATCH 2/6] hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize() Bernhard Beschow
@ 2023-01-02 21:35 ` Bernhard Beschow
  2023-01-02 21:35 ` [PATCH 4/6] hw/isa/piix: Avoid Xen-specific variant of piix_write_config() Bernhard Beschow
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

xen_intx_set_irq() doesn't depend on PIIX state. In order to resolve
TYPE_PIIX3_XEN_DEVICE and in order to make Xen agnostic about the
precise south bridge being used, set up Xen's PCI IRQ handling of PIIX3
in the board.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c | 12 ++++++++++++
 hw/isa/piix.c     | 24 +-----------------------
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index aacdb72b7c..c02f68010d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -67,6 +67,7 @@
 #include "kvm/kvm-cpu.h"
 
 #define MAX_IDE_BUS 2
+#define XEN_PIIX_NUM_PIRQS 128ULL
 
 #ifdef CONFIG_IDE_ISA
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
@@ -246,6 +247,17 @@ static void pc_init1(MachineState *machine,
                                  &error_abort);
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
+        if (xen_enabled()) {
+            /*
+             * Xen supports additional interrupt routes from the PCI devices to
+             * the IOAPIC: the four pins of each PCI device on the bus are also
+             * connected to the IOAPIC directly.
+             * These additional routes can be discovered through ACPI.
+             */
+            pci_bus_irqs(pci_bus, xen_intx_set_irq, pci_dev,
+                         XEN_PIIX_NUM_PIRQS);
+        }
+
         dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index a1281c2d77..ac04781f46 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -38,8 +38,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-#define XEN_PIIX_NUM_PIRQS      128ULL
-
 static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
     qemu_set_irq(piix->pic.in_irqs[pic_irq],
@@ -487,33 +485,13 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
-static void piix3_xen_realize(PCIDevice *dev, Error **errp)
-{
-    ERRP_GUARD();
-    PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
-    PCIBus *pci_bus = pci_get_bus(dev);
-
-    piix3_realize(dev, errp);
-    if (*errp) {
-        return;
-    }
-
-    /*
-     * Xen supports additional interrupt routes from the PCI devices to
-     * the IOAPIC: the four pins of each PCI device on the bus are also
-     * connected to the IOAPIC directly.
-     * These additional routes can be discovered through ACPI.
-     */
-    pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
-}
-
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->config_write = piix3_write_config_xen;
-    k->realize = piix3_xen_realize;
+    k->realize = piix3_realize;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
     dc->vmsd = &vmstate_piix3;
-- 
2.39.0



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

* [PATCH 4/6] hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
                   ` (2 preceding siblings ...)
  2023-01-02 21:35 ` [PATCH 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3 Bernhard Beschow
@ 2023-01-02 21:35 ` Bernhard Beschow
  2023-01-02 21:35 ` [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments Bernhard Beschow
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

Subscribe to pci_bus_fire_intx_routing_notifier() instead which allows for
having a common piix_write_config() for all PIIX device models.

While at it, move the subscription into machine code in order to resolve
TYPE_PIIX3_XEN_DEVICE.

In a possible future followup, pci_bus_fire_intx_routing_notifier() could
be adjusted in such a way that subscribing to it doesn't require
knowledge of the device firing it.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c | 18 ++++++++++++++++++
 hw/isa/piix.c     | 22 +---------------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c02f68010d..7ef0054b3a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -86,6 +86,21 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
     return (pci_intx + slot_addend) & 3;
 }
 
+static void piix_intx_routing_notifier_xen(PCIDevice *dev)
+{
+    int i;
+
+    /* Scan for updates to PCI link routes (0x60-0x63). */
+    for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+        uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1);
+        if (v & 0x80) {
+            v = 0;
+        }
+        v &= 0xf;
+        xen_set_pci_link_route(i, v);
+    }
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      const char *host_type, const char *pci_type)
@@ -248,6 +263,9 @@ static void pc_init1(MachineState *machine,
         pci_realize_and_unref(pci_dev, pci_bus, &error_fatal);
 
         if (xen_enabled()) {
+            pci_device_set_intx_routing_notifier(
+                        pci_dev, piix_intx_routing_notifier_xen);
+
             /*
              * Xen supports additional interrupt routes from the PCI devices to
              * the IOAPIC: the four pins of each PCI device on the bus are also
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index ac04781f46..d4cdb3dadb 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -125,26 +125,6 @@ static void piix_write_config(PCIDevice *dev, uint32_t address, uint32_t val,
     }
 }
 
-static void piix3_write_config_xen(PCIDevice *dev,
-                                   uint32_t address, uint32_t val, int len)
-{
-    int i;
-
-    /* Scan for updates to PCI link routes (0x60-0x63). */
-    for (i = 0; i < len; i++) {
-        uint8_t v = (val >> (8 * i)) & 0xff;
-        if (v & 0x80) {
-            v = 0;
-        }
-        v &= 0xf;
-        if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) {
-            xen_set_pci_link_route(address + i - PIIX_PIRQCA, v);
-        }
-    }
-
-    piix_write_config(dev, address, val, len);
-}
-
 static void piix_reset(DeviceState *dev)
 {
     PIIXState *d = PIIX_PCI_DEVICE(dev);
@@ -490,7 +470,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix3_write_config_xen;
+    k->config_write = piix_write_config;
     k->realize = piix3_realize;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
-- 
2.39.0



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

* [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
                   ` (3 preceding siblings ...)
  2023-01-02 21:35 ` [PATCH 4/6] hw/isa/piix: Avoid Xen-specific variant of piix_write_config() Bernhard Beschow
@ 2023-01-02 21:35 ` Bernhard Beschow
  2023-01-03 13:20   ` Philippe Mathieu-Daudé
  2023-01-02 21:35 ` [PATCH 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
  2023-01-03  3:15 ` [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Chuck Zmudzinski
  6 siblings, 1 reply; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

The previous patch unified handling of piix_write_config() accross all
PIIX device models which allows for assigning k->config_write once in the
base class.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/piix.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index d4cdb3dadb..98e9b12661 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -396,6 +396,7 @@ static void pci_piix_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+    k->config_write = piix_write_config;
     dc->reset       = piix_reset;
     dc->desc        = "ISA bridge";
     dc->hotpluggable   = false;
@@ -451,7 +452,6 @@ static void piix3_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix_write_config;
     k->realize = piix3_realize;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -470,7 +470,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix_write_config;
     k->realize = piix3_realize;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -519,7 +518,6 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->config_write = piix_write_config;
     k->realize = piix4_realize;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     dc->vmsd = &vmstate_piix4;
-- 
2.39.0



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

* [PATCH 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
                   ` (4 preceding siblings ...)
  2023-01-02 21:35 ` [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments Bernhard Beschow
@ 2023-01-02 21:35 ` Bernhard Beschow
  2023-01-03  3:15 ` [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Chuck Zmudzinski
  6 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-02 21:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson,
	Bernhard Beschow

During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of
TYPE_PIIX3_DEVICE. Remove this redundancy.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c             |  4 +---
 hw/isa/piix.c                 | 20 --------------------
 include/hw/southbridge/piix.h |  1 -
 3 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7ef0054b3a..76d98183ac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -235,8 +235,6 @@ static void pc_init1(MachineState *machine,
     if (pcmc->pci_enabled) {
         DeviceState *dev;
         PCIDevice *pci_dev;
-        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
-                                         : TYPE_PIIX3_DEVICE;
         int i;
 
         pci_bus = i440fx_init(pci_type,
@@ -250,7 +248,7 @@ static void pc_init1(MachineState *machine,
                                        : pci_slot_get_pirq);
         pcms->bus = pci_bus;
 
-        pci_dev = pci_new_multifunction(-1, true, type);
+        pci_dev = pci_new_multifunction(-1, true, TYPE_PIIX3_DEVICE);
         object_property_set_bool(OBJECT(pci_dev), "has-usb",
                                  machine_usb(machine), &error_abort);
         object_property_set_bool(OBJECT(pci_dev), "has-acpi",
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 98e9b12661..e4587352c9 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -33,7 +33,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/ide/piix.h"
 #include "hw/isa/isa.h"
-#include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
@@ -465,24 +464,6 @@ static const TypeInfo piix3_info = {
     .class_init    = piix3_class_init,
 };
 
-static void piix3_xen_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = piix3_realize;
-    /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
-    k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
-    dc->vmsd = &vmstate_piix3;
-}
-
-static const TypeInfo piix3_xen_info = {
-    .name          = TYPE_PIIX3_XEN_DEVICE,
-    .parent        = TYPE_PIIX_PCI_DEVICE,
-    .instance_init = piix3_init,
-    .class_init    = piix3_xen_class_init,
-};
-
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
     ERRP_GUARD();
@@ -534,7 +515,6 @@ static void piix3_register_types(void)
 {
     type_register_static(&piix_pci_type_info);
     type_register_static(&piix3_info);
-    type_register_static(&piix3_xen_info);
     type_register_static(&piix4_info);
 }
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 65ad8569da..b1fc94a742 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -77,7 +77,6 @@ struct PIIXState {
 OBJECT_DECLARE_SIMPLE_TYPE(PIIXState, PIIX_PCI_DEVICE)
 
 #define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
 #endif
-- 
2.39.0



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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
                   ` (5 preceding siblings ...)
  2023-01-02 21:35 ` [PATCH 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
@ 2023-01-03  3:15 ` Chuck Zmudzinski
  2023-01-03 13:17   ` Philippe Mathieu-Daudé
  6 siblings, 1 reply; 18+ messages in thread
From: Chuck Zmudzinski @ 2023-01-03  3:15 UTC (permalink / raw)
  To: Bernhard Beschow, qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Philippe Mathieu-Daudé, Eduardo Habkost, xen-devel,
	Hervé Poussineau, Paolo Bonzini, Richard Henderson

On 1/2/23 4:34 PM, Bernhard Beschow wrote:
> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
> machine agnostic to the precise southbridge being used. 2/ will become
> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>
> Testing done:
> None, because I don't know how to conduct this properly :(
>
> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>           "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
> Bernhard Beschow (6):
>   include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>   hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>   hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>   hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>   hw/isa/piix: Resolve redundant k->config_write assignments
>   hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>
>  hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>  hw/i386/xen/xen-hvm.c         |  9 +++--
>  hw/isa/piix.c                 | 66 +----------------------------------

This file does not exist on the Qemu master branch.
But hw/isa/piix3.c and hw/isa/piix4.c do exist.

I tried renaming it from piix.c to piix3.c in the patch, but
the patch set still does not apply cleanly on my tree.

Is this patch set re-based against something other than
the current master Qemu branch?

I have a system that is suitable for testing this patch set, but
I need guidance on how to apply it to the Qemu source tree.

Thanks,

Chuck Zmudzinski

>  include/hw/southbridge/piix.h |  1 -
>  include/hw/xen/xen.h          |  2 +-
>  stubs/xen-hw-stub.c           |  2 +-
>  6 files changed, 40 insertions(+), 74 deletions(-)
>



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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-03  3:15 ` [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Chuck Zmudzinski
@ 2023-01-03 13:17   ` Philippe Mathieu-Daudé
  2023-01-03 13:38     ` Bernhard Beschow
  0 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-03 13:17 UTC (permalink / raw)
  To: Chuck Zmudzinski, Bernhard Beschow, qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

Hi Chuck,

On 3/1/23 04:15, Chuck Zmudzinski wrote:
> On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>> machine agnostic to the precise southbridge being used. 2/ will become
>> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>>
>> Testing done:
>> None, because I don't know how to conduct this properly :(
>>
>> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>>            "[PATCH v4 00/30] Consolidate PIIX south bridges"

This series is based on a previous series:
https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
(which itself also is).

>> Bernhard Beschow (6):
>>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>>    hw/isa/piix: Resolve redundant k->config_write assignments
>>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>>
>>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>>   hw/i386/xen/xen-hvm.c         |  9 +++--
>>   hw/isa/piix.c                 | 66 +----------------------------------
> 
> This file does not exist on the Qemu master branch.
> But hw/isa/piix3.c and hw/isa/piix4.c do exist.
> 
> I tried renaming it from piix.c to piix3.c in the patch, but
> the patch set still does not apply cleanly on my tree.
> 
> Is this patch set re-based against something other than
> the current master Qemu branch?
> 
> I have a system that is suitable for testing this patch set, but
> I need guidance on how to apply it to the Qemu source tree.

You can ask Bernhard to publish a branch with the full work,
or apply each series locally. I use the b4 tool for that:
https://b4.docs.kernel.org/en/latest/installing.html

i.e.:

$ git checkout -b shentey_work
$ b4 am 20221120150550.63059-1-shentey@gmail.com
$ git am 
./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
$ b4 am 20221221170003.2929-1-shentey@gmail.com
$ git am 
./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
$ b4 am 20230102213504.14646-1-shentey@gmail.com
$ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx

Now the branch 'shentey_work' contains all the patches and you can test.

Regards,

Phil.


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

* Re: [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments
  2023-01-02 21:35 ` [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments Bernhard Beschow
@ 2023-01-03 13:20   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-03 13:20 UTC (permalink / raw)
  To: Bernhard Beschow, qemu-devel
  Cc: Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

On 2/1/23 22:35, Bernhard Beschow wrote:
> The previous patch unified handling of piix_write_config() accross all
> PIIX device models which allows for assigning k->config_write once in the
> base class.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/piix.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-03 13:17   ` Philippe Mathieu-Daudé
@ 2023-01-03 13:38     ` Bernhard Beschow
  2023-01-03 17:25       ` Chuck Zmudzinski
  2023-01-04  8:18       ` Chuck Zmudzinski
  0 siblings, 2 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-03 13:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Chuck Zmudzinski, qemu-devel, Paul Durrant, Marcel Apfelbaum,
	Michael S. Tsirkin, Stefano Stabellini, Anthony Perard,
	Aurelien Jarno, Eduardo Habkost, xen-devel, Hervé Poussineau,
	Paolo Bonzini, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi Chuck,
>
> On 3/1/23 04:15, Chuck Zmudzinski wrote:
> > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
> >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally
> removes
> >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen
> in the PC
> >> machine agnostic to the precise southbridge being used. 2/ will become
> >> particularily interesting once PIIX4 becomes usable in the PC machine,
> avoiding
> >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> >>
> >> Testing done:
> >> None, because I don't know how to conduct this properly :(
> >>
> >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
> >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
> This series is based on a previous series:
> https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
> (which itself also is).
>
> >> Bernhard Beschow (6):
> >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
> >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
> >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
> >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
> >>    hw/isa/piix: Resolve redundant k->config_write assignments
> >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
> >>
> >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
> >>   hw/i386/xen/xen-hvm.c         |  9 +++--
> >>   hw/isa/piix.c                 | 66 +----------------------------------
> >
> > This file does not exist on the Qemu master branch.
> > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
> >
> > I tried renaming it from piix.c to piix3.c in the patch, but
> > the patch set still does not apply cleanly on my tree.
> >
> > Is this patch set re-based against something other than
> > the current master Qemu branch?
> >
> > I have a system that is suitable for testing this patch set, but
> > I need guidance on how to apply it to the Qemu source tree.
>
> You can ask Bernhard to publish a branch with the full work,
>

Hi Chuck,

... or just visit
https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There
you'll find a git tag with a complete history and all instructions!

Thanks for giving my series a test ride!

Best regards,
Bernhard

or apply each series locally. I use the b4 tool for that:
> https://b4.docs.kernel.org/en/latest/installing.html
>
> i.e.:
>
> $ git checkout -b shentey_work
> $ b4 am 20221120150550.63059-1-shentey@gmail.com
> $ git am
> ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
> $ b4 am 20221221170003.2929-1-shentey@gmail.com
> $ git am
>
> ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
> $ b4 am 20230102213504.14646-1-shentey@gmail.com
> $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>
> Now the branch 'shentey_work' contains all the patches and you can test.
>
> Regards,
>
> Phil.
>

[-- Attachment #2: Type: text/html, Size: 4668 bytes --]

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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-03 13:38     ` Bernhard Beschow
@ 2023-01-03 17:25       ` Chuck Zmudzinski
  2023-01-03 23:12         ` Bernhard Beschow
  2023-01-04  8:18       ` Chuck Zmudzinski
  1 sibling, 1 reply; 18+ messages in thread
From: Chuck Zmudzinski @ 2023-01-03 17:25 UTC (permalink / raw)
  To: Bernhard Beschow, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>
>
> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
>     Hi Chuck,
>
>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>     >> machine agnostic to the precise southbridge being used. 2/ will become
>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>     >>
>     >> Testing done:
>     >> None, because I don't know how to conduct this properly :(
>     >>
>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
>     This series is based on a previous series:
>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>     (which itself also is).
>
>     >> Bernhard Beschow (6):
>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>     >>
>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>     >
>     > This file does not exist on the Qemu master branch.
>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>     >
>     > I tried renaming it from piix.c to piix3.c in the patch, but
>     > the patch set still does not apply cleanly on my tree.
>     >
>     > Is this patch set re-based against something other than
>     > the current master Qemu branch?
>     >
>     > I have a system that is suitable for testing this patch set, but
>     > I need guidance on how to apply it to the Qemu source tree.
>
>     You can ask Bernhard to publish a branch with the full work,
>
>
> Hi Chuck,
>
> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>
> Thanks for giving my series a test ride!
>
> Best regards,
> Bernhard
>
>     or apply each series locally. I use the b4 tool for that:
>     https://b4.docs.kernel.org/en/latest/installing.html
>
>     i.e.:
>
>     $ git checkout -b shentey_work
>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>     $ git am
>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>     $ git am
>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>
>     Now the branch 'shentey_work' contains all the patches and you can test.
>
>     Regards,
>
>     Phil.
>

OK, I didn't see the "Consolidate PIIX south bridges" series is a
prerequisite.

I will try it - it may take a couple of days because I need to test both
patch series in my environment and I can only work on this in my spare
time.

I will provide Tested-by tags to both series if successful. Otherwise,
I will reply with an explanation of any problems.

Chuck


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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-03 17:25       ` Chuck Zmudzinski
@ 2023-01-03 23:12         ` Bernhard Beschow
  0 siblings, 0 replies; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-03 23:12 UTC (permalink / raw)
  To: Chuck Zmudzinski, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson



Am 3. Januar 2023 17:25:35 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>>
>>
>> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>>     Hi Chuck,
>>
>>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>>     >> machine agnostic to the precise southbridge being used. 2/ will become
>>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>>     >>
>>     >> Testing done:
>>     >> None, because I don't know how to conduct this properly :(
>>     >>
>>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>>
>>     This series is based on a previous series:
>>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>>     (which itself also is).
>>
>>     >> Bernhard Beschow (6):
>>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>>     >>
>>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>>     >
>>     > This file does not exist on the Qemu master branch.
>>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>>     >
>>     > I tried renaming it from piix.c to piix3.c in the patch, but
>>     > the patch set still does not apply cleanly on my tree.
>>     >
>>     > Is this patch set re-based against something other than
>>     > the current master Qemu branch?
>>     >
>>     > I have a system that is suitable for testing this patch set, but
>>     > I need guidance on how to apply it to the Qemu source tree.
>>
>>     You can ask Bernhard to publish a branch with the full work,
>>
>>
>> Hi Chuck,
>>
>> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>>
>> Thanks for giving my series a test ride!
>>
>> Best regards,
>> Bernhard
>>
>>     or apply each series locally. I use the b4 tool for that:
>>     https://b4.docs.kernel.org/en/latest/installing.html
>>
>>     i.e.:
>>
>>     $ git checkout -b shentey_work
>>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>>     $ git am
>>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>>     $ git am
>>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>>
>>     Now the branch 'shentey_work' contains all the patches and you can test.
>>
>>     Regards,
>>
>>     Phil.
>>
>
>OK, I didn't see the "Consolidate PIIX south bridges" series is a
>prerequisite.
>
>I will try it - it may take a couple of days because I need to test both
>patch series in my environment and I can only work on this in my spare
>time.
>
>I will provide Tested-by tags to both series if successful. Otherwise,
>I will reply with an explanation of any problems.

Sounds good! You don't need to test the prerequisite though since it is already reviewed. It would be completely sufficient if you could test this series to fill in the gap for my limited Xen knowledge -- thanks!

Best regards,
Bernhard

>
>Chuck


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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-03 13:38     ` Bernhard Beschow
  2023-01-03 17:25       ` Chuck Zmudzinski
@ 2023-01-04  8:18       ` Chuck Zmudzinski
  2023-01-04 12:13         ` Bernhard Beschow
  1 sibling, 1 reply; 18+ messages in thread
From: Chuck Zmudzinski @ 2023-01-04  8:18 UTC (permalink / raw)
  To: Bernhard Beschow, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>
>
> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
>     Hi Chuck,
>
>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>     >> machine agnostic to the precise southbridge being used. 2/ will become
>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>     >>
>     >> Testing done:
>     >> None, because I don't know how to conduct this properly :(
>     >>
>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
>     This series is based on a previous series:
>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>     (which itself also is).
>
>     >> Bernhard Beschow (6):
>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>     >>
>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>     >
>     > This file does not exist on the Qemu master branch.
>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>     >
>     > I tried renaming it from piix.c to piix3.c in the patch, but
>     > the patch set still does not apply cleanly on my tree.
>     >
>     > Is this patch set re-based against something other than
>     > the current master Qemu branch?
>     >
>     > I have a system that is suitable for testing this patch set, but
>     > I need guidance on how to apply it to the Qemu source tree.
>
>     You can ask Bernhard to publish a branch with the full work,
>
>
> Hi Chuck,
>
> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>
> Thanks for giving my series a test ride!
>
> Best regards,
> Bernhard
>
>     or apply each series locally. I use the b4 tool for that:
>     https://b4.docs.kernel.org/en/latest/installing.html
>
>     i.e.:
>
>     $ git checkout -b shentey_work
>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>     $ git am
>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>     $ git am
>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>
>     Now the branch 'shentey_work' contains all the patches and you can test.
>
>     Regards,
>
>     Phil.
>

Hi Phil and Bernard,

I tried applying these 3 patch series on top of the current qemu
master branch.

Unfortunately, I saw a regression, so I can't give a tested-by tag yet.

Here are the details of the testing I did so far:

Xen only needs one target, the i386-softmmu target which creates
the qemu-system-i386 binary that Xen uses for its device model.
That target compiled and linked with no problems with these 3
patch series applied on top of qemu master. I didn't try building
any other targets.

My tests used the xenfv machine type with the xen platform
pci device, which is ordinarily called a xen hvm guest with xen
paravirtualized network and block device drivers. It is based on the
i440fx machine type and so emulates piix3. I tested the xen
hvm guests with two different configurations as described below.

I tested both Linux and Windows guests, with mixed results. With the
current Qemu master (commit 222059a0fccf4 without the 3 patch
series applied), all tested guest configurations work normally for both
Linux and Windows guests.

With these 3 patch series applied on top of the qemu master branch,
which tries to consolidate piix3 and piix4 and resolve the xen piix3
device that my guests use, I unfortunately got a regression.

The regression occurred with a configuration that uses the qemu
bochs stdvga graphics device with a vnc display, and the qemu
usb-tablet device to emulate the mouse and keyboard. After applying
the 3 patch series, the emulated mouse is not working at all for Linux
guests. It works for Windows guests, but the mouse pointer in the
guest does not follow the mouse pointer in the vnc window as closely
as it does without the 3 patch series. So this is the bad news of a
regression introduced somewhere in these 3 patch series.

The good news is that by using guests in a configuration that does
not use the qemu usb-tablet device or the bochs stdvga device but
instead uses a real passed through usb3 controller with a real usb
mouse and a real usb keyboard connected, and also the real sound
card and vga device passed through and a 1920x1080 HDMI monitor,
there is no regression introduced by the 3 patch series and both Linux
and Windows guests in that configuration work perfectly.

My next test will be to test Bernhard's published git tag without
trying to merge the 3 patch series into master and see if that also
has the regression. I also will double check that I didn't make
any mistakes in merging the 3 patch series by creating the shentey_work
branch with b4 and git as Phil described and compare that to my
working tree.

I also will try testing only the first series, then the first series and the
second series, to try to determine in which of the 3 series the regression
is introduced.

Best regards,

Chuck


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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-04  8:18       ` Chuck Zmudzinski
@ 2023-01-04 12:13         ` Bernhard Beschow
  2023-01-04 13:11           ` Chuck Zmudzinski
  0 siblings, 1 reply; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-04 12:13 UTC (permalink / raw)
  To: Chuck Zmudzinski, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson



Am 4. Januar 2023 08:18:59 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>>
>>
>> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>>     Hi Chuck,
>>
>>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>>     >> machine agnostic to the precise southbridge being used. 2/ will become
>>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>>     >>
>>     >> Testing done:
>>     >> None, because I don't know how to conduct this properly :(
>>     >>
>>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>>
>>     This series is based on a previous series:
>>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>>     (which itself also is).
>>
>>     >> Bernhard Beschow (6):
>>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>>     >>
>>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>>     >
>>     > This file does not exist on the Qemu master branch.
>>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>>     >
>>     > I tried renaming it from piix.c to piix3.c in the patch, but
>>     > the patch set still does not apply cleanly on my tree.
>>     >
>>     > Is this patch set re-based against something other than
>>     > the current master Qemu branch?
>>     >
>>     > I have a system that is suitable for testing this patch set, but
>>     > I need guidance on how to apply it to the Qemu source tree.
>>
>>     You can ask Bernhard to publish a branch with the full work,
>>
>>
>> Hi Chuck,
>>
>> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>>
>> Thanks for giving my series a test ride!
>>
>> Best regards,
>> Bernhard
>>
>>     or apply each series locally. I use the b4 tool for that:
>>     https://b4.docs.kernel.org/en/latest/installing.html
>>
>>     i.e.:
>>
>>     $ git checkout -b shentey_work
>>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>>     $ git am
>>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>>     $ git am
>>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>>
>>     Now the branch 'shentey_work' contains all the patches and you can test.
>>
>>     Regards,
>>
>>     Phil.
>>
>
>Hi Phil and Bernard,
>
>I tried applying these 3 patch series on top of the current qemu
>master branch.
>
>Unfortunately, I saw a regression, so I can't give a tested-by tag yet.

Hi Chuck,

Thanks for your valuable test report! I think the culprit may be commit https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00102.html where now 128 PIRQs are considered rather than four. I'll revisit my series and will prepare a v2 in the next days. I think there is no need for further testing v1.

Thanks,
Bernhard

>
>Here are the details of the testing I did so far:
>
>Xen only needs one target, the i386-softmmu target which creates
>the qemu-system-i386 binary that Xen uses for its device model.
>That target compiled and linked with no problems with these 3
>patch series applied on top of qemu master. I didn't try building
>any other targets.
>
>My tests used the xenfv machine type with the xen platform
>pci device, which is ordinarily called a xen hvm guest with xen
>paravirtualized network and block device drivers. It is based on the
>i440fx machine type and so emulates piix3. I tested the xen
>hvm guests with two different configurations as described below.
>
>I tested both Linux and Windows guests, with mixed results. With the
>current Qemu master (commit 222059a0fccf4 without the 3 patch
>series applied), all tested guest configurations work normally for both
>Linux and Windows guests.
>
>With these 3 patch series applied on top of the qemu master branch,
>which tries to consolidate piix3 and piix4 and resolve the xen piix3
>device that my guests use, I unfortunately got a regression.
>
>The regression occurred with a configuration that uses the qemu
>bochs stdvga graphics device with a vnc display, and the qemu
>usb-tablet device to emulate the mouse and keyboard. After applying
>the 3 patch series, the emulated mouse is not working at all for Linux
>guests. It works for Windows guests, but the mouse pointer in the
>guest does not follow the mouse pointer in the vnc window as closely
>as it does without the 3 patch series. So this is the bad news of a
>regression introduced somewhere in these 3 patch series.
>
>The good news is that by using guests in a configuration that does
>not use the qemu usb-tablet device or the bochs stdvga device but
>instead uses a real passed through usb3 controller with a real usb
>mouse and a real usb keyboard connected, and also the real sound
>card and vga device passed through and a 1920x1080 HDMI monitor,
>there is no regression introduced by the 3 patch series and both Linux
>and Windows guests in that configuration work perfectly.
>
>My next test will be to test Bernhard's published git tag without
>trying to merge the 3 patch series into master and see if that also
>has the regression. I also will double check that I didn't make
>any mistakes in merging the 3 patch series by creating the shentey_work
>branch with b4 and git as Phil described and compare that to my
>working tree.
>
>I also will try testing only the first series, then the first series and the
>second series, to try to determine in which of the 3 series the regression
>is introduced.
>
>Best regards,
>
>Chuck


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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-04 12:13         ` Bernhard Beschow
@ 2023-01-04 13:11           ` Chuck Zmudzinski
  2023-01-04 16:12             ` Bernhard Beschow
  0 siblings, 1 reply; 18+ messages in thread
From: Chuck Zmudzinski @ 2023-01-04 13:11 UTC (permalink / raw)
  To: Bernhard Beschow, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

On 1/4/2023 7:13 AM, Bernhard Beschow wrote:
> Am 4. Januar 2023 08:18:59 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
> >On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
> >>
> >>
> >> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >>     Hi Chuck,
> >>
> >>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
> >>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
> >>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
> >>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
> >>     >> machine agnostic to the precise southbridge being used. 2/ will become
> >>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
> >>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> >>     >>
> >>     >> Testing done:
> >>     >> None, because I don't know how to conduct this properly :(
> >>     >>
> >>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
> >>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
> >>
> >>     This series is based on a previous series:
> >>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
> >>     (which itself also is).
> >>
> >>     >> Bernhard Beschow (6):
> >>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
> >>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
> >>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
> >>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
> >>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
> >>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
> >>     >>
> >>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
> >>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
> >>     >>   hw/isa/piix.c                 | 66 +----------------------------------
> >>     >
> >>     > This file does not exist on the Qemu master branch.
> >>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
> >>     >
> >>     > I tried renaming it from piix.c to piix3.c in the patch, but
> >>     > the patch set still does not apply cleanly on my tree.
> >>     >
> >>     > Is this patch set re-based against something other than
> >>     > the current master Qemu branch?
> >>     >
> >>     > I have a system that is suitable for testing this patch set, but
> >>     > I need guidance on how to apply it to the Qemu source tree.
> >>
> >>     You can ask Bernhard to publish a branch with the full work,
> >>
> >>
> >> Hi Chuck,
> >>
> >> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
> >>
> >> Thanks for giving my series a test ride!
> >>
> >> Best regards,
> >> Bernhard
> >>
> >>     or apply each series locally. I use the b4 tool for that:
> >>     https://b4.docs.kernel.org/en/latest/installing.html
> >>
> >>     i.e.:
> >>
> >>     $ git checkout -b shentey_work
> >>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
> >>     $ git am
> >>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
> >>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
> >>     $ git am
> >>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
> >>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
> >>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
> >>
> >>     Now the branch 'shentey_work' contains all the patches and you can test.
> >>
> >>     Regards,
> >>
> >>     Phil.
> >>
> >
> >Hi Phil and Bernard,
> >
> >I tried applying these 3 patch series on top of the current qemu
> >master branch.
> >
> >Unfortunately, I saw a regression, so I can't give a tested-by tag yet.
>
> Hi Chuck,
>
> Thanks for your valuable test report! I think the culprit may be commit https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00102.html where now 128 PIRQs are considered rather than four. I'll revisit my series and will prepare a v2 in the next days. I think there is no need for further testing v1.
>
> Thanks,
> Bernhard

Hi Bernhard,

Thanks for letting me know I do not need to test v1 further. I agree the
symptoms are that it is an IRQ problem - it looks like IRQs associated with
the emulated usb tablet device are not making it to the guest with the
patched v1 piix device on xen.

I will be looking for your v2 in coming days and try it out also!

Best regards,

Chuck

>
> >
> >Here are the details of the testing I did so far:
> >
> >Xen only needs one target, the i386-softmmu target which creates
> >the qemu-system-i386 binary that Xen uses for its device model.
> >That target compiled and linked with no problems with these 3
> >patch series applied on top of qemu master. I didn't try building
> >any other targets.
> >
> >My tests used the xenfv machine type with the xen platform
> >pci device, which is ordinarily called a xen hvm guest with xen
> >paravirtualized network and block device drivers. It is based on the
> >i440fx machine type and so emulates piix3. I tested the xen
> >hvm guests with two different configurations as described below.
> >
> >I tested both Linux and Windows guests, with mixed results. With the
> >current Qemu master (commit 222059a0fccf4 without the 3 patch
> >series applied), all tested guest configurations work normally for both
> >Linux and Windows guests.
> >
> >With these 3 patch series applied on top of the qemu master branch,
> >which tries to consolidate piix3 and piix4 and resolve the xen piix3
> >device that my guests use, I unfortunately got a regression.
> >
> >The regression occurred with a configuration that uses the qemu
> >bochs stdvga graphics device with a vnc display, and the qemu
> >usb-tablet device to emulate the mouse and keyboard. After applying
> >the 3 patch series, the emulated mouse is not working at all for Linux
> >guests. It works for Windows guests, but the mouse pointer in the
> >guest does not follow the mouse pointer in the vnc window as closely
> >as it does without the 3 patch series. So this is the bad news of a
> >regression introduced somewhere in these 3 patch series.
> >
> >The good news is that by using guests in a configuration that does
> >not use the qemu usb-tablet device or the bochs stdvga device but
> >instead uses a real passed through usb3 controller with a real usb
> >mouse and a real usb keyboard connected, and also the real sound
> >card and vga device passed through and a 1920x1080 HDMI monitor,
> >there is no regression introduced by the 3 patch series and both Linux
> >and Windows guests in that configuration work perfectly.
> >
> >My next test will be to test Bernhard's published git tag without
> >trying to merge the 3 patch series into master and see if that also
> >has the regression. I also will double check that I didn't make
> >any mistakes in merging the 3 patch series by creating the shentey_work
> >branch with b4 and git as Phil described and compare that to my
> >working tree.
> >
> >I also will try testing only the first series, then the first series and the
> >second series, to try to determine in which of the 3 series the regression
> >is introduced.
> >
> >Best regards,
> >
> >Chuck



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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-04 13:11           ` Chuck Zmudzinski
@ 2023-01-04 16:12             ` Bernhard Beschow
  2023-01-04 16:49               ` Chuck Zmudzinski
  0 siblings, 1 reply; 18+ messages in thread
From: Bernhard Beschow @ 2023-01-04 16:12 UTC (permalink / raw)
  To: Chuck Zmudzinski, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson



Am 4. Januar 2023 13:11:16 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>On 1/4/2023 7:13 AM, Bernhard Beschow wrote:
>> Am 4. Januar 2023 08:18:59 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>> >On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>> >>
>> >>
>> >> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> >>
>> >>     Hi Chuck,
>> >>
>> >>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>> >>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>> >>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>> >>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>> >>     >> machine agnostic to the precise southbridge being used. 2/ will become
>> >>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>> >>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>> >>     >>
>> >>     >> Testing done:
>> >>     >> None, because I don't know how to conduct this properly :(
>> >>     >>
>> >>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>> >>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>> >>
>> >>     This series is based on a previous series:
>> >>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>> >>     (which itself also is).
>> >>
>> >>     >> Bernhard Beschow (6):
>> >>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>> >>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>> >>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>> >>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>> >>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>> >>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>> >>     >>
>> >>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>> >>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>> >>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>> >>     >
>> >>     > This file does not exist on the Qemu master branch.
>> >>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>> >>     >
>> >>     > I tried renaming it from piix.c to piix3.c in the patch, but
>> >>     > the patch set still does not apply cleanly on my tree.
>> >>     >
>> >>     > Is this patch set re-based against something other than
>> >>     > the current master Qemu branch?
>> >>     >
>> >>     > I have a system that is suitable for testing this patch set, but
>> >>     > I need guidance on how to apply it to the Qemu source tree.
>> >>
>> >>     You can ask Bernhard to publish a branch with the full work,
>> >>
>> >>
>> >> Hi Chuck,
>> >>
>> >> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>> >>
>> >> Thanks for giving my series a test ride!
>> >>
>> >> Best regards,
>> >> Bernhard
>> >>
>> >>     or apply each series locally. I use the b4 tool for that:
>> >>     https://b4.docs.kernel.org/en/latest/installing.html
>> >>
>> >>     i.e.:
>> >>
>> >>     $ git checkout -b shentey_work
>> >>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>> >>     $ git am
>> >>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>> >>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>> >>     $ git am
>> >>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>> >>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>> >>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>> >>
>> >>     Now the branch 'shentey_work' contains all the patches and you can test.
>> >>
>> >>     Regards,
>> >>
>> >>     Phil.
>> >>
>> >
>> >Hi Phil and Bernard,
>> >
>> >I tried applying these 3 patch series on top of the current qemu
>> >master branch.
>> >
>> >Unfortunately, I saw a regression, so I can't give a tested-by tag yet.
>>
>> Hi Chuck,
>>
>> Thanks for your valuable test report! I think the culprit may be commit https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00102.html where now 128 PIRQs are considered rather than four. I'll revisit my series and will prepare a v2 in the next days. I think there is no need for further testing v1.
>>
>> Thanks,
>> Bernhard
>
>Hi Bernhard,
>
>Thanks for letting me know I do not need to test v1 further. I agree the
>symptoms are that it is an IRQ problem - it looks like IRQs associated with
>the emulated usb tablet device are not making it to the guest with the
>patched v1 piix device on xen.

All PCI IRQs were routed to PCI slot 0. This should be fixed in v2 now.

>I will be looking for your v2 in coming days and try it out also!

Thank you! Here it is: https://patchew.org/QEMU/20230104144437.27479-1-shentey@gmail.com/

Best regards,
Bernhard

>
>Best regards,
>
>Chuck
>
>>
>> >
>> >Here are the details of the testing I did so far:
>> >
>> >Xen only needs one target, the i386-softmmu target which creates
>> >the qemu-system-i386 binary that Xen uses for its device model.
>> >That target compiled and linked with no problems with these 3
>> >patch series applied on top of qemu master. I didn't try building
>> >any other targets.
>> >
>> >My tests used the xenfv machine type with the xen platform
>> >pci device, which is ordinarily called a xen hvm guest with xen
>> >paravirtualized network and block device drivers. It is based on the
>> >i440fx machine type and so emulates piix3. I tested the xen
>> >hvm guests with two different configurations as described below.
>> >
>> >I tested both Linux and Windows guests, with mixed results. With the
>> >current Qemu master (commit 222059a0fccf4 without the 3 patch
>> >series applied), all tested guest configurations work normally for both
>> >Linux and Windows guests.
>> >
>> >With these 3 patch series applied on top of the qemu master branch,
>> >which tries to consolidate piix3 and piix4 and resolve the xen piix3
>> >device that my guests use, I unfortunately got a regression.
>> >
>> >The regression occurred with a configuration that uses the qemu
>> >bochs stdvga graphics device with a vnc display, and the qemu
>> >usb-tablet device to emulate the mouse and keyboard. After applying
>> >the 3 patch series, the emulated mouse is not working at all for Linux
>> >guests. It works for Windows guests, but the mouse pointer in the
>> >guest does not follow the mouse pointer in the vnc window as closely
>> >as it does without the 3 patch series. So this is the bad news of a
>> >regression introduced somewhere in these 3 patch series.
>> >
>> >The good news is that by using guests in a configuration that does
>> >not use the qemu usb-tablet device or the bochs stdvga device but
>> >instead uses a real passed through usb3 controller with a real usb
>> >mouse and a real usb keyboard connected, and also the real sound
>> >card and vga device passed through and a 1920x1080 HDMI monitor,
>> >there is no regression introduced by the 3 patch series and both Linux
>> >and Windows guests in that configuration work perfectly.
>> >
>> >My next test will be to test Bernhard's published git tag without
>> >trying to merge the 3 patch series into master and see if that also
>> >has the regression. I also will double check that I didn't make
>> >any mistakes in merging the 3 patch series by creating the shentey_work
>> >branch with b4 and git as Phil described and compare that to my
>> >working tree.
>> >
>> >I also will try testing only the first series, then the first series and the
>> >second series, to try to determine in which of the 3 series the regression
>> >is introduced.
>> >
>> >Best regards,
>> >
>> >Chuck
>


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

* Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE
  2023-01-04 16:12             ` Bernhard Beschow
@ 2023-01-04 16:49               ` Chuck Zmudzinski
  0 siblings, 0 replies; 18+ messages in thread
From: Chuck Zmudzinski @ 2023-01-04 16:49 UTC (permalink / raw)
  To: Bernhard Beschow, Philippe Mathieu-Daudé
  Cc: qemu-devel, Paul Durrant, Marcel Apfelbaum, Michael S. Tsirkin,
	Stefano Stabellini, Anthony Perard, Aurelien Jarno,
	Eduardo Habkost, xen-devel, Hervé Poussineau, Paolo Bonzini,
	Richard Henderson

On 1/4/23 11:12 AM, Bernhard Beschow wrote:
> 
> 
> Am 4. Januar 2023 13:11:16 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>>On 1/4/2023 7:13 AM, Bernhard Beschow wrote:
>>> Am 4. Januar 2023 08:18:59 UTC schrieb Chuck Zmudzinski <brchuckz@aol.com>:
>>> >On 1/3/2023 8:38 AM, Bernhard Beschow wrote:
>>> >>
>>> >>
>>> >> On Tue, Jan 3, 2023 at 2:17 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>> >>
>>> >>     Hi Chuck,
>>> >>
>>> >>     On 3/1/23 04:15, Chuck Zmudzinski wrote:
>>> >>     > On 1/2/23 4:34 PM, Bernhard Beschow wrote:
>>> >>     >> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
>>> >>     >> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
>>> >>     >> machine agnostic to the precise southbridge being used. 2/ will become
>>> >>     >> particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
>>> >>     >> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>>> >>     >>
>>> >>     >> Testing done:
>>> >>     >> None, because I don't know how to conduct this properly :(
>>> >>     >>
>>> >>     >> Based-on: <20221221170003.2929-1-shentey@gmail.com>
>>> >>     >>            "[PATCH v4 00/30] Consolidate PIIX south bridges"
>>> >>
>>> >>     This series is based on a previous series:
>>> >>     https://lore.kernel.org/qemu-devel/20221221170003.2929-1-shentey@gmail.com/
>>> >>     (which itself also is).
>>> >>
>>> >>     >> Bernhard Beschow (6):
>>> >>     >>    include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>>> >>     >>    hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>>> >>     >>    hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>>> >>     >>    hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>>> >>     >>    hw/isa/piix: Resolve redundant k->config_write assignments
>>> >>     >>    hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>>> >>     >>
>>> >>     >>   hw/i386/pc_piix.c             | 34 ++++++++++++++++--
>>> >>     >>   hw/i386/xen/xen-hvm.c         |  9 +++--
>>> >>     >>   hw/isa/piix.c                 | 66 +----------------------------------
>>> >>     >
>>> >>     > This file does not exist on the Qemu master branch.
>>> >>     > But hw/isa/piix3.c and hw/isa/piix4.c do exist.
>>> >>     >
>>> >>     > I tried renaming it from piix.c to piix3.c in the patch, but
>>> >>     > the patch set still does not apply cleanly on my tree.
>>> >>     >
>>> >>     > Is this patch set re-based against something other than
>>> >>     > the current master Qemu branch?
>>> >>     >
>>> >>     > I have a system that is suitable for testing this patch set, but
>>> >>     > I need guidance on how to apply it to the Qemu source tree.
>>> >>
>>> >>     You can ask Bernhard to publish a branch with the full work,
>>> >>
>>> >>
>>> >> Hi Chuck,
>>> >>
>>> >> ... or just visit https://patchew.org/QEMU/20230102213504.14646-1-shentey@gmail.com/ . There you'll find a git tag with a complete history and all instructions!
>>> >>
>>> >> Thanks for giving my series a test ride!
>>> >>
>>> >> Best regards,
>>> >> Bernhard
>>> >>
>>> >>     or apply each series locally. I use the b4 tool for that:
>>> >>     https://b4.docs.kernel.org/en/latest/installing.html
>>> >>
>>> >>     i.e.:
>>> >>
>>> >>     $ git checkout -b shentey_work
>>> >>     $ b4 am 20221120150550.63059-1-shentey@gmail.com
>>> >>     $ git am
>>> >>     ./v2_20221120_shentey_decouple_intx_to_lnkx_routing_from_south_bridges.mbx
>>> >>     $ b4 am 20221221170003.2929-1-shentey@gmail.com
>>> >>     $ git am
>>> >>     ./v4_20221221_shentey_this_series_consolidates_the_implementations_of_the_piix3_and_piix4_south.mbx
>>> >>     $ b4 am 20230102213504.14646-1-shentey@gmail.com
>>> >>     $ git am ./20230102_shentey_resolve_type_piix3_xen_device.mbx
>>> >>
>>> >>     Now the branch 'shentey_work' contains all the patches and you can test.
>>> >>
>>> >>     Regards,
>>> >>
>>> >>     Phil.
>>> >>
>>> >
>>> >Hi Phil and Bernard,
>>> >
>>> >I tried applying these 3 patch series on top of the current qemu
>>> >master branch.
>>> >
>>> >Unfortunately, I saw a regression, so I can't give a tested-by tag yet.
>>>
>>> Hi Chuck,
>>>
>>> Thanks for your valuable test report! I think the culprit may be commit https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00102.html where now 128 PIRQs are considered rather than four. I'll revisit my series and will prepare a v2 in the next days. I think there is no need for further testing v1.
>>>
>>> Thanks,
>>> Bernhard
>>
>>Hi Bernhard,
>>
>>Thanks for letting me know I do not need to test v1 further. I agree the
>>symptoms are that it is an IRQ problem - it looks like IRQs associated with
>>the emulated usb tablet device are not making it to the guest with the
>>patched v1 piix device on xen.
> 
> All PCI IRQs were routed to PCI slot 0. This should be fixed in v2 now.
> 
>>I will be looking for your v2 in coming days and try it out also!
> 
> Thank you! Here it is: https://patchew.org/QEMU/20230104144437.27479-1-shentey@gmail.com/

That fixed it! I added my Tested-by tag to the last patch of v2:

[PATCH v2 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE

AFAICT, v2 is is ready to go!

Best regards,

Chuck


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

end of thread, other threads:[~2023-01-04 16:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-02 21:34 [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
2023-01-02 21:34 ` [PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it Bernhard Beschow
2023-01-02 21:35 ` [PATCH 2/6] hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize() Bernhard Beschow
2023-01-02 21:35 ` [PATCH 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3 Bernhard Beschow
2023-01-02 21:35 ` [PATCH 4/6] hw/isa/piix: Avoid Xen-specific variant of piix_write_config() Bernhard Beschow
2023-01-02 21:35 ` [PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments Bernhard Beschow
2023-01-03 13:20   ` Philippe Mathieu-Daudé
2023-01-02 21:35 ` [PATCH 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE Bernhard Beschow
2023-01-03  3:15 ` [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE Chuck Zmudzinski
2023-01-03 13:17   ` Philippe Mathieu-Daudé
2023-01-03 13:38     ` Bernhard Beschow
2023-01-03 17:25       ` Chuck Zmudzinski
2023-01-03 23:12         ` Bernhard Beschow
2023-01-04  8:18       ` Chuck Zmudzinski
2023-01-04 12:13         ` Bernhard Beschow
2023-01-04 13:11           ` Chuck Zmudzinski
2023-01-04 16:12             ` Bernhard Beschow
2023-01-04 16:49               ` Chuck Zmudzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).