* [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros
@ 2014-06-25 2:04 Eduardo Habkost
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Eduardo Habkost @ 2014-06-25 2:04 UTC (permalink / raw)
To: qemu-devel, Michael S. Tsirkin
Cc: Peter Maydell, Marcel Apfelbaum, Alexey Kardashevskiy,
Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
This series is an attempt to make the compat_props lists from the PC code
reusable by other machine-types. All the compat bits that are on those lists are
not tied to a specific machine-type, but instead to the device code that was
present on a given QEMU version.
This series is based on Michael's "pci" branch, and it also requires the
following patch to be applied first:
From: Eduardo Habkost <ehabkost@redhat.com>
Date: Tue, 24 Jun 2014 19:57:55 -0300
Message-Id: <1403650675-3368-1-git-send-email-ehabkost@redhat.com>
Subject: [PATCH] pc: Fix "prog_if" typo on PC_COMPAT_2_0
Changes v1 -> v2:
* Move QEMU_COMPAT_* to hw/compat.h
* Eliminate machine-type-specific PC_{Q35,I440FX}_COMPAT_* macros, as they
are not necessary today
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Marcel Apfelbaum <marcel.a@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Alexander Graf <agraf@suse.com>
Eduardo Habkost (2):
pc: Move q35 compat props to PC_COMPAT_*
machine: Introduce QEMU_COMPAT_* macros
hw/i386/pc_q35.c | 10 +--
include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 225 +++------------------------------------------------
3 files changed, 226 insertions(+), 219 deletions(-)
create mode 100644 include/hw/compat.h
--
1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_*
2014-06-25 2:04 [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-25 2:04 ` Eduardo Habkost
2014-06-25 5:53 ` Michael S. Tsirkin
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25 5:58 ` [Qemu-devel] [PATCH v2 0/2] Introduce common " Michael S. Tsirkin
2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2014-06-25 2:04 UTC (permalink / raw)
To: qemu-devel, Michael S. Tsirkin; +Cc: Peter Maydell, Liu Ping Fan
For each compat property on PC_Q35_COMPAT_*, there are only two
possibilities:
* If the device is never instantiated when using a machine other than
pc-q35, then the compat property can be safely added to
PC_COMPAT_*;
* If the device can be instantiated when using a machine other than
pc-q35, that means the other machines also need the compat property
to be set.
That means we don't need separate PC_Q35_COMPAT_* macros at all, today.
The hpet.hpet-intcap case is interesting: piix and q35 do have something
that emulates different defaults, but the machine-specific default is
applied _after_ compat_props are applied, by simply checking if the
property is zero (which is the real default on the hpet code).
The hpet.hpet-intcap=0x4 compat property can (should?) be applied to
piix too, because 0x4 was the default on both piix and q35 before the
hpet-intcap property was introduced.
Now, if one day we change the default HPET intcap on one of the PC
machine-types again, we may want to introduce PC_{Q35,I440FX}_COMPAT
macros. But while we don't need that, we can keep the code simple.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
hw/i386/pc_q35.c | 10 +++++-----
include/hw/i386/pc.h | 55 +++++++++++++++++-----------------------------------
2 files changed, 23 insertions(+), 42 deletions(-)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 155db99..36b6ab0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -361,7 +361,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
.name = "pc-q35-2.0",
.init = pc_q35_init_2_0,
.compat_props = (GlobalProperty[]) {
- PC_Q35_COMPAT_2_0,
+ PC_COMPAT_2_0,
{ /* end of list */ }
},
};
@@ -373,7 +373,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
.name = "pc-q35-1.7",
.init = pc_q35_init_1_7,
.compat_props = (GlobalProperty[]) {
- PC_Q35_COMPAT_1_7,
+ PC_COMPAT_1_7,
{ /* end of list */ }
},
};
@@ -385,7 +385,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
.name = "pc-q35-1.6",
.init = pc_q35_init_1_6,
.compat_props = (GlobalProperty[]) {
- PC_Q35_COMPAT_1_6,
+ PC_COMPAT_1_6,
{ /* end of list */ }
},
};
@@ -395,7 +395,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
.name = "pc-q35-1.5",
.init = pc_q35_init_1_5,
.compat_props = (GlobalProperty[]) {
- PC_Q35_COMPAT_1_5,
+ PC_COMPAT_1_5,
{ /* end of list */ }
},
};
@@ -409,7 +409,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
.name = "pc-q35-1.4",
.init = pc_q35_init_1_4,
.compat_props = (GlobalProperty[]) {
- PC_Q35_COMPAT_1_4,
+ PC_COMPAT_1_4,
{ /* end of list */ }
},
};
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1331d5a..1c0c382 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,43 +294,6 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
-#define PC_Q35_COMPAT_2_0 \
- PC_COMPAT_2_0, \
- {\
- .driver = "ICH9-LPC",\
- .property = "memory-hotplug-support",\
- .value = "off",\
- },{\
- .driver = "xio3130-downstream",\
- .property = COMPAT_PROP_PCP,\
- .value = "off",\
- },{\
- .driver = "ioh3420",\
- .property = COMPAT_PROP_PCP,\
- .value = "off",\
- }
-
-#define PC_Q35_COMPAT_1_7 \
- PC_COMPAT_1_7, \
- PC_Q35_COMPAT_2_0, \
- {\
- .driver = "hpet",\
- .property = HPET_INTCAP,\
- .value = stringify(4),\
- }
-
-#define PC_Q35_COMPAT_1_6 \
- PC_COMPAT_1_6, \
- PC_Q35_COMPAT_1_7
-
-#define PC_Q35_COMPAT_1_5 \
- PC_COMPAT_1_5, \
- PC_Q35_COMPAT_1_6
-
-#define PC_Q35_COMPAT_1_4 \
- PC_COMPAT_1_4, \
- PC_Q35_COMPAT_1_5
-
#define PC_COMPAT_2_0 \
{\
.driver = "virtio-scsi-pci",\
@@ -370,6 +333,19 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.driver = "virtio-net-pci",\
.property = "guest_announce",\
.value = "off",\
+ },\
+ {\
+ .driver = "ICH9-LPC",\
+ .property = "memory-hotplug-support",\
+ .value = "off",\
+ },{\
+ .driver = "xio3130-downstream",\
+ .property = COMPAT_PROP_PCP,\
+ .value = "off",\
+ },{\
+ .driver = "ioh3420",\
+ .property = COMPAT_PROP_PCP,\
+ .value = "off",\
}
#define PC_COMPAT_1_7 \
@@ -383,6 +359,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.driver = "PIIX4_PM",\
.property = "acpi-pci-hotplug-with-bridge-support",\
.value = "off",\
+ },\
+ {\
+ .driver = "hpet",\
+ .property = HPET_INTCAP,\
+ .value = stringify(4),\
}
#define PC_COMPAT_1_6 \
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 2:04 [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
@ 2014-06-25 2:04 ` Eduardo Habkost
2014-06-25 5:56 ` Michael S. Tsirkin
2014-06-25 5:58 ` [Qemu-devel] [PATCH v2 0/2] Introduce common " Michael S. Tsirkin
2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2014-06-25 2:04 UTC (permalink / raw)
To: qemu-devel, Michael S. Tsirkin; +Cc: Peter Maydell
The QEMU_COMPAT_* macros will contain compat properties that are not
specific to PC, and may be reused by other machine-types.
The compat properties for PC-specific devices were moved to
QEMU_COMPAT_* too, because they are simply not going to be applied to
any device if they are not instantiated.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 206 +++-----------------------------------------------
2 files changed, 221 insertions(+), 195 deletions(-)
create mode 100644 include/hw/compat.h
diff --git a/include/hw/compat.h b/include/hw/compat.h
new file mode 100644
index 0000000..dff55c8
--- /dev/null
+++ b/include/hw/compat.h
@@ -0,0 +1,210 @@
+/* Common compat properties that can be used by machine-types to keep guest ABI
+ * stable between QEMU versions.
+ */
+
+#ifndef HW_COMPAT_H
+#define HW_COMPAT_H
+
+#define QEMU_COMPAT_2_0 \
+ {\
+ .driver = "virtio-scsi-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = "PIIX4_PM",\
+ .property = "memory-hotplug-support",\
+ .value = "off",\
+ },\
+ {\
+ .driver = "apic",\
+ .property = "version",\
+ .value = stringify(0x11),\
+ },\
+ {\
+ .driver = "nec-usb-xhci",\
+ .property = "superspeed-ports-first",\
+ .value = "off",\
+ },\
+ {\
+ .driver = "pci-serial",\
+ .property = "prog_if",\
+ .value = stringify(0),\
+ },\
+ {\
+ .driver = "pci-serial-2x",\
+ .property = "prog_if",\
+ .value = stringify(0),\
+ },\
+ {\
+ .driver = "pci-serial-4x",\
+ .property = "prog_if",\
+ .value = stringify(0),\
+ },\
+ {\
+ .driver = "virtio-net-pci",\
+ .property = "guest_announce",\
+ .value = "off",\
+ },\
+ {\
+ .driver = "ICH9-LPC",\
+ .property = "memory-hotplug-support",\
+ .value = "off",\
+ },{\
+ .driver = "xio3130-downstream",\
+ .property = COMPAT_PROP_PCP,\
+ .value = "off",\
+ },{\
+ .driver = "ioh3420",\
+ .property = COMPAT_PROP_PCP,\
+ .value = "off",\
+ }
+
+#define QEMU_COMPAT_1_7 \
+ {\
+ .driver = TYPE_USB_DEVICE,\
+ .property = "msos-desc",\
+ .value = "no",\
+ },\
+ {\
+ .driver = "PIIX4_PM",\
+ .property = "acpi-pci-hotplug-with-bridge-support",\
+ .value = "off",\
+ },\
+ {\
+ .driver = "hpet",\
+ .property = HPET_INTCAP,\
+ .value = stringify(4),\
+ }
+
+#define QEMU_COMPAT_1_6 \
+ {\
+ .driver = "e1000",\
+ .property = "mitigation",\
+ .value = "off",\
+ },{\
+ .driver = "qemu64-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(2),\
+ },{\
+ .driver = "qemu32-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(3),\
+ },{\
+ .driver = "i440FX-pcihost",\
+ .property = "short_root_bus",\
+ .value = stringify(1),\
+ },{\
+ .driver = "q35-pcihost",\
+ .property = "short_root_bus",\
+ .value = stringify(1),\
+ }
+
+#define QEMU_COMPAT_1_5 \
+ {\
+ .driver = "Conroe-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(2),\
+ },{\
+ .driver = "Conroe-" TYPE_X86_CPU,\
+ .property = "level",\
+ .value = stringify(2),\
+ },{\
+ .driver = "Penryn-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(2),\
+ },{\
+ .driver = "Penryn-" TYPE_X86_CPU,\
+ .property = "level",\
+ .value = stringify(2),\
+ },{\
+ .driver = "Nehalem-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(2),\
+ },{\
+ .driver = "Nehalem-" TYPE_X86_CPU,\
+ .property = "level",\
+ .value = stringify(2),\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = TYPE_X86_CPU,\
+ .property = "pmu",\
+ .value = "on",\
+ },{\
+ .driver = "i440FX-pcihost",\
+ .property = "short_root_bus",\
+ .value = stringify(0),\
+ },{\
+ .driver = "q35-pcihost",\
+ .property = "short_root_bus",\
+ .value = stringify(0),\
+ }
+
+#define QEMU_COMPAT_1_4 \
+ {\
+ .driver = "scsi-hd",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "scsi-cd",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "scsi-disk",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "ide-hd",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "ide-cd",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "ide-drive",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "virtio-blk-pci",\
+ .property = "discard_granularity",\
+ .value = stringify(0),\
+ },{\
+ .driver = "virtio-serial-pci",\
+ .property = "vectors",\
+ /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
+ .value = stringify(0xFFFFFFFF),\
+ },{ \
+ .driver = "virtio-net-pci", \
+ .property = "ctrl_guest_offloads", \
+ .value = "off", \
+ },{\
+ .driver = "e1000",\
+ .property = "romfile",\
+ .value = "pxe-e1000.rom",\
+ },{\
+ .driver = "ne2k_pci",\
+ .property = "romfile",\
+ .value = "pxe-ne2k_pci.rom",\
+ },{\
+ .driver = "pcnet",\
+ .property = "romfile",\
+ .value = "pxe-pcnet.rom",\
+ },{\
+ .driver = "rtl8139",\
+ .property = "romfile",\
+ .value = "pxe-rtl8139.rom",\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "romfile",\
+ .value = "pxe-virtio.rom",\
+ },{\
+ .driver = "486-" TYPE_X86_CPU,\
+ .property = "model",\
+ .value = stringify(0),\
+ }
+
+
+#endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1c0c382..5907e99 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -4,6 +4,7 @@
#include "qemu-common.h"
#include "exec/memory.h"
#include "hw/boards.h"
+#include "hw/compat.h"
#include "hw/isa/isa.h"
#include "hw/block/fdc.h"
#include "net/net.h"
@@ -295,209 +296,24 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_0 \
- {\
- .driver = "virtio-scsi-pci",\
- .property = "any_layout",\
- .value = "off",\
- },{\
- .driver = "PIIX4_PM",\
- .property = "memory-hotplug-support",\
- .value = "off",\
- },\
- {\
- .driver = "apic",\
- .property = "version",\
- .value = stringify(0x11),\
- },\
- {\
- .driver = "nec-usb-xhci",\
- .property = "superspeed-ports-first",\
- .value = "off",\
- },\
- {\
- .driver = "pci-serial",\
- .property = "prog_if",\
- .value = stringify(0),\
- },\
- {\
- .driver = "pci-serial-2x",\
- .property = "prog_if",\
- .value = stringify(0),\
- },\
- {\
- .driver = "pci-serial-4x",\
- .property = "prog_if",\
- .value = stringify(0),\
- },\
- {\
- .driver = "virtio-net-pci",\
- .property = "guest_announce",\
- .value = "off",\
- },\
- {\
- .driver = "ICH9-LPC",\
- .property = "memory-hotplug-support",\
- .value = "off",\
- },{\
- .driver = "xio3130-downstream",\
- .property = COMPAT_PROP_PCP,\
- .value = "off",\
- },{\
- .driver = "ioh3420",\
- .property = COMPAT_PROP_PCP,\
- .value = "off",\
- }
+ QEMU_COMPAT_2_0
#define PC_COMPAT_1_7 \
- PC_COMPAT_2_0, \
- {\
- .driver = TYPE_USB_DEVICE,\
- .property = "msos-desc",\
- .value = "no",\
- },\
- {\
- .driver = "PIIX4_PM",\
- .property = "acpi-pci-hotplug-with-bridge-support",\
- .value = "off",\
- },\
- {\
- .driver = "hpet",\
- .property = HPET_INTCAP,\
- .value = stringify(4),\
- }
+ PC_COMPAT_2_0,\
+ QEMU_COMPAT_1_7
#define PC_COMPAT_1_6 \
- PC_COMPAT_1_7, \
- {\
- .driver = "e1000",\
- .property = "mitigation",\
- .value = "off",\
- },{\
- .driver = "qemu64-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(2),\
- },{\
- .driver = "qemu32-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(3),\
- },{\
- .driver = "i440FX-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(1),\
- },{\
- .driver = "q35-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(1),\
- }
+ PC_COMPAT_1_7,\
+ QEMU_COMPAT_1_6
#define PC_COMPAT_1_5 \
- PC_COMPAT_1_6, \
- {\
- .driver = "Conroe-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(2),\
- },{\
- .driver = "Conroe-" TYPE_X86_CPU,\
- .property = "level",\
- .value = stringify(2),\
- },{\
- .driver = "Penryn-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(2),\
- },{\
- .driver = "Penryn-" TYPE_X86_CPU,\
- .property = "level",\
- .value = stringify(2),\
- },{\
- .driver = "Nehalem-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(2),\
- },{\
- .driver = "Nehalem-" TYPE_X86_CPU,\
- .property = "level",\
- .value = stringify(2),\
- },{\
- .driver = "virtio-net-pci",\
- .property = "any_layout",\
- .value = "off",\
- },{\
- .driver = TYPE_X86_CPU,\
- .property = "pmu",\
- .value = "on",\
- },{\
- .driver = "i440FX-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(0),\
- },{\
- .driver = "q35-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(0),\
- }
+ PC_COMPAT_1_6,\
+ QEMU_COMPAT_1_5
#define PC_COMPAT_1_4 \
- PC_COMPAT_1_5, \
- {\
- .driver = "scsi-hd",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "scsi-cd",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "scsi-disk",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "ide-hd",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "ide-cd",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "ide-drive",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "virtio-blk-pci",\
- .property = "discard_granularity",\
- .value = stringify(0),\
- },{\
- .driver = "virtio-serial-pci",\
- .property = "vectors",\
- /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
- .value = stringify(0xFFFFFFFF),\
- },{ \
- .driver = "virtio-net-pci", \
- .property = "ctrl_guest_offloads", \
- .value = "off", \
- },{\
- .driver = "e1000",\
- .property = "romfile",\
- .value = "pxe-e1000.rom",\
- },{\
- .driver = "ne2k_pci",\
- .property = "romfile",\
- .value = "pxe-ne2k_pci.rom",\
- },{\
- .driver = "pcnet",\
- .property = "romfile",\
- .value = "pxe-pcnet.rom",\
- },{\
- .driver = "rtl8139",\
- .property = "romfile",\
- .value = "pxe-rtl8139.rom",\
- },{\
- .driver = "virtio-net-pci",\
- .property = "romfile",\
- .value = "pxe-virtio.rom",\
- },{\
- .driver = "486-" TYPE_X86_CPU,\
- .property = "model",\
- .value = stringify(0),\
- }
+ PC_COMPAT_1_5,\
+ QEMU_COMPAT_1_4
+
#define PC_COMMON_MACHINE_OPTIONS \
.default_boot_order = "cad"
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_*
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
@ 2014-06-25 5:53 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:53 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Peter Maydell, Liu Ping Fan, qemu-devel
On Tue, Jun 24, 2014 at 11:04:44PM -0300, Eduardo Habkost wrote:
> For each compat property on PC_Q35_COMPAT_*, there are only two
> possibilities:
>
> * If the device is never instantiated when using a machine other than
> pc-q35, then the compat property can be safely added to
> PC_COMPAT_*;
> * If the device can be instantiated when using a machine other than
> pc-q35, that means the other machines also need the compat property
> to be set.
> That means we don't need separate PC_Q35_COMPAT_* macros at all, today.
>
> The hpet.hpet-intcap case is interesting: piix and q35 do have something
> that emulates different defaults, but the machine-specific default is
> applied _after_ compat_props are applied, by simply checking if the
> property is zero (which is the real default on the hpet code).
>
> The hpet.hpet-intcap=0x4 compat property can (should?) be applied to
> piix too, because 0x4 was the default on both piix and q35 before the
> hpet-intcap property was introduced.
True. That's a very good explanation, thanks!
> Now, if one day we change the default HPET intcap on one of the PC
> machine-types again, we may want to introduce PC_{Q35,I440FX}_COMPAT
> macros. But while we don't need that, we can keep the code simple.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
Small patch, less code, same functionality, what's not to like.
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/pc_q35.c | 10 +++++-----
> include/hw/i386/pc.h | 55 +++++++++++++++++-----------------------------------
> 2 files changed, 23 insertions(+), 42 deletions(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 155db99..36b6ab0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -361,7 +361,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
> .name = "pc-q35-2.0",
> .init = pc_q35_init_2_0,
> .compat_props = (GlobalProperty[]) {
> - PC_Q35_COMPAT_2_0,
> + PC_COMPAT_2_0,
> { /* end of list */ }
> },
> };
> @@ -373,7 +373,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
> .name = "pc-q35-1.7",
> .init = pc_q35_init_1_7,
> .compat_props = (GlobalProperty[]) {
> - PC_Q35_COMPAT_1_7,
> + PC_COMPAT_1_7,
> { /* end of list */ }
> },
> };
> @@ -385,7 +385,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
> .name = "pc-q35-1.6",
> .init = pc_q35_init_1_6,
> .compat_props = (GlobalProperty[]) {
> - PC_Q35_COMPAT_1_6,
> + PC_COMPAT_1_6,
> { /* end of list */ }
> },
> };
> @@ -395,7 +395,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
> .name = "pc-q35-1.5",
> .init = pc_q35_init_1_5,
> .compat_props = (GlobalProperty[]) {
> - PC_Q35_COMPAT_1_5,
> + PC_COMPAT_1_5,
> { /* end of list */ }
> },
> };
> @@ -409,7 +409,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
> .name = "pc-q35-1.4",
> .init = pc_q35_init_1_4,
> .compat_props = (GlobalProperty[]) {
> - PC_Q35_COMPAT_1_4,
> + PC_COMPAT_1_4,
> { /* end of list */ }
> },
> };
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1331d5a..1c0c382 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,43 +294,6 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> -#define PC_Q35_COMPAT_2_0 \
> - PC_COMPAT_2_0, \
> - {\
> - .driver = "ICH9-LPC",\
> - .property = "memory-hotplug-support",\
> - .value = "off",\
> - },{\
> - .driver = "xio3130-downstream",\
> - .property = COMPAT_PROP_PCP,\
> - .value = "off",\
> - },{\
> - .driver = "ioh3420",\
> - .property = COMPAT_PROP_PCP,\
> - .value = "off",\
> - }
> -
> -#define PC_Q35_COMPAT_1_7 \
> - PC_COMPAT_1_7, \
> - PC_Q35_COMPAT_2_0, \
> - {\
> - .driver = "hpet",\
> - .property = HPET_INTCAP,\
> - .value = stringify(4),\
> - }
> -
> -#define PC_Q35_COMPAT_1_6 \
> - PC_COMPAT_1_6, \
> - PC_Q35_COMPAT_1_7
> -
> -#define PC_Q35_COMPAT_1_5 \
> - PC_COMPAT_1_5, \
> - PC_Q35_COMPAT_1_6
> -
> -#define PC_Q35_COMPAT_1_4 \
> - PC_COMPAT_1_4, \
> - PC_Q35_COMPAT_1_5
> -
> #define PC_COMPAT_2_0 \
> {\
> .driver = "virtio-scsi-pci",\
> @@ -370,6 +333,19 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .driver = "virtio-net-pci",\
> .property = "guest_announce",\
> .value = "off",\
> + },\
> + {\
> + .driver = "ICH9-LPC",\
> + .property = "memory-hotplug-support",\
> + .value = "off",\
> + },{\
> + .driver = "xio3130-downstream",\
> + .property = COMPAT_PROP_PCP,\
> + .value = "off",\
> + },{\
> + .driver = "ioh3420",\
> + .property = COMPAT_PROP_PCP,\
> + .value = "off",\
> }
>
> #define PC_COMPAT_1_7 \
> @@ -383,6 +359,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> .driver = "PIIX4_PM",\
> .property = "acpi-pci-hotplug-with-bridge-support",\
> .value = "off",\
> + },\
> + {\
> + .driver = "hpet",\
> + .property = HPET_INTCAP,\
> + .value = stringify(4),\
> }
>
> #define PC_COMPAT_1_6 \
> --
> 1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-25 5:56 ` Michael S. Tsirkin
2014-06-25 13:23 ` Eduardo Habkost
0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:56 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Peter Maydell, qemu-devel
On Tue, Jun 24, 2014 at 11:04:45PM -0300, Eduardo Habkost wrote:
> The QEMU_COMPAT_* macros will contain compat properties that are not
> specific to PC, and may be reused by other machine-types.
>
> The compat properties for PC-specific devices were moved to
> QEMU_COMPAT_* too, because they are simply not going to be applied to
> any device if they are not instantiated.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
I don't see value in this.
If some target wants to start versioning their machine types,
let them start from 2.0 (or even 2.1 - does any non-PC target
plan to support cross-version compatibility for 2.0?).
Why carry around useless code for < 2.0 for non PC platforms?
> ---
> include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 206 +++-----------------------------------------------
> 2 files changed, 221 insertions(+), 195 deletions(-)
> create mode 100644 include/hw/compat.h
And we have lost all we have gained in 1/2 :(
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> new file mode 100644
> index 0000000..dff55c8
> --- /dev/null
> +++ b/include/hw/compat.h
> @@ -0,0 +1,210 @@
> +/* Common compat properties that can be used by machine-types to keep guest ABI
> + * stable between QEMU versions.
> + */
> +
> +#ifndef HW_COMPAT_H
> +#define HW_COMPAT_H
> +
> +#define QEMU_COMPAT_2_0 \
> + {\
> + .driver = "virtio-scsi-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "PIIX4_PM",\
> + .property = "memory-hotplug-support",\
> + .value = "off",\
> + },\
> + {\
> + .driver = "apic",\
> + .property = "version",\
> + .value = stringify(0x11),\
> + },\
> + {\
> + .driver = "nec-usb-xhci",\
> + .property = "superspeed-ports-first",\
> + .value = "off",\
> + },\
> + {\
> + .driver = "pci-serial",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "pci-serial-2x",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "pci-serial-4x",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "virtio-net-pci",\
> + .property = "guest_announce",\
> + .value = "off",\
> + },\
> + {\
> + .driver = "ICH9-LPC",\
> + .property = "memory-hotplug-support",\
> + .value = "off",\
> + },{\
> + .driver = "xio3130-downstream",\
> + .property = COMPAT_PROP_PCP,\
> + .value = "off",\
> + },{\
> + .driver = "ioh3420",\
> + .property = COMPAT_PROP_PCP,\
> + .value = "off",\
> + }
> +
> +#define QEMU_COMPAT_1_7 \
> + {\
> + .driver = TYPE_USB_DEVICE,\
> + .property = "msos-desc",\
> + .value = "no",\
> + },\
> + {\
> + .driver = "PIIX4_PM",\
> + .property = "acpi-pci-hotplug-with-bridge-support",\
> + .value = "off",\
> + },\
> + {\
> + .driver = "hpet",\
> + .property = HPET_INTCAP,\
> + .value = stringify(4),\
> + }
> +
> +#define QEMU_COMPAT_1_6 \
> + {\
> + .driver = "e1000",\
> + .property = "mitigation",\
> + .value = "off",\
> + },{\
> + .driver = "qemu64-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(2),\
> + },{\
> + .driver = "qemu32-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(3),\
> + },{\
> + .driver = "i440FX-pcihost",\
> + .property = "short_root_bus",\
> + .value = stringify(1),\
> + },{\
> + .driver = "q35-pcihost",\
> + .property = "short_root_bus",\
> + .value = stringify(1),\
> + }
> +
> +#define QEMU_COMPAT_1_5 \
> + {\
> + .driver = "Conroe-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(2),\
> + },{\
> + .driver = "Conroe-" TYPE_X86_CPU,\
> + .property = "level",\
> + .value = stringify(2),\
> + },{\
> + .driver = "Penryn-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(2),\
> + },{\
> + .driver = "Penryn-" TYPE_X86_CPU,\
> + .property = "level",\
> + .value = stringify(2),\
> + },{\
> + .driver = "Nehalem-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(2),\
> + },{\
> + .driver = "Nehalem-" TYPE_X86_CPU,\
> + .property = "level",\
> + .value = stringify(2),\
> + },{\
> + .driver = "virtio-net-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = TYPE_X86_CPU,\
> + .property = "pmu",\
> + .value = "on",\
> + },{\
> + .driver = "i440FX-pcihost",\
> + .property = "short_root_bus",\
> + .value = stringify(0),\
> + },{\
> + .driver = "q35-pcihost",\
> + .property = "short_root_bus",\
> + .value = stringify(0),\
> + }
> +
> +#define QEMU_COMPAT_1_4 \
> + {\
> + .driver = "scsi-hd",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "scsi-cd",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "scsi-disk",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "ide-hd",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "ide-cd",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "ide-drive",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "virtio-blk-pci",\
> + .property = "discard_granularity",\
> + .value = stringify(0),\
> + },{\
> + .driver = "virtio-serial-pci",\
> + .property = "vectors",\
> + /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> + .value = stringify(0xFFFFFFFF),\
> + },{ \
> + .driver = "virtio-net-pci", \
> + .property = "ctrl_guest_offloads", \
> + .value = "off", \
> + },{\
> + .driver = "e1000",\
> + .property = "romfile",\
> + .value = "pxe-e1000.rom",\
> + },{\
> + .driver = "ne2k_pci",\
> + .property = "romfile",\
> + .value = "pxe-ne2k_pci.rom",\
> + },{\
> + .driver = "pcnet",\
> + .property = "romfile",\
> + .value = "pxe-pcnet.rom",\
> + },{\
> + .driver = "rtl8139",\
> + .property = "romfile",\
> + .value = "pxe-rtl8139.rom",\
> + },{\
> + .driver = "virtio-net-pci",\
> + .property = "romfile",\
> + .value = "pxe-virtio.rom",\
> + },{\
> + .driver = "486-" TYPE_X86_CPU,\
> + .property = "model",\
> + .value = stringify(0),\
> + }
> +
> +
Two empty lines.
> +#endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1c0c382..5907e99 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -4,6 +4,7 @@
> #include "qemu-common.h"
> #include "exec/memory.h"
> #include "hw/boards.h"
> +#include "hw/compat.h"
> #include "hw/isa/isa.h"
> #include "hw/block/fdc.h"
> #include "net/net.h"
> @@ -295,209 +296,24 @@ int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> #define PC_COMPAT_2_0 \
> - {\
> - .driver = "virtio-scsi-pci",\
> - .property = "any_layout",\
> - .value = "off",\
> - },{\
> - .driver = "PIIX4_PM",\
> - .property = "memory-hotplug-support",\
> - .value = "off",\
> - },\
> - {\
> - .driver = "apic",\
> - .property = "version",\
> - .value = stringify(0x11),\
> - },\
> - {\
> - .driver = "nec-usb-xhci",\
> - .property = "superspeed-ports-first",\
> - .value = "off",\
> - },\
> - {\
> - .driver = "pci-serial",\
> - .property = "prog_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "pci-serial-2x",\
> - .property = "prog_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "pci-serial-4x",\
> - .property = "prog_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "virtio-net-pci",\
> - .property = "guest_announce",\
> - .value = "off",\
> - },\
> - {\
> - .driver = "ICH9-LPC",\
> - .property = "memory-hotplug-support",\
> - .value = "off",\
> - },{\
> - .driver = "xio3130-downstream",\
> - .property = COMPAT_PROP_PCP,\
> - .value = "off",\
> - },{\
> - .driver = "ioh3420",\
> - .property = COMPAT_PROP_PCP,\
> - .value = "off",\
> - }
> + QEMU_COMPAT_2_0
>
> #define PC_COMPAT_1_7 \
> - PC_COMPAT_2_0, \
> - {\
> - .driver = TYPE_USB_DEVICE,\
> - .property = "msos-desc",\
> - .value = "no",\
> - },\
> - {\
> - .driver = "PIIX4_PM",\
> - .property = "acpi-pci-hotplug-with-bridge-support",\
> - .value = "off",\
> - },\
> - {\
> - .driver = "hpet",\
> - .property = HPET_INTCAP,\
> - .value = stringify(4),\
> - }
> + PC_COMPAT_2_0,\
> + QEMU_COMPAT_1_7
>
> #define PC_COMPAT_1_6 \
> - PC_COMPAT_1_7, \
> - {\
> - .driver = "e1000",\
> - .property = "mitigation",\
> - .value = "off",\
> - },{\
> - .driver = "qemu64-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(2),\
> - },{\
> - .driver = "qemu32-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(3),\
> - },{\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - }
> + PC_COMPAT_1_7,\
> + QEMU_COMPAT_1_6
>
> #define PC_COMPAT_1_5 \
> - PC_COMPAT_1_6, \
> - {\
> - .driver = "Conroe-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(2),\
> - },{\
> - .driver = "Conroe-" TYPE_X86_CPU,\
> - .property = "level",\
> - .value = stringify(2),\
> - },{\
> - .driver = "Penryn-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(2),\
> - },{\
> - .driver = "Penryn-" TYPE_X86_CPU,\
> - .property = "level",\
> - .value = stringify(2),\
> - },{\
> - .driver = "Nehalem-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(2),\
> - },{\
> - .driver = "Nehalem-" TYPE_X86_CPU,\
> - .property = "level",\
> - .value = stringify(2),\
> - },{\
> - .driver = "virtio-net-pci",\
> - .property = "any_layout",\
> - .value = "off",\
> - },{\
> - .driver = TYPE_X86_CPU,\
> - .property = "pmu",\
> - .value = "on",\
> - },{\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - }
> + PC_COMPAT_1_6,\
> + QEMU_COMPAT_1_5
>
> #define PC_COMPAT_1_4 \
> - PC_COMPAT_1_5, \
> - {\
> - .driver = "scsi-hd",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "scsi-cd",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "scsi-disk",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "ide-hd",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "ide-cd",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "ide-drive",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "virtio-blk-pci",\
> - .property = "discard_granularity",\
> - .value = stringify(0),\
> - },{\
> - .driver = "virtio-serial-pci",\
> - .property = "vectors",\
> - /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> - .value = stringify(0xFFFFFFFF),\
> - },{ \
> - .driver = "virtio-net-pci", \
> - .property = "ctrl_guest_offloads", \
> - .value = "off", \
> - },{\
> - .driver = "e1000",\
> - .property = "romfile",\
> - .value = "pxe-e1000.rom",\
> - },{\
> - .driver = "ne2k_pci",\
> - .property = "romfile",\
> - .value = "pxe-ne2k_pci.rom",\
> - },{\
> - .driver = "pcnet",\
> - .property = "romfile",\
> - .value = "pxe-pcnet.rom",\
> - },{\
> - .driver = "rtl8139",\
> - .property = "romfile",\
> - .value = "pxe-rtl8139.rom",\
> - },{\
> - .driver = "virtio-net-pci",\
> - .property = "romfile",\
> - .value = "pxe-virtio.rom",\
> - },{\
> - .driver = "486-" TYPE_X86_CPU,\
> - .property = "model",\
> - .value = stringify(0),\
> - }
> + PC_COMPAT_1_5,\
> + QEMU_COMPAT_1_4
> +
>
Two emoty lines.
> #define PC_COMMON_MACHINE_OPTIONS \
> .default_boot_order = "cad"
> --
> 1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros
2014-06-25 2:04 [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-25 5:58 ` Michael S. Tsirkin
2 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:58 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Marcel Apfelbaum, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Tue, Jun 24, 2014 at 11:04:43PM -0300, Eduardo Habkost wrote:
> This series is an attempt to make the compat_props lists from the PC code
> reusable by other machine-types. All the compat bits that are on those lists are
> not tied to a specific machine-type, but instead to the device code that was
> present on a given QEMU version.
I applied 1/2, thanks!
> This series is based on Michael's "pci" branch, and it also requires the
> following patch to be applied first:
>
> From: Eduardo Habkost <ehabkost@redhat.com>
> Date: Tue, 24 Jun 2014 19:57:55 -0300
> Message-Id: <1403650675-3368-1-git-send-email-ehabkost@redhat.com>
> Subject: [PATCH] pc: Fix "prog_if" typo on PC_COMPAT_2_0
>
> Changes v1 -> v2:
> * Move QEMU_COMPAT_* to hw/compat.h
> * Eliminate machine-type-specific PC_{Q35,I440FX}_COMPAT_* macros, as they
> are not necessary today
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Cc: Marcel Apfelbaum <marcel.a@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: QEMU Developers <qemu-devel@nongnu.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Andreas Färber <afaerber@suse.de>
> Cc: Alexander Graf <agraf@suse.com>
>
> Eduardo Habkost (2):
> pc: Move q35 compat props to PC_COMPAT_*
> machine: Introduce QEMU_COMPAT_* macros
>
> hw/i386/pc_q35.c | 10 +--
> include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 225 +++------------------------------------------------
> 3 files changed, 226 insertions(+), 219 deletions(-)
> create mode 100644 include/hw/compat.h
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 5:56 ` Michael S. Tsirkin
@ 2014-06-25 13:23 ` Eduardo Habkost
2014-06-25 14:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2014-06-25 13:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Marcel Apfelbaum, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, Jun 25, 2014 at 08:56:43AM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 24, 2014 at 11:04:45PM -0300, Eduardo Habkost wrote:
> > The QEMU_COMPAT_* macros will contain compat properties that are not
> > specific to PC, and may be reused by other machine-types.
> >
> > The compat properties for PC-specific devices were moved to
> > QEMU_COMPAT_* too, because they are simply not going to be applied to
> > any device if they are not instantiated.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> I don't see value in this.
> If some target wants to start versioning their machine types,
> let them start from 2.0 (or even 2.1 - does any non-PC target
> plan to support cross-version compatibility for 2.0?).
> Why carry around useless code for < 2.0 for non PC platforms?
>
> > ---
> > include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 206 +++-----------------------------------------------
> > 2 files changed, 221 insertions(+), 195 deletions(-)
> > create mode 100644 include/hw/compat.h
>
> And we have lost all we have gained in 1/2 :(
>
As we will be required to follow the QEMU_COMPAT/PC_COMPAT pattern
starting from 2.1, I like to keep exactly the same pattern for all older
macros, even if QEMU_COMPAT_1_* are used only by PC. Makes it easier to
read and understand, and information easier to find (instead of having
the old compat lists scattered all over the place, most of them are now
in a single header file).
Also, note that the PC_COMPAT_* macros will be removed once we convert
PC to pure QOM code after 2.1.0.
Anyway, if the maintainers see no value in having QEMU_COMPAT_* macros
used only by PC, we can do this only after 2.1. Otherwise, if there's
value in a QEMU_COMPAT_2_0 macro used only by PC, there's value in
QEMU_COMPAT_1_* too.
In either case, this is simply a demonstration of how we can proceed to
introduce the versioned non-PC machine-types later. We don't strictly
need this patch today.
> >
[...]
> > + }
> > +
> > +
>
> Two empty lines.
I will fix it in case I submit it again. Thanks.
>
[...]
> > - }
> > + PC_COMPAT_1_5,\
> > + QEMU_COMPAT_1_4
> > +
> >
>
> Two emoty lines.
Ditto.
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 13:23 ` Eduardo Habkost
@ 2014-06-25 14:18 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 14:18 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Marcel Apfelbaum, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, Jun 25, 2014 at 10:23:06AM -0300, Eduardo Habkost wrote:
> On Wed, Jun 25, 2014 at 08:56:43AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 24, 2014 at 11:04:45PM -0300, Eduardo Habkost wrote:
> > > The QEMU_COMPAT_* macros will contain compat properties that are not
> > > specific to PC, and may be reused by other machine-types.
> > >
> > > The compat properties for PC-specific devices were moved to
> > > QEMU_COMPAT_* too, because they are simply not going to be applied to
> > > any device if they are not instantiated.
> > >
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >
> > I don't see value in this.
> > If some target wants to start versioning their machine types,
> > let them start from 2.0 (or even 2.1 - does any non-PC target
> > plan to support cross-version compatibility for 2.0?).
> > Why carry around useless code for < 2.0 for non PC platforms?
> >
> > > ---
> > > include/hw/compat.h | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > include/hw/i386/pc.h | 206 +++-----------------------------------------------
> > > 2 files changed, 221 insertions(+), 195 deletions(-)
> > > create mode 100644 include/hw/compat.h
> >
> > And we have lost all we have gained in 1/2 :(
> >
>
> As we will be required to follow the QEMU_COMPAT/PC_COMPAT pattern
> starting from 2.1, I like to keep exactly the same pattern for all older
> macros, even if QEMU_COMPAT_1_* are used only by PC. Makes it easier to
> read and understand, and information easier to find (instead of having
> the old compat lists scattered all over the place, most of them are now
> in a single header file).
>
> Also, note that the PC_COMPAT_* macros will be removed once we convert
> PC to pure QOM code after 2.1.0.
>
> Anyway, if the maintainers see no value in having QEMU_COMPAT_* macros
> used only by PC, we can do this only after 2.1. Otherwise, if there's
> value in a QEMU_COMPAT_2_0 macro used only by PC, there's value in
> QEMU_COMPAT_1_* too.
>
> In either case, this is simply a demonstration of how we can proceed to
> introduce the versioned non-PC machine-types later. We don't strictly
> need this patch today.
I think it's best for you guys to hammer on it on a private
branch. It's past devel freeze and this is new code,
so I'm not applying anything that's not trivial.
Patch 1/2 seems to hit the sweet spot.
> > >
> [...]
> > > + }
> > > +
> > > +
> >
> > Two empty lines.
>
> I will fix it in case I submit it again. Thanks.
>
> >
> [...]
> > > - }
> > > + PC_COMPAT_1_5,\
> > > + QEMU_COMPAT_1_4
> > > +
> > >
> >
> > Two emoty lines.
>
> Ditto.
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-06-25 14:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-25 2:04 [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
2014-06-25 5:53 ` Michael S. Tsirkin
2014-06-25 2:04 ` [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25 5:56 ` Michael S. Tsirkin
2014-06-25 13:23 ` Eduardo Habkost
2014-06-25 14:18 ` Michael S. Tsirkin
2014-06-25 5:58 ` [Qemu-devel] [PATCH v2 0/2] Introduce common " 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.