* [Qemu-devel] [PATCH 1/4] q35: Move q35-specific compat macros to pc_q35.c
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-24 18:02 ` Eduardo Habkost
2014-06-24 19:23 ` Marcel Apfelbaum
2014-06-24 18:02 ` [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros Eduardo Habkost
` (5 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 18:02 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
They are not used anywhere else, to it is simpler to just keep them
closer to the places where they are used.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/pc_q35.c | 37 +++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 37 -------------------------------------
2 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 155db99..c640e7b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -354,6 +354,22 @@ static QEMUMachine pc_q35_machine_v2_1 = {
.init = pc_q35_init,
};
+#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_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
static QEMUMachine pc_q35_machine_v2_0 = {
@@ -366,6 +382,15 @@ static QEMUMachine pc_q35_machine_v2_0 = {
},
};
+#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_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
static QEMUMachine pc_q35_machine_v1_7 = {
@@ -378,6 +403,10 @@ static QEMUMachine pc_q35_machine_v1_7 = {
},
};
+#define PC_Q35_COMPAT_1_6 \
+ PC_COMPAT_1_6, \
+ PC_Q35_COMPAT_1_7
+
#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
static QEMUMachine pc_q35_machine_v1_6 = {
@@ -390,6 +419,10 @@ static QEMUMachine pc_q35_machine_v1_6 = {
},
};
+#define PC_Q35_COMPAT_1_5 \
+ PC_COMPAT_1_5, \
+ PC_Q35_COMPAT_1_6
+
static QEMUMachine pc_q35_machine_v1_5 = {
PC_Q35_1_6_MACHINE_OPTIONS,
.name = "pc-q35-1.5",
@@ -400,6 +433,10 @@ static QEMUMachine pc_q35_machine_v1_5 = {
},
};
+#define PC_Q35_COMPAT_1_4 \
+ PC_COMPAT_1_4, \
+ PC_Q35_COMPAT_1_5
+
#define PC_Q35_1_4_MACHINE_OPTIONS \
PC_Q35_1_6_MACHINE_OPTIONS, \
.hot_add_cpu = NULL
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 486e98f..fb7d68d 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",\
--
1.9.3
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 1/4] q35: Move q35-specific compat macros to pc_q35.c
2014-06-24 18:02 ` [Qemu-devel] [PATCH 1/4] q35: Move q35-specific compat macros to pc_q35.c Eduardo Habkost
@ 2014-06-24 19:23 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2014-06-24 19:23 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Michael S. Tsirkin, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Tue, 2014-06-24 at 15:02 -0300, Eduardo Habkost wrote:
> They are not used anywhere else, to it is simpler to just keep them
> closer to the places where they are used.
Isolation is always welcomed.
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/i386/pc_q35.c | 37 +++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 37 -------------------------------------
> 2 files changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 155db99..c640e7b 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -354,6 +354,22 @@ static QEMUMachine pc_q35_machine_v2_1 = {
> .init = pc_q35_init,
> };
>
> +#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_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
>
> static QEMUMachine pc_q35_machine_v2_0 = {
> @@ -366,6 +382,15 @@ static QEMUMachine pc_q35_machine_v2_0 = {
> },
> };
>
> +#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_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> static QEMUMachine pc_q35_machine_v1_7 = {
> @@ -378,6 +403,10 @@ static QEMUMachine pc_q35_machine_v1_7 = {
> },
> };
>
> +#define PC_Q35_COMPAT_1_6 \
> + PC_COMPAT_1_6, \
> + PC_Q35_COMPAT_1_7
> +
> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> static QEMUMachine pc_q35_machine_v1_6 = {
> @@ -390,6 +419,10 @@ static QEMUMachine pc_q35_machine_v1_6 = {
> },
> };
>
> +#define PC_Q35_COMPAT_1_5 \
> + PC_COMPAT_1_5, \
> + PC_Q35_COMPAT_1_6
> +
> static QEMUMachine pc_q35_machine_v1_5 = {
> PC_Q35_1_6_MACHINE_OPTIONS,
> .name = "pc-q35-1.5",
> @@ -400,6 +433,10 @@ static QEMUMachine pc_q35_machine_v1_5 = {
> },
> };
>
> +#define PC_Q35_COMPAT_1_4 \
> + PC_COMPAT_1_4, \
> + PC_Q35_COMPAT_1_5
> +
> #define PC_Q35_1_4_MACHINE_OPTIONS \
> PC_Q35_1_6_MACHINE_OPTIONS, \
> .hot_add_cpu = NULL
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 486e98f..fb7d68d 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",\
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-24 18:02 ` [Qemu-devel] [PATCH 1/4] q35: Move q35-specific compat macros to pc_q35.c Eduardo Habkost
@ 2014-06-24 18:02 ` Eduardo Habkost
2014-06-25 5:13 ` Michael S. Tsirkin
2014-06-24 18:02 ` [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 18:02 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
The PC_COMPAT_* macros include each other, and the PC_Q35_COMPAT_*
macros _also_ included each other. That caused lots of properties to
appear multiple times on PC_Q35_COMPAT_*.
For example, PC_Q35_COMPAT_1_4 expanded to the following:
PC_Q35_COMPAT_1_4:
|-> PC_COMPAT_1_4:
| |-> PC_COMPAT_1_5:
| | |-> PC_COMPAT_1_6:
| | | |-> PC_COMPAT_1_7:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-1.7 vars]
| | | `-> [pc-1.6 vars]
| | `-> [pc-1.5 vars]
| `-> [pc-1.4 vars]
|-> PC_Q35_COMPAT_1_5:
| |-> PC_COMPAT_1_5:
| | |-> PC_COMPAT_1_6:
| | | |-> PC_COMPAT_1_7:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-1.7 vars]
| | | `-> [pc-1.6 vars]
| | `-> [pc-1.5 vars]
| |-> PC_Q35_COMPAT_1_6:
| | |-> PC_COMPAT_1_6:
| | | |-> PC_COMPAT_1_7:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-1.7 vars]
| | | `-> [pc-1.6 vars]
| | |-> PC_Q35_COMPAT_1_7:
| | | |-> PC_COMPAT_1_7:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-1.7 vars]
| | | |-> PC_Q35_COMPAT_2_0:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-q35-2.0 vars]
| | | `-> [pc-q35-1.7 vars]
| | `-> [pc-q35-1.6 vars]
| `-> [pc-q35-1.5 vars]
`-> [pc-q35-1.4 vars]
In other words, the pc-2.0 variables from PC_COMPAT_2_0 appeared 5 times
inside PC_Q35_COMPAT_1_4.
This makes the following changes:
* Reusable PC_COMPAT_* macros on pc.h have NO nesting;
* Instead of using PC_COMPAT_* directly, now pc_piix.c has its own
PC_I440FX_COMPAT_* macros.
* Machine-type-specific PC_*_COMPAT_* macros on .c files are
nested, and reuse the common PC_COMPAT_* macros.
Thus the expansion of PC_Q35_COMPAT_1_4 becomes:
PC_Q35_COMPAT_1_4:
|-> PC_Q35_COMPAT_1_5:
| |-> PC_Q35_COMPAT_1_6:
| | |-> PC_Q35_COMPAT_1_7:
| | | |-> PC_Q35_COMPAT_2_0:
| | | | |-> PC_COMPAT_2_0:
| | | | | `-> [pc-2.0 vars]
| | | | `-> [pc-q35-2.0 vars]
| | | |-> PC_COMPAT_1_7:
| | | | `-> [pc-1.7 vars]
| | | `-> [pc-q35-1.7 vars]
| | |-> PC_COMPAT_1_6:
| | | `-> [pc-1.6 vars]
| | `-> [pc-q35-1.6 vars]
| |-> PC_COMPAT_1_5:
| | `-> [pc-1.5 vars]
| `-> [pc-q35-1.5 vars]
|-> PC_COMPAT_1_4:
| `-> [pc-1.4 vars]
`-> [pc-q35-1.4 vars]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/pc_piix.c | 31 +++++++++++++++++++++++++------
hw/i386/pc_q35.c | 14 +++++++-------
include/hw/i386/pc.h | 4 ----
3 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 47546b7..d8010d16 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -441,6 +441,9 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
.is_default = 1,
};
+#define PC_I440FX_COMPAT_2_0 \
+ PC_COMPAT_2_0
+
#define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
static QEMUMachine pc_i440fx_machine_v2_0 = {
@@ -448,11 +451,15 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
.name = "pc-i440fx-2.0",
.init = pc_init_pci_2_0,
.compat_props = (GlobalProperty[]) {
- PC_COMPAT_2_0,
+ PC_I440FX_COMPAT_2_0,
{ /* end of list */ }
},
};
+#define PC_I440FX_COMPAT_1_7 \
+ PC_I440FX_COMPAT_2_0, \
+ PC_COMPAT_1_7
+
#define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
static QEMUMachine pc_i440fx_machine_v1_7 = {
@@ -460,11 +467,15 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
.name = "pc-i440fx-1.7",
.init = pc_init_pci_1_7,
.compat_props = (GlobalProperty[]) {
- PC_COMPAT_1_7,
+ PC_I440FX_COMPAT_1_7,
{ /* end of list */ }
},
};
+#define PC_I440FX_COMPAT_1_6 \
+ PC_I440FX_COMPAT_1_7, \
+ PC_COMPAT_1_6
+
#define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
static QEMUMachine pc_i440fx_machine_v1_6 = {
@@ -472,21 +483,29 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
.name = "pc-i440fx-1.6",
.init = pc_init_pci_1_6,
.compat_props = (GlobalProperty[]) {
- PC_COMPAT_1_6,
+ PC_I440FX_COMPAT_1_6,
{ /* end of list */ }
},
};
+#define PC_I440FX_COMPAT_1_5 \
+ PC_I440FX_COMPAT_1_6, \
+ PC_COMPAT_1_5
+
static QEMUMachine pc_i440fx_machine_v1_5 = {
PC_I440FX_1_6_MACHINE_OPTIONS,
.name = "pc-i440fx-1.5",
.init = pc_init_pci_1_5,
.compat_props = (GlobalProperty[]) {
- PC_COMPAT_1_5,
+ PC_I440FX_COMPAT_1_5,
{ /* end of list */ }
},
};
+#define PC_I440FX_COMPAT_1_4 \
+ PC_I440FX_COMPAT_1_5, \
+ PC_COMPAT_1_4
+
#define PC_I440FX_1_4_MACHINE_OPTIONS \
PC_I440FX_1_6_MACHINE_OPTIONS, \
.hot_add_cpu = NULL
@@ -496,13 +515,13 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
.name = "pc-i440fx-1.4",
.init = pc_init_pci_1_4,
.compat_props = (GlobalProperty[]) {
- PC_COMPAT_1_4,
+ PC_I440FX_COMPAT_1_4,
{ /* end of list */ }
},
};
#define PC_COMPAT_1_3 \
- PC_COMPAT_1_4, \
+ PC_I440FX_COMPAT_1_4, \
{\
.driver = "usb-tablet",\
.property = "usb_version",\
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c640e7b..57eecc3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -383,8 +383,8 @@ static QEMUMachine pc_q35_machine_v2_0 = {
};
#define PC_Q35_COMPAT_1_7 \
- PC_COMPAT_1_7, \
PC_Q35_COMPAT_2_0, \
+ PC_COMPAT_1_7, \
{\
.driver = "hpet",\
.property = HPET_INTCAP,\
@@ -404,8 +404,8 @@ static QEMUMachine pc_q35_machine_v1_7 = {
};
#define PC_Q35_COMPAT_1_6 \
- PC_COMPAT_1_6, \
- PC_Q35_COMPAT_1_7
+ PC_Q35_COMPAT_1_7, \
+ PC_COMPAT_1_6
#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
@@ -420,8 +420,8 @@ static QEMUMachine pc_q35_machine_v1_6 = {
};
#define PC_Q35_COMPAT_1_5 \
- PC_COMPAT_1_5, \
- PC_Q35_COMPAT_1_6
+ PC_Q35_COMPAT_1_6, \
+ PC_COMPAT_1_5
static QEMUMachine pc_q35_machine_v1_5 = {
PC_Q35_1_6_MACHINE_OPTIONS,
@@ -434,8 +434,8 @@ static QEMUMachine pc_q35_machine_v1_5 = {
};
#define PC_Q35_COMPAT_1_4 \
- PC_COMPAT_1_4, \
- PC_Q35_COMPAT_1_5
+ PC_Q35_COMPAT_1_5, \
+ PC_COMPAT_1_4
#define PC_Q35_1_4_MACHINE_OPTIONS \
PC_Q35_1_6_MACHINE_OPTIONS, \
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fb7d68d..d988b80 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -336,7 +336,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_7 \
- PC_COMPAT_2_0, \
{\
.driver = TYPE_USB_DEVICE,\
.property = "msos-desc",\
@@ -349,7 +348,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_6 \
- PC_COMPAT_1_7, \
{\
.driver = "e1000",\
.property = "mitigation",\
@@ -373,7 +371,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_5 \
- PC_COMPAT_1_6, \
{\
.driver = "Conroe-" TYPE_X86_CPU,\
.property = "model",\
@@ -417,7 +414,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_4 \
- PC_COMPAT_1_5, \
{\
.driver = "scsi-hd",\
.property = "discard_granularity",\
--
1.9.3
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros
2014-06-24 18:02 ` [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros Eduardo Habkost
@ 2014-06-25 5:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:13 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 03:02:02PM -0300, Eduardo Habkost wrote:
> The PC_COMPAT_* macros include each other, and the PC_Q35_COMPAT_*
> macros _also_ included each other. That caused lots of properties to
> appear multiple times on PC_Q35_COMPAT_*.
>
> For example, PC_Q35_COMPAT_1_4 expanded to the following:
>
> PC_Q35_COMPAT_1_4:
> |-> PC_COMPAT_1_4:
> | |-> PC_COMPAT_1_5:
> | | |-> PC_COMPAT_1_6:
> | | | |-> PC_COMPAT_1_7:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-1.7 vars]
> | | | `-> [pc-1.6 vars]
> | | `-> [pc-1.5 vars]
> | `-> [pc-1.4 vars]
> |-> PC_Q35_COMPAT_1_5:
> | |-> PC_COMPAT_1_5:
> | | |-> PC_COMPAT_1_6:
> | | | |-> PC_COMPAT_1_7:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-1.7 vars]
> | | | `-> [pc-1.6 vars]
> | | `-> [pc-1.5 vars]
> | |-> PC_Q35_COMPAT_1_6:
> | | |-> PC_COMPAT_1_6:
> | | | |-> PC_COMPAT_1_7:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-1.7 vars]
> | | | `-> [pc-1.6 vars]
> | | |-> PC_Q35_COMPAT_1_7:
> | | | |-> PC_COMPAT_1_7:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-1.7 vars]
> | | | |-> PC_Q35_COMPAT_2_0:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-q35-2.0 vars]
> | | | `-> [pc-q35-1.7 vars]
> | | `-> [pc-q35-1.6 vars]
> | `-> [pc-q35-1.5 vars]
> `-> [pc-q35-1.4 vars]
>
> In other words, the pc-2.0 variables from PC_COMPAT_2_0 appeared 5 times
> inside PC_Q35_COMPAT_1_4.
>
> This makes the following changes:
>
> * Reusable PC_COMPAT_* macros on pc.h have NO nesting;
> * Instead of using PC_COMPAT_* directly, now pc_piix.c has its own
> PC_I440FX_COMPAT_* macros.
> * Machine-type-specific PC_*_COMPAT_* macros on .c files are
> nested, and reuse the common PC_COMPAT_* macros.
>
> Thus the expansion of PC_Q35_COMPAT_1_4 becomes:
>
> PC_Q35_COMPAT_1_4:
> |-> PC_Q35_COMPAT_1_5:
> | |-> PC_Q35_COMPAT_1_6:
> | | |-> PC_Q35_COMPAT_1_7:
> | | | |-> PC_Q35_COMPAT_2_0:
> | | | | |-> PC_COMPAT_2_0:
> | | | | | `-> [pc-2.0 vars]
> | | | | `-> [pc-q35-2.0 vars]
> | | | |-> PC_COMPAT_1_7:
> | | | | `-> [pc-1.7 vars]
> | | | `-> [pc-q35-1.7 vars]
> | | |-> PC_COMPAT_1_6:
> | | | `-> [pc-1.6 vars]
> | | `-> [pc-q35-1.6 vars]
> | |-> PC_COMPAT_1_5:
> | | `-> [pc-1.5 vars]
> | `-> [pc-q35-1.5 vars]
> |-> PC_COMPAT_1_4:
> | `-> [pc-1.4 vars]
> `-> [pc-q35-1.4 vars]
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/i386/pc_piix.c | 31 +++++++++++++++++++++++++------
> hw/i386/pc_q35.c | 14 +++++++-------
> include/hw/i386/pc.h | 4 ----
> 3 files changed, 32 insertions(+), 17 deletions(-)
More boiler-plate to add with each new machine, yeh.
How about this instead:
#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",\
}
static QEMUMachine pc_q35_machine_v2_0 = {
PC_Q35_2_0_MACHINE_OPTIONS,
.name = "pc-q35-2.0",
.init = pc_q35_init_2_0,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_2_0,
PC_Q35_COMPAT_2_0,
{ /* end of list */ }
},
};
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 47546b7..d8010d16 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -441,6 +441,9 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
> .is_default = 1,
> };
>
> +#define PC_I440FX_COMPAT_2_0 \
> + PC_COMPAT_2_0
> +
> #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
>
> static QEMUMachine pc_i440fx_machine_v2_0 = {
> @@ -448,11 +451,15 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
> .name = "pc-i440fx-2.0",
> .init = pc_init_pci_2_0,
> .compat_props = (GlobalProperty[]) {
> - PC_COMPAT_2_0,
> + PC_I440FX_COMPAT_2_0,
> { /* end of list */ }
> },
> };
>
> +#define PC_I440FX_COMPAT_1_7 \
> + PC_I440FX_COMPAT_2_0, \
> + PC_COMPAT_1_7
> +
> #define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
> static QEMUMachine pc_i440fx_machine_v1_7 = {
> @@ -460,11 +467,15 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
> .name = "pc-i440fx-1.7",
> .init = pc_init_pci_1_7,
> .compat_props = (GlobalProperty[]) {
> - PC_COMPAT_1_7,
> + PC_I440FX_COMPAT_1_7,
> { /* end of list */ }
> },
> };
>
> +#define PC_I440FX_COMPAT_1_6 \
> + PC_I440FX_COMPAT_1_7, \
> + PC_COMPAT_1_6
> +
> #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
> static QEMUMachine pc_i440fx_machine_v1_6 = {
> @@ -472,21 +483,29 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> .name = "pc-i440fx-1.6",
> .init = pc_init_pci_1_6,
> .compat_props = (GlobalProperty[]) {
> - PC_COMPAT_1_6,
> + PC_I440FX_COMPAT_1_6,
> { /* end of list */ }
> },
> };
>
> +#define PC_I440FX_COMPAT_1_5 \
> + PC_I440FX_COMPAT_1_6, \
> + PC_COMPAT_1_5
> +
> static QEMUMachine pc_i440fx_machine_v1_5 = {
> PC_I440FX_1_6_MACHINE_OPTIONS,
> .name = "pc-i440fx-1.5",
> .init = pc_init_pci_1_5,
> .compat_props = (GlobalProperty[]) {
> - PC_COMPAT_1_5,
> + PC_I440FX_COMPAT_1_5,
> { /* end of list */ }
> },
> };
>
> +#define PC_I440FX_COMPAT_1_4 \
> + PC_I440FX_COMPAT_1_5, \
> + PC_COMPAT_1_4
> +
> #define PC_I440FX_1_4_MACHINE_OPTIONS \
> PC_I440FX_1_6_MACHINE_OPTIONS, \
> .hot_add_cpu = NULL
> @@ -496,13 +515,13 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
> .name = "pc-i440fx-1.4",
> .init = pc_init_pci_1_4,
> .compat_props = (GlobalProperty[]) {
> - PC_COMPAT_1_4,
> + PC_I440FX_COMPAT_1_4,
> { /* end of list */ }
> },
> };
>
> #define PC_COMPAT_1_3 \
> - PC_COMPAT_1_4, \
> + PC_I440FX_COMPAT_1_4, \
> {\
> .driver = "usb-tablet",\
> .property = "usb_version",\
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c640e7b..57eecc3 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -383,8 +383,8 @@ static QEMUMachine pc_q35_machine_v2_0 = {
> };
>
> #define PC_Q35_COMPAT_1_7 \
> - PC_COMPAT_1_7, \
> PC_Q35_COMPAT_2_0, \
> + PC_COMPAT_1_7, \
> {\
> .driver = "hpet",\
> .property = HPET_INTCAP,\
> @@ -404,8 +404,8 @@ static QEMUMachine pc_q35_machine_v1_7 = {
> };
>
> #define PC_Q35_COMPAT_1_6 \
> - PC_COMPAT_1_6, \
> - PC_Q35_COMPAT_1_7
> + PC_Q35_COMPAT_1_7, \
> + PC_COMPAT_1_6
>
> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> @@ -420,8 +420,8 @@ static QEMUMachine pc_q35_machine_v1_6 = {
> };
>
> #define PC_Q35_COMPAT_1_5 \
> - PC_COMPAT_1_5, \
> - PC_Q35_COMPAT_1_6
> + PC_Q35_COMPAT_1_6, \
> + PC_COMPAT_1_5
>
> static QEMUMachine pc_q35_machine_v1_5 = {
> PC_Q35_1_6_MACHINE_OPTIONS,
> @@ -434,8 +434,8 @@ static QEMUMachine pc_q35_machine_v1_5 = {
> };
>
> #define PC_Q35_COMPAT_1_4 \
> - PC_COMPAT_1_4, \
> - PC_Q35_COMPAT_1_5
> + PC_Q35_COMPAT_1_5, \
> + PC_COMPAT_1_4
>
> #define PC_Q35_1_4_MACHINE_OPTIONS \
> PC_Q35_1_6_MACHINE_OPTIONS, \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fb7d68d..d988b80 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -336,7 +336,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_7 \
> - PC_COMPAT_2_0, \
> {\
> .driver = TYPE_USB_DEVICE,\
> .property = "msos-desc",\
> @@ -349,7 +348,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_6 \
> - PC_COMPAT_1_7, \
> {\
> .driver = "e1000",\
> .property = "mitigation",\
> @@ -373,7 +371,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_5 \
> - PC_COMPAT_1_6, \
> {\
> .driver = "Conroe-" TYPE_X86_CPU,\
> .property = "model",\
> @@ -417,7 +414,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_4 \
> - PC_COMPAT_1_5, \
> {\
> .driver = "scsi-hd",\
> .property = "discard_granularity",\
> --
> 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-24 18:02 ` [Qemu-devel] [PATCH 1/4] q35: Move q35-specific compat macros to pc_q35.c Eduardo Habkost
2014-06-24 18:02 ` [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros Eduardo Habkost
@ 2014-06-24 18:02 ` Eduardo Habkost
2014-06-24 19:20 ` Marcel Apfelbaum
` (2 more replies)
2014-06-24 18:02 ` [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props Eduardo Habkost
` (3 subsequent siblings)
6 siblings, 3 replies; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 18:02 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
The QEMU_COMPAT_* macros will contain compat properties that are not
specific to PC, and may be reused by other machine-types.
PC-specific properties were left on the PC_COMPAT_* macros.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 150 ++---------------------------------------------
2 files changed, 166 insertions(+), 145 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 605a970..709b582 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -134,4 +134,165 @@ struct MachineState {
const char *cpu_model;
};
+
+/* Macros for compat_props corresponding to specific QEMU versions: */
+
+#define QEMU_COMPAT_2_0 \
+ {\
+ .driver = "virtio-scsi-pci",\
+ .property = "any_layout",\
+ .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 = "prof_if",\
+ .value = stringify(0),\
+ },\
+ {\
+ .driver = "pci-serial-4x",\
+ .property = "prog_if",\
+ .value = stringify(0),\
+ },\
+ {\
+ .driver = "virtio-net-pci",\
+ .property = "guest_announce",\
+ .value = "off",\
+ }
+
+#define QEMU_COMPAT_1_7 \
+ {\
+ .driver = TYPE_USB_DEVICE,\
+ .property = "msos-desc",\
+ .value = "no",\
+ }
+
+#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),\
+ }
+
+#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",\
+ }
+
+#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 d988b80..631afe7 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -295,52 +295,15 @@ int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_COMPAT_2_0 \
+ 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 = "prof_if",\
- .value = stringify(0),\
- },\
- {\
- .driver = "pci-serial-4x",\
- .property = "prog_if",\
- .value = stringify(0),\
- },\
- {\
- .driver = "virtio-net-pci",\
- .property = "guest_announce",\
- .value = "off",\
}
#define PC_COMPAT_1_7 \
- {\
- .driver = TYPE_USB_DEVICE,\
- .property = "msos-desc",\
- .value = "no",\
- },\
+ QEMU_COMPAT_1_7,\
{\
.driver = "PIIX4_PM",\
.property = "acpi-pci-hotplug-with-bridge-support",\
@@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_6 \
+ 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),\
@@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_COMPAT_1_5 \
+ QEMU_COMPAT_1_7,\
{\
- .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),\
@@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
}
#define PC_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),\
- }
+ QEMU_COMPAT_1_4
#define PC_COMMON_MACHINE_OPTIONS \
.default_boot_order = "cad"
--
1.9.3
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 18:02 ` [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-24 19:20 ` Marcel Apfelbaum
2014-06-25 6:20 ` Igor Mammedov
2014-06-24 20:58 ` BALATON Zoltan
2014-06-25 5:20 ` Michael S. Tsirkin
2 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2014-06-24 19:20 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Michael S. Tsirkin, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Tue, 2014-06-24 at 15:02 -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.
>
> PC-specific properties were left on the PC_COMPAT_* macros.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 150 ++---------------------------------------------
> 2 files changed, 166 insertions(+), 145 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 605a970..709b582 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -134,4 +134,165 @@ struct MachineState {
> const char *cpu_model;
> };
>
> +
> +/* Macros for compat_props corresponding to specific QEMU versions: */
Only a suggestion: Maybe we can move all the compat props to a new header file,
say include/hw/compat.h so we don't need to modify board.h every time we need a
compat prop? board.h will remain cleaner this way.
Thanks,
Marcel
> +
> +#define QEMU_COMPAT_2_0 \
> + {\
> + .driver = "virtio-scsi-pci",\
> + .property = "any_layout",\
> + .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 = "prof_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "pci-serial-4x",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "virtio-net-pci",\
> + .property = "guest_announce",\
> + .value = "off",\
> + }
> +
> +#define QEMU_COMPAT_1_7 \
> + {\
> + .driver = TYPE_USB_DEVICE,\
> + .property = "msos-desc",\
> + .value = "no",\
> + }
> +
> +#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),\
> + }
> +
> +#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",\
> + }
> +
> +#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 d988b80..631afe7 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> #define PC_COMPAT_2_0 \
> + 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 = "prof_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "pci-serial-4x",\
> - .property = "prog_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "virtio-net-pci",\
> - .property = "guest_announce",\
> - .value = "off",\
> }
>
> #define PC_COMPAT_1_7 \
> - {\
> - .driver = TYPE_USB_DEVICE,\
> - .property = "msos-desc",\
> - .value = "no",\
> - },\
> + QEMU_COMPAT_1_7,\
> {\
> .driver = "PIIX4_PM",\
> .property = "acpi-pci-hotplug-with-bridge-support",\
> @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_6 \
> + 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),\
> @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_5 \
> + QEMU_COMPAT_1_7,\
> {\
> - .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),\
> @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_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),\
> - }
> + QEMU_COMPAT_1_4
>
> #define PC_COMMON_MACHINE_OPTIONS \
> .default_boot_order = "cad"
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 19:20 ` Marcel Apfelbaum
@ 2014-06-25 6:20 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2014-06-25 6:20 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Markus Armbruster, qemu-devel,
Paul Mackerras, Anthony Liguori, Paolo Bonzini,
Andreas Färber, Alexander Graf
On Tue, 24 Jun 2014 22:20:50 +0300
Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> On Tue, 2014-06-24 at 15:02 -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.
> >
> > PC-specific properties were left on the PC_COMPAT_* macros.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 150 ++---------------------------------------------
> > 2 files changed, 166 insertions(+), 145 deletions(-)
> >
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 605a970..709b582 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -134,4 +134,165 @@ struct MachineState {
> > const char *cpu_model;
> > };
> >
> > +
> > +/* Macros for compat_props corresponding to specific QEMU versions: */
> Only a suggestion: Maybe we can move all the compat props to a new header file,
> say include/hw/compat.h so we don't need to modify board.h every time we need a
> compat prop? board.h will remain cleaner this way.
these are PC specific compat props, so it should be moved into target specific
pc.h instead of global header.
> Thanks,
> Marcel
>
> > +
> > +#define QEMU_COMPAT_2_0 \
> > + {\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "any_layout",\
> > + .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 = "prof_if",\
> > + .value = stringify(0),\
> > + },\
> > + {\
> > + .driver = "pci-serial-4x",\
> > + .property = "prog_if",\
> > + .value = stringify(0),\
> > + },\
> > + {\
> > + .driver = "virtio-net-pci",\
> > + .property = "guest_announce",\
> > + .value = "off",\
> > + }
> > +
> > +#define QEMU_COMPAT_1_7 \
> > + {\
> > + .driver = TYPE_USB_DEVICE,\
> > + .property = "msos-desc",\
> > + .value = "no",\
> > + }
> > +
> > +#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),\
> > + }
> > +
> > +#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",\
> > + }
> > +
> > +#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 d988b80..631afe7 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >
> > #define PC_COMPAT_2_0 \
> > + 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 = "prof_if",\
> > - .value = stringify(0),\
> > - },\
> > - {\
> > - .driver = "pci-serial-4x",\
> > - .property = "prog_if",\
> > - .value = stringify(0),\
> > - },\
> > - {\
> > - .driver = "virtio-net-pci",\
> > - .property = "guest_announce",\
> > - .value = "off",\
> > }
> >
> > #define PC_COMPAT_1_7 \
> > - {\
> > - .driver = TYPE_USB_DEVICE,\
> > - .property = "msos-desc",\
> > - .value = "no",\
> > - },\
> > + QEMU_COMPAT_1_7,\
> > {\
> > .driver = "PIIX4_PM",\
> > .property = "acpi-pci-hotplug-with-bridge-support",\
> > @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_COMPAT_1_6 \
> > + 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),\
> > @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_COMPAT_1_5 \
> > + QEMU_COMPAT_1_7,\
> > {\
> > - .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),\
> > @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_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),\
> > - }
> > + QEMU_COMPAT_1_4
> >
> > #define PC_COMMON_MACHINE_OPTIONS \
> > .default_boot_order = "cad"
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 18:02 ` [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
2014-06-24 19:20 ` Marcel Apfelbaum
@ 2014-06-24 20:58 ` BALATON Zoltan
2014-06-24 23:01 ` Eduardo Habkost
2014-06-25 5:20 ` Michael S. Tsirkin
2 siblings, 1 reply; 25+ messages in thread
From: BALATON Zoltan @ 2014-06-24 20:58 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Marcel Apfelbaum, Michael S. Tsirkin, qemu-devel,
Anthony Liguori, Paolo Bonzini
On Tue, 24 Jun 2014, 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.
>
> PC-specific properties were left on the PC_COMPAT_* macros.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 150 ++---------------------------------------------
> 2 files changed, 166 insertions(+), 145 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 605a970..709b582 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -134,4 +134,165 @@ struct MachineState {
> const char *cpu_model;
> };
>
> +
> +/* Macros for compat_props corresponding to specific QEMU versions: */
> +
> +#define QEMU_COMPAT_2_0 \
> + {\
> + .driver = "virtio-scsi-pci",\
> + .property = "any_layout",\
> + .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 = "prof_if",\
Just noticed a typo now in this part which was added by me previously.
This should also be "prog_if" instead of "prof_if". Can you also correct
that while changing this or otherwise while merging or should I send a
separate patch with just this change?
Regards,
BALATON Zoltan
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "pci-serial-4x",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "virtio-net-pci",\
> + .property = "guest_announce",\
> + .value = "off",\
> + }
> +
> +#define QEMU_COMPAT_1_7 \
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 20:58 ` BALATON Zoltan
@ 2014-06-24 23:01 ` Eduardo Habkost
0 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 23:01 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Peter Maydell, Marcel Apfelbaum, Michael S. Tsirkin, qemu-devel,
Anthony Liguori, Paolo Bonzini
On Tue, Jun 24, 2014 at 10:58:29PM +0200, BALATON Zoltan wrote:
> On Tue, 24 Jun 2014, Eduardo Habkost wrote:
[...]
> >+ .driver = "pci-serial",\
> >+ .property = "prog_if",\
> >+ .value = stringify(0),\
> >+ },\
> >+ {\
> >+ .driver = "pci-serial-2x",\
> >+ .property = "prof_if",\
>
> Just noticed a typo now in this part which was added by me previously. This
> should also be "prog_if" instead of "prof_if". Can you also correct that
> while changing this or otherwise while merging or should I send a separate
> patch with just this change?
I just sent a separate patch for that. I prefer not to change code while
moving it.
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-24 18:02 ` [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
2014-06-24 19:20 ` Marcel Apfelbaum
2014-06-24 20:58 ` BALATON Zoltan
@ 2014-06-25 5:20 ` Michael S. Tsirkin
2014-06-25 6:46 ` Marcel Apfelbaum
2 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:20 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 03:02:03PM -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.
>
> PC-specific properties were left on the PC_COMPAT_* macros.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 150 ++---------------------------------------------
> 2 files changed, 166 insertions(+), 145 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 605a970..709b582 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -134,4 +134,165 @@ struct MachineState {
> const char *cpu_model;
> };
>
> +
> +/* Macros for compat_props corresponding to specific QEMU versions: */
> +
> +#define QEMU_COMPAT_2_0 \
> + {\
> + .driver = "virtio-scsi-pci",\
> + .property = "any_layout",\
> + .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 = "prof_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "pci-serial-4x",\
> + .property = "prog_if",\
> + .value = stringify(0),\
> + },\
> + {\
> + .driver = "virtio-net-pci",\
> + .property = "guest_announce",\
> + .value = "off",\
> + }
> +
> +#define QEMU_COMPAT_1_7 \
> + {\
> + .driver = TYPE_USB_DEVICE,\
> + .property = "msos-desc",\
> + .value = "no",\
> + }
> +
> +#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),\
> + }
> +
> +#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),\
Above are x86 CPUs - why have them in a common header?
There's no chance any machine except PIIX&Q35 needs these.
> + .driver = "virtio-net-pci",\
> + .property = "any_layout",\
> + .value = "off",\
Here's an example.
If you are using a non x86 target, QEMU 2.0
has no way to select a machine with this
value.
So why expose it?
> + },{\
> + .driver = TYPE_X86_CPU,\
> + .property = "pmu",\
> + .value = "on",\
> + }
> +
> +#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 d988b80..631afe7 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> #define PC_COMPAT_2_0 \
> + 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 = "prof_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "pci-serial-4x",\
> - .property = "prog_if",\
> - .value = stringify(0),\
> - },\
> - {\
> - .driver = "virtio-net-pci",\
> - .property = "guest_announce",\
> - .value = "off",\
> }
>
> #define PC_COMPAT_1_7 \
> - {\
> - .driver = TYPE_USB_DEVICE,\
> - .property = "msos-desc",\
> - .value = "no",\
> - },\
> + QEMU_COMPAT_1_7,\
> {\
> .driver = "PIIX4_PM",\
> .property = "acpi-pci-hotplug-with-bridge-support",\
> @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_6 \
> + 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),\
> @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_COMPAT_1_5 \
> + QEMU_COMPAT_1_7,\
> {\
> - .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),\
> @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> }
>
> #define PC_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),\
> - }
> + QEMU_COMPAT_1_4
>
> #define PC_COMMON_MACHINE_OPTIONS \
> .default_boot_order = "cad"
> --
> 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 5:20 ` Michael S. Tsirkin
@ 2014-06-25 6:46 ` Marcel Apfelbaum
2014-06-25 7:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2014-06-25 6:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Eduardo Habkost, Alexey Kardashevskiy,
Markus Armbruster, qemu-devel, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 24, 2014 at 03:02:03PM -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.
> >
> > PC-specific properties were left on the PC_COMPAT_* macros.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 150 ++---------------------------------------------
> > 2 files changed, 166 insertions(+), 145 deletions(-)
> >
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 605a970..709b582 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -134,4 +134,165 @@ struct MachineState {
> > const char *cpu_model;
> > };
> >
> > +
> > +/* Macros for compat_props corresponding to specific QEMU versions: */
> > +
> > +#define QEMU_COMPAT_2_0 \
> > + {\
> > + .driver = "virtio-scsi-pci",\
> > + .property = "any_layout",\
> > + .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 = "prof_if",\
> > + .value = stringify(0),\
> > + },\
> > + {\
> > + .driver = "pci-serial-4x",\
> > + .property = "prog_if",\
> > + .value = stringify(0),\
> > + },\
> > + {\
> > + .driver = "virtio-net-pci",\
> > + .property = "guest_announce",\
> > + .value = "off",\
> > + }
> > +
> > +#define QEMU_COMPAT_1_7 \
> > + {\
> > + .driver = TYPE_USB_DEVICE,\
> > + .property = "msos-desc",\
> > + .value = "no",\
> > + }
> > +
> > +#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),\
> > + }
> > +
> > +#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),\
>
> Above are x86 CPUs - why have them in a common header?
> There's no chance any machine except PIIX&Q35 needs these.
>
> > + .driver = "virtio-net-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
>
> Here's an example.
> If you are using a non x86 target, QEMU 2.0
> has no way to select a machine with this
> value.
> So why expose it?
As we talked on the KVM call, the objective is to assign the
compat properties per QEMU version and close to their device.
As a *middle* step we shall put all together in a common file
per version and not per machine type, and then work on a registration
mechanism that will be based on the fact that compat properties
are per device and versioned by QEMU.
Thanks,
Marcel
>
>
> > + },{\
> > + .driver = TYPE_X86_CPU,\
> > + .property = "pmu",\
> > + .value = "on",\
> > + }
> > +
> > +#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 d988b80..631afe7 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >
> > #define PC_COMPAT_2_0 \
> > + 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 = "prof_if",\
> > - .value = stringify(0),\
> > - },\
> > - {\
> > - .driver = "pci-serial-4x",\
> > - .property = "prog_if",\
> > - .value = stringify(0),\
> > - },\
> > - {\
> > - .driver = "virtio-net-pci",\
> > - .property = "guest_announce",\
> > - .value = "off",\
> > }
> >
> > #define PC_COMPAT_1_7 \
> > - {\
> > - .driver = TYPE_USB_DEVICE,\
> > - .property = "msos-desc",\
> > - .value = "no",\
> > - },\
> > + QEMU_COMPAT_1_7,\
> > {\
> > .driver = "PIIX4_PM",\
> > .property = "acpi-pci-hotplug-with-bridge-support",\
> > @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_COMPAT_1_6 \
> > + 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),\
> > @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_COMPAT_1_5 \
> > + QEMU_COMPAT_1_7,\
> > {\
> > - .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),\
> > @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > }
> >
> > #define PC_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),\
> > - }
> > + QEMU_COMPAT_1_4
> >
> > #define PC_COMMON_MACHINE_OPTIONS \
> > .default_boot_order = "cad"
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 6:46 ` Marcel Apfelbaum
@ 2014-06-25 7:18 ` Michael S. Tsirkin
2014-06-25 7:20 ` Marcel Apfelbaum
0 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 7:18 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: Peter Maydell, Eduardo Habkost, Alexey Kardashevskiy,
Markus Armbruster, qemu-devel, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, Jun 25, 2014 at 09:46:40AM +0300, Marcel Apfelbaum wrote:
> On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 24, 2014 at 03:02:03PM -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.
> > >
> > > PC-specific properties were left on the PC_COMPAT_* macros.
> > >
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > include/hw/i386/pc.h | 150 ++---------------------------------------------
> > > 2 files changed, 166 insertions(+), 145 deletions(-)
> > >
> > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > index 605a970..709b582 100644
> > > --- a/include/hw/boards.h
> > > +++ b/include/hw/boards.h
> > > @@ -134,4 +134,165 @@ struct MachineState {
> > > const char *cpu_model;
> > > };
> > >
> > > +
> > > +/* Macros for compat_props corresponding to specific QEMU versions: */
> > > +
> > > +#define QEMU_COMPAT_2_0 \
> > > + {\
> > > + .driver = "virtio-scsi-pci",\
> > > + .property = "any_layout",\
> > > + .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 = "prof_if",\
> > > + .value = stringify(0),\
> > > + },\
> > > + {\
> > > + .driver = "pci-serial-4x",\
> > > + .property = "prog_if",\
> > > + .value = stringify(0),\
> > > + },\
> > > + {\
> > > + .driver = "virtio-net-pci",\
> > > + .property = "guest_announce",\
> > > + .value = "off",\
> > > + }
> > > +
> > > +#define QEMU_COMPAT_1_7 \
> > > + {\
> > > + .driver = TYPE_USB_DEVICE,\
> > > + .property = "msos-desc",\
> > > + .value = "no",\
> > > + }
> > > +
> > > +#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),\
> > > + }
> > > +
> > > +#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),\
> >
> > Above are x86 CPUs - why have them in a common header?
> > There's no chance any machine except PIIX&Q35 needs these.
> >
> > > + .driver = "virtio-net-pci",\
> > > + .property = "any_layout",\
> > > + .value = "off",\
> >
> > Here's an example.
> > If you are using a non x86 target, QEMU 2.0
> > has no way to select a machine with this
> > value.
> > So why expose it?
> As we talked on the KVM call, the objective is to assign the
> compat properties per QEMU version and close to their device.
> As a *middle* step we shall put all together in a common file
> per version and not per machine type, and then work on a registration
> mechanism that will be based on the fact that compat properties
> are per device and versioned by QEMU.
>
> Thanks,
> Marcel
Yes but that'll stay on a branch and get merged after 2.1.
Let's keep this patch on that branch there as well.
> >
> >
> > > + },{\
> > > + .driver = TYPE_X86_CPU,\
> > > + .property = "pmu",\
> > > + .value = "on",\
> > > + }
> > > +
> > > +#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 d988b80..631afe7 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > >
> > > #define PC_COMPAT_2_0 \
> > > + 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 = "prof_if",\
> > > - .value = stringify(0),\
> > > - },\
> > > - {\
> > > - .driver = "pci-serial-4x",\
> > > - .property = "prog_if",\
> > > - .value = stringify(0),\
> > > - },\
> > > - {\
> > > - .driver = "virtio-net-pci",\
> > > - .property = "guest_announce",\
> > > - .value = "off",\
> > > }
> > >
> > > #define PC_COMPAT_1_7 \
> > > - {\
> > > - .driver = TYPE_USB_DEVICE,\
> > > - .property = "msos-desc",\
> > > - .value = "no",\
> > > - },\
> > > + QEMU_COMPAT_1_7,\
> > > {\
> > > .driver = "PIIX4_PM",\
> > > .property = "acpi-pci-hotplug-with-bridge-support",\
> > > @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > }
> > >
> > > #define PC_COMPAT_1_6 \
> > > + 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),\
> > > @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > }
> > >
> > > #define PC_COMPAT_1_5 \
> > > + QEMU_COMPAT_1_7,\
> > > {\
> > > - .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),\
> > > @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > }
> > >
> > > #define PC_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),\
> > > - }
> > > + QEMU_COMPAT_1_4
> > >
> > > #define PC_COMMON_MACHINE_OPTIONS \
> > > .default_boot_order = "cad"
> > > --
> > > 1.9.3
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
2014-06-25 7:18 ` Michael S. Tsirkin
@ 2014-06-25 7:20 ` Marcel Apfelbaum
0 siblings, 0 replies; 25+ messages in thread
From: Marcel Apfelbaum @ 2014-06-25 7:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Eduardo Habkost, Alexey Kardashevskiy,
Markus Armbruster, qemu-devel, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, 2014-06-25 at 10:18 +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 25, 2014 at 09:46:40AM +0300, Marcel Apfelbaum wrote:
> > On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Jun 24, 2014 at 03:02:03PM -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.
> > > >
> > > > PC-specific properties were left on the PC_COMPAT_* macros.
> > > >
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > > include/hw/boards.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > include/hw/i386/pc.h | 150 ++---------------------------------------------
> > > > 2 files changed, 166 insertions(+), 145 deletions(-)
> > > >
> > > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > > index 605a970..709b582 100644
> > > > --- a/include/hw/boards.h
> > > > +++ b/include/hw/boards.h
> > > > @@ -134,4 +134,165 @@ struct MachineState {
> > > > const char *cpu_model;
> > > > };
> > > >
> > > > +
> > > > +/* Macros for compat_props corresponding to specific QEMU versions: */
> > > > +
> > > > +#define QEMU_COMPAT_2_0 \
> > > > + {\
> > > > + .driver = "virtio-scsi-pci",\
> > > > + .property = "any_layout",\
> > > > + .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 = "prof_if",\
> > > > + .value = stringify(0),\
> > > > + },\
> > > > + {\
> > > > + .driver = "pci-serial-4x",\
> > > > + .property = "prog_if",\
> > > > + .value = stringify(0),\
> > > > + },\
> > > > + {\
> > > > + .driver = "virtio-net-pci",\
> > > > + .property = "guest_announce",\
> > > > + .value = "off",\
> > > > + }
> > > > +
> > > > +#define QEMU_COMPAT_1_7 \
> > > > + {\
> > > > + .driver = TYPE_USB_DEVICE,\
> > > > + .property = "msos-desc",\
> > > > + .value = "no",\
> > > > + }
> > > > +
> > > > +#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),\
> > > > + }
> > > > +
> > > > +#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),\
> > >
> > > Above are x86 CPUs - why have them in a common header?
> > > There's no chance any machine except PIIX&Q35 needs these.
> > >
> > > > + .driver = "virtio-net-pci",\
> > > > + .property = "any_layout",\
> > > > + .value = "off",\
> > >
> > > Here's an example.
> > > If you are using a non x86 target, QEMU 2.0
> > > has no way to select a machine with this
> > > value.
> > > So why expose it?
> > As we talked on the KVM call, the objective is to assign the
> > compat properties per QEMU version and close to their device.
> > As a *middle* step we shall put all together in a common file
> > per version and not per machine type, and then work on a registration
> > mechanism that will be based on the fact that compat properties
> > are per device and versioned by QEMU.
> >
> > Thanks,
> > Marcel
>
> Yes but that'll stay on a branch and get merged after 2.1.
> Let's keep this patch on that branch there as well.
Fine by me
Thanks,
Marcel
>
>
> > >
> > >
> > > > + },{\
> > > > + .driver = TYPE_X86_CPU,\
> > > > + .property = "pmu",\
> > > > + .value = "on",\
> > > > + }
> > > > +
> > > > +#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 d988b80..631afe7 100644
> > > > --- a/include/hw/i386/pc.h
> > > > +++ b/include/hw/i386/pc.h
> > > > @@ -295,52 +295,15 @@ int e820_get_num_entries(void);
> > > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > >
> > > > #define PC_COMPAT_2_0 \
> > > > + 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 = "prof_if",\
> > > > - .value = stringify(0),\
> > > > - },\
> > > > - {\
> > > > - .driver = "pci-serial-4x",\
> > > > - .property = "prog_if",\
> > > > - .value = stringify(0),\
> > > > - },\
> > > > - {\
> > > > - .driver = "virtio-net-pci",\
> > > > - .property = "guest_announce",\
> > > > - .value = "off",\
> > > > }
> > > >
> > > > #define PC_COMPAT_1_7 \
> > > > - {\
> > > > - .driver = TYPE_USB_DEVICE,\
> > > > - .property = "msos-desc",\
> > > > - .value = "no",\
> > > > - },\
> > > > + QEMU_COMPAT_1_7,\
> > > > {\
> > > > .driver = "PIIX4_PM",\
> > > > .property = "acpi-pci-hotplug-with-bridge-support",\
> > > > @@ -348,19 +311,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > > }
> > > >
> > > > #define PC_COMPAT_1_6 \
> > > > + 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),\
> > > > @@ -371,39 +323,8 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > > }
> > > >
> > > > #define PC_COMPAT_1_5 \
> > > > + QEMU_COMPAT_1_7,\
> > > > {\
> > > > - .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),\
> > > > @@ -414,68 +335,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > > > }
> > > >
> > > > #define PC_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),\
> > > > - }
> > > > + QEMU_COMPAT_1_4
> > > >
> > > > #define PC_COMMON_MACHINE_OPTIONS \
> > > > .default_boot_order = "cad"
> > > > --
> > > > 1.9.3
> >
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
` (2 preceding siblings ...)
2014-06-24 18:02 ` [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
@ 2014-06-24 18:02 ` Eduardo Habkost
2014-06-24 19:51 ` Marcel Apfelbaum
2014-06-25 4:55 ` Michael S. Tsirkin
2014-06-25 2:39 ` [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Alexey Kardashevskiy
` (2 subsequent siblings)
6 siblings, 2 replies; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 18:02 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
All compat properties are only applied if a device of an specific type
is instantiated. There's no need to keep a PC-specific list of compat
properties, as properties for PC-specific devices won't affect other
machine-types anyway.
This patch moves all compat_props properties from PC compat macros, to
QEMU_COMPAT_*.
In case somebody wonders why the nesting was kept on the
machine-type-specific PC_*_COMPAT_* macros instead of making
QEMU_COMPAT_* macros nested. The answer is that making QEMU_COMPAT_*
nested would make it more difficult to migrate the existing machine-type
code to QOM later using class_init reuse. See:
https://github.com/ehabkost/qemu-hacks/commit/9edb1f70fc2bd74131f54a2dfed1e890a5859736
PC_COMPAT_1_3 and older are being left untouched by now, as they still
need some additional cleanup and are unlikely to be reused by new
machine-types anyway.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/i386/pc_piix.c | 10 +++++-----
hw/i386/pc_q35.c | 28 +++++-----------------------
include/hw/boards.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 43 -------------------------------------------
4 files changed, 56 insertions(+), 71 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d8010d16..5e3d24d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -442,7 +442,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
};
#define PC_I440FX_COMPAT_2_0 \
- PC_COMPAT_2_0
+ QEMU_COMPAT_2_0
#define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
@@ -458,7 +458,7 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
#define PC_I440FX_COMPAT_1_7 \
PC_I440FX_COMPAT_2_0, \
- PC_COMPAT_1_7
+ QEMU_COMPAT_1_7
#define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
@@ -474,7 +474,7 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
#define PC_I440FX_COMPAT_1_6 \
PC_I440FX_COMPAT_1_7, \
- PC_COMPAT_1_6
+ QEMU_COMPAT_1_6
#define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
@@ -490,7 +490,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
#define PC_I440FX_COMPAT_1_5 \
PC_I440FX_COMPAT_1_6, \
- PC_COMPAT_1_5
+ QEMU_COMPAT_1_5
static QEMUMachine pc_i440fx_machine_v1_5 = {
PC_I440FX_1_6_MACHINE_OPTIONS,
@@ -504,7 +504,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
#define PC_I440FX_COMPAT_1_4 \
PC_I440FX_COMPAT_1_5, \
- PC_COMPAT_1_4
+ QEMU_COMPAT_1_4
#define PC_I440FX_1_4_MACHINE_OPTIONS \
PC_I440FX_1_6_MACHINE_OPTIONS, \
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 57eecc3..b59d22d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -355,20 +355,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
};
#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",\
- }
+ QEMU_COMPAT_2_0
#define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
@@ -384,12 +371,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
#define PC_Q35_COMPAT_1_7 \
PC_Q35_COMPAT_2_0, \
- PC_COMPAT_1_7, \
- {\
- .driver = "hpet",\
- .property = HPET_INTCAP,\
- .value = stringify(4),\
- }
+ QEMU_COMPAT_1_7
#define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
@@ -405,7 +387,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
#define PC_Q35_COMPAT_1_6 \
PC_Q35_COMPAT_1_7, \
- PC_COMPAT_1_6
+ QEMU_COMPAT_1_6
#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
@@ -421,7 +403,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
#define PC_Q35_COMPAT_1_5 \
PC_Q35_COMPAT_1_6, \
- PC_COMPAT_1_5
+ QEMU_COMPAT_1_5
static QEMUMachine pc_q35_machine_v1_5 = {
PC_Q35_1_6_MACHINE_OPTIONS,
@@ -435,7 +417,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
#define PC_Q35_COMPAT_1_4 \
PC_Q35_COMPAT_1_5, \
- PC_COMPAT_1_4
+ QEMU_COMPAT_1_4
#define PC_Q35_1_4_MACHINE_OPTIONS \
PC_Q35_1_6_MACHINE_OPTIONS, \
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 709b582..fa7ceb7 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -172,6 +172,24 @@ struct MachineState {
.driver = "virtio-net-pci",\
.property = "guest_announce",\
.value = "off",\
+ },\
+ {\
+ .driver = "PIIX4_PM",\
+ .property = "memory-hotplug-support",\
+ .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 \
@@ -179,6 +197,16 @@ struct MachineState {
.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 \
@@ -194,6 +222,15 @@ struct MachineState {
.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 \
@@ -229,6 +266,15 @@ struct MachineState {
.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 \
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 631afe7..8503bee 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,49 +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_COMPAT_2_0 \
- QEMU_COMPAT_2_0,\
- {\
- .driver = "PIIX4_PM",\
- .property = "memory-hotplug-support",\
- .value = "off",\
- }
-
-#define PC_COMPAT_1_7 \
- QEMU_COMPAT_1_7,\
- {\
- .driver = "PIIX4_PM",\
- .property = "acpi-pci-hotplug-with-bridge-support",\
- .value = "off",\
- }
-
-#define PC_COMPAT_1_6 \
- QEMU_COMPAT_1_6,\
- {\
- .driver = "i440FX-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(1),\
- },{\
- .driver = "q35-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(1),\
- }
-
-#define PC_COMPAT_1_5 \
- QEMU_COMPAT_1_7,\
- {\
- .driver = "i440FX-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(0),\
- },{\
- .driver = "q35-pcihost",\
- .property = "short_root_bus",\
- .value = stringify(0),\
- }
-
-#define PC_COMPAT_1_4 \
- QEMU_COMPAT_1_4
-
#define PC_COMMON_MACHINE_OPTIONS \
.default_boot_order = "cad"
--
1.9.3
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-24 18:02 ` [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props Eduardo Habkost
@ 2014-06-24 19:51 ` Marcel Apfelbaum
2014-06-24 19:59 ` Eduardo Habkost
2014-06-25 4:55 ` Michael S. Tsirkin
1 sibling, 1 reply; 25+ messages in thread
From: Marcel Apfelbaum @ 2014-06-24 19:51 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Michael S. Tsirkin, Alexey Kardashevskiy,
qemu-devel, Markus Armbruster, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Tue, 2014-06-24 at 15:02 -0300, Eduardo Habkost wrote:
> All compat properties are only applied if a device of an specific type
> is instantiated. There's no need to keep a PC-specific list of compat
> properties, as properties for PC-specific devices won't affect other
> machine-types anyway.
So why do we still need 2 sets of hierarchies PC_I440FX_COMPAT_x_y and
PC_Q35_COMPAT_x_y? It seems that QEMU_COMPAT_x_y is enough.
Am I missing something?
Thanks,
Marcel
>
> This patch moves all compat_props properties from PC compat macros, to
> QEMU_COMPAT_*.
>
> In case somebody wonders why the nesting was kept on the
> machine-type-specific PC_*_COMPAT_* macros instead of making
> QEMU_COMPAT_* macros nested. The answer is that making QEMU_COMPAT_*
> nested would make it more difficult to migrate the existing machine-type
> code to QOM later using class_init reuse. See:
> https://github.com/ehabkost/qemu-hacks/commit/9edb1f70fc2bd74131f54a2dfed1e890a5859736
>
> PC_COMPAT_1_3 and older are being left untouched by now, as they still
> need some additional cleanup and are unlikely to be reused by new
> machine-types anyway.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/i386/pc_piix.c | 10 +++++-----
> hw/i386/pc_q35.c | 28 +++++-----------------------
> include/hw/boards.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 43 -------------------------------------------
> 4 files changed, 56 insertions(+), 71 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d8010d16..5e3d24d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -442,7 +442,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
> };
>
> #define PC_I440FX_COMPAT_2_0 \
> - PC_COMPAT_2_0
> + QEMU_COMPAT_2_0
>
> #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
>
> @@ -458,7 +458,7 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
>
> #define PC_I440FX_COMPAT_1_7 \
> PC_I440FX_COMPAT_2_0, \
> - PC_COMPAT_1_7
> + QEMU_COMPAT_1_7
>
> #define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
> @@ -474,7 +474,7 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
>
> #define PC_I440FX_COMPAT_1_6 \
> PC_I440FX_COMPAT_1_7, \
> - PC_COMPAT_1_6
> + QEMU_COMPAT_1_6
>
> #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
> @@ -490,7 +490,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>
> #define PC_I440FX_COMPAT_1_5 \
> PC_I440FX_COMPAT_1_6, \
> - PC_COMPAT_1_5
> + QEMU_COMPAT_1_5
>
> static QEMUMachine pc_i440fx_machine_v1_5 = {
> PC_I440FX_1_6_MACHINE_OPTIONS,
> @@ -504,7 +504,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>
> #define PC_I440FX_COMPAT_1_4 \
> PC_I440FX_COMPAT_1_5, \
> - PC_COMPAT_1_4
> + QEMU_COMPAT_1_4
>
> #define PC_I440FX_1_4_MACHINE_OPTIONS \
> PC_I440FX_1_6_MACHINE_OPTIONS, \
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 57eecc3..b59d22d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -355,20 +355,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
> };
>
> #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",\
> - }
> + QEMU_COMPAT_2_0
>
> #define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
>
> @@ -384,12 +371,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
>
> #define PC_Q35_COMPAT_1_7 \
> PC_Q35_COMPAT_2_0, \
> - PC_COMPAT_1_7, \
> - {\
> - .driver = "hpet",\
> - .property = HPET_INTCAP,\
> - .value = stringify(4),\
> - }
> + QEMU_COMPAT_1_7
>
> #define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> @@ -405,7 +387,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
>
> #define PC_Q35_COMPAT_1_6 \
> PC_Q35_COMPAT_1_7, \
> - PC_COMPAT_1_6
> + QEMU_COMPAT_1_6
>
> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> @@ -421,7 +403,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
>
> #define PC_Q35_COMPAT_1_5 \
> PC_Q35_COMPAT_1_6, \
> - PC_COMPAT_1_5
> + QEMU_COMPAT_1_5
>
> static QEMUMachine pc_q35_machine_v1_5 = {
> PC_Q35_1_6_MACHINE_OPTIONS,
> @@ -435,7 +417,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>
> #define PC_Q35_COMPAT_1_4 \
> PC_Q35_COMPAT_1_5, \
> - PC_COMPAT_1_4
> + QEMU_COMPAT_1_4
>
> #define PC_Q35_1_4_MACHINE_OPTIONS \
> PC_Q35_1_6_MACHINE_OPTIONS, \
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 709b582..fa7ceb7 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -172,6 +172,24 @@ struct MachineState {
> .driver = "virtio-net-pci",\
> .property = "guest_announce",\
> .value = "off",\
> + },\
> + {\
> + .driver = "PIIX4_PM",\
> + .property = "memory-hotplug-support",\
> + .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 \
> @@ -179,6 +197,16 @@ struct MachineState {
> .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 \
> @@ -194,6 +222,15 @@ struct MachineState {
> .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 \
> @@ -229,6 +266,15 @@ struct MachineState {
> .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 \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 631afe7..8503bee 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,49 +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_COMPAT_2_0 \
> - QEMU_COMPAT_2_0,\
> - {\
> - .driver = "PIIX4_PM",\
> - .property = "memory-hotplug-support",\
> - .value = "off",\
> - }
> -
> -#define PC_COMPAT_1_7 \
> - QEMU_COMPAT_1_7,\
> - {\
> - .driver = "PIIX4_PM",\
> - .property = "acpi-pci-hotplug-with-bridge-support",\
> - .value = "off",\
> - }
> -
> -#define PC_COMPAT_1_6 \
> - QEMU_COMPAT_1_6,\
> - {\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - }
> -
> -#define PC_COMPAT_1_5 \
> - QEMU_COMPAT_1_7,\
> - {\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - }
> -
> -#define PC_COMPAT_1_4 \
> - QEMU_COMPAT_1_4
> -
> #define PC_COMMON_MACHINE_OPTIONS \
> .default_boot_order = "cad"
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-24 19:51 ` Marcel Apfelbaum
@ 2014-06-24 19:59 ` Eduardo Habkost
0 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-24 19:59 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: Peter Maydell, Michael S. Tsirkin, 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 10:51:39PM +0300, Marcel Apfelbaum wrote:
> On Tue, 2014-06-24 at 15:02 -0300, Eduardo Habkost wrote:
> > All compat properties are only applied if a device of an specific type
> > is instantiated. There's no need to keep a PC-specific list of compat
> > properties, as properties for PC-specific devices won't affect other
> > machine-types anyway.
>
> So why do we still need 2 sets of hierarchies PC_I440FX_COMPAT_x_y and
> PC_Q35_COMPAT_x_y? It seems that QEMU_COMPAT_x_y is enough.
> Am I missing something?
A single PC_COMPAT_* hierarchy should be enough after this patch, yes. I
left them there because I was not sure this RFC would be accepted, and
wanted to keep the changes simple.
If people are OK with this patch, I can reorder the changes in this
series and move everything to QEMU_COMPAT_* in fewer steps, without even
creating the new PC_I440FX_COMPAT_* macros.
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-24 18:02 ` [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props Eduardo Habkost
2014-06-24 19:51 ` Marcel Apfelbaum
@ 2014-06-25 4:55 ` Michael S. Tsirkin
2014-06-25 13:25 ` Eduardo Habkost
1 sibling, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 4:55 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 03:02:04PM -0300, Eduardo Habkost wrote:
> All compat properties are only applied if a device of an specific type
> is instantiated. There's no need to keep a PC-specific list of compat
> properties, as properties for PC-specific devices won't affect other
> machine-types anyway.
>
> This patch moves all compat_props properties from PC compat macros, to
> QEMU_COMPAT_*.
>
> In case somebody wonders why the nesting was kept on the
> machine-type-specific PC_*_COMPAT_* macros instead of making
> QEMU_COMPAT_* macros nested. The answer is that making QEMU_COMPAT_*
> nested would make it more difficult to migrate the existing machine-type
> code to QOM later using class_init reuse. See:
> https://github.com/ehabkost/qemu-hacks/commit/9edb1f70fc2bd74131f54a2dfed1e890a5859736
>
> PC_COMPAT_1_3 and older are being left untouched by now, as they still
> need some additional cleanup and are unlikely to be reused by new
> machine-types anyway.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/i386/pc_piix.c | 10 +++++-----
> hw/i386/pc_q35.c | 28 +++++-----------------------
> include/hw/boards.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 43 -------------------------------------------
> 4 files changed, 56 insertions(+), 71 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d8010d16..5e3d24d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -442,7 +442,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
> };
>
> #define PC_I440FX_COMPAT_2_0 \
> - PC_COMPAT_2_0
> + QEMU_COMPAT_2_0
>
> #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
>
> @@ -458,7 +458,7 @@ static QEMUMachine pc_i440fx_machine_v2_0 = {
>
> #define PC_I440FX_COMPAT_1_7 \
> PC_I440FX_COMPAT_2_0, \
> - PC_COMPAT_1_7
> + QEMU_COMPAT_1_7
>
> #define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
As far as I can tell this will break hpet,
because you set it incorrectly for PIIX
#define QEMU_COMPAT_1_7 \
@@ -179,6 +197,16 @@ struct MachineState {
.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),\
}
AFAIK above is appropriate for Q35 but not PIIX.
See commit log for
commit 7a10ef51c2397ac4323bc786af02c58b413b5cd2
hpet: enable to entitle more irq pins for hpet
> @@ -474,7 +474,7 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
>
> #define PC_I440FX_COMPAT_1_6 \
> PC_I440FX_COMPAT_1_7, \
> - PC_COMPAT_1_6
> + QEMU_COMPAT_1_6
>
> #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
>
> @@ -490,7 +490,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>
> #define PC_I440FX_COMPAT_1_5 \
> PC_I440FX_COMPAT_1_6, \
> - PC_COMPAT_1_5
> + QEMU_COMPAT_1_5
>
> static QEMUMachine pc_i440fx_machine_v1_5 = {
> PC_I440FX_1_6_MACHINE_OPTIONS,
> @@ -504,7 +504,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>
> #define PC_I440FX_COMPAT_1_4 \
> PC_I440FX_COMPAT_1_5, \
> - PC_COMPAT_1_4
> + QEMU_COMPAT_1_4
>
> #define PC_I440FX_1_4_MACHINE_OPTIONS \
> PC_I440FX_1_6_MACHINE_OPTIONS, \
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 57eecc3..b59d22d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -355,20 +355,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
> };
>
> #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",\
> - }
> + QEMU_COMPAT_2_0
>
> #define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
>
> @@ -384,12 +371,7 @@ static QEMUMachine pc_q35_machine_v2_0 = {
>
> #define PC_Q35_COMPAT_1_7 \
> PC_Q35_COMPAT_2_0, \
> - PC_COMPAT_1_7, \
> - {\
> - .driver = "hpet",\
> - .property = HPET_INTCAP,\
> - .value = stringify(4),\
> - }
> + QEMU_COMPAT_1_7
>
> #define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> @@ -405,7 +387,7 @@ static QEMUMachine pc_q35_machine_v1_7 = {
>
> #define PC_Q35_COMPAT_1_6 \
> PC_Q35_COMPAT_1_7, \
> - PC_COMPAT_1_6
> + QEMU_COMPAT_1_6
>
> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
>
> @@ -421,7 +403,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
>
> #define PC_Q35_COMPAT_1_5 \
> PC_Q35_COMPAT_1_6, \
> - PC_COMPAT_1_5
> + QEMU_COMPAT_1_5
>
> static QEMUMachine pc_q35_machine_v1_5 = {
> PC_Q35_1_6_MACHINE_OPTIONS,
> @@ -435,7 +417,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>
> #define PC_Q35_COMPAT_1_4 \
> PC_Q35_COMPAT_1_5, \
> - PC_COMPAT_1_4
> + QEMU_COMPAT_1_4
>
> #define PC_Q35_1_4_MACHINE_OPTIONS \
> PC_Q35_1_6_MACHINE_OPTIONS, \
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 709b582..fa7ceb7 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -172,6 +172,24 @@ struct MachineState {
> .driver = "virtio-net-pci",\
> .property = "guest_announce",\
> .value = "off",\
> + },\
> + {\
> + .driver = "PIIX4_PM",\
> + .property = "memory-hotplug-support",\
> + .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 \
> @@ -179,6 +197,16 @@ struct MachineState {
> .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 \
> @@ -194,6 +222,15 @@ struct MachineState {
> .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 \
> @@ -229,6 +266,15 @@ struct MachineState {
> .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 \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 631afe7..8503bee 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,49 +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_COMPAT_2_0 \
> - QEMU_COMPAT_2_0,\
> - {\
> - .driver = "PIIX4_PM",\
> - .property = "memory-hotplug-support",\
> - .value = "off",\
> - }
> -
> -#define PC_COMPAT_1_7 \
> - QEMU_COMPAT_1_7,\
> - {\
> - .driver = "PIIX4_PM",\
> - .property = "acpi-pci-hotplug-with-bridge-support",\
> - .value = "off",\
> - }
> -
> -#define PC_COMPAT_1_6 \
> - QEMU_COMPAT_1_6,\
> - {\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(1),\
> - }
> -
> -#define PC_COMPAT_1_5 \
> - QEMU_COMPAT_1_7,\
> - {\
> - .driver = "i440FX-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - },{\
> - .driver = "q35-pcihost",\
> - .property = "short_root_bus",\
> - .value = stringify(0),\
> - }
> -
> -#define PC_COMPAT_1_4 \
> - QEMU_COMPAT_1_4
> -
> #define PC_COMMON_MACHINE_OPTIONS \
> .default_boot_order = "cad"
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-25 4:55 ` Michael S. Tsirkin
@ 2014-06-25 13:25 ` Eduardo Habkost
2014-06-25 14:12 ` Michael S. Tsirkin
0 siblings, 1 reply; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-25 13:25 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 07:55:53AM +0300, Michael S. Tsirkin wrote:
[...]
>
> As far as I can tell this will break hpet,
> because you set it incorrectly for PIIX
>
> #define QEMU_COMPAT_1_7 \
> @@ -179,6 +197,16 @@ struct MachineState {
> .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),\
> }
>
>
> AFAIK above is appropriate for Q35 but not PIIX.
>
> See commit log for
> commit 7a10ef51c2397ac4323bc786af02c58b413b5cd2
> hpet: enable to entitle more irq pins for hpet
I assume this was explained in the v2 commit message, and there was no
bug here, right?
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props
2014-06-25 13:25 ` Eduardo Habkost
@ 2014-06-25 14:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 14:12 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:25:12AM -0300, Eduardo Habkost wrote:
> On Wed, Jun 25, 2014 at 07:55:53AM +0300, Michael S. Tsirkin wrote:
> [...]
> >
> > As far as I can tell this will break hpet,
> > because you set it incorrectly for PIIX
> >
> > #define QEMU_COMPAT_1_7 \
> > @@ -179,6 +197,16 @@ struct MachineState {
> > .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),\
> > }
> >
> >
> > AFAIK above is appropriate for Q35 but not PIIX.
> >
> > See commit log for
> > commit 7a10ef51c2397ac4323bc786af02c58b413b5cd2
> > hpet: enable to entitle more irq pins for hpet
>
> I assume this was explained in the v2 commit message, and there was no
> bug here, right?
>
> --
> Eduardo
yes.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
` (3 preceding siblings ...)
2014-06-24 18:02 ` [Qemu-devel] [PATCH 4/4] [RFC] Eliminate PC-specific compat_props Eduardo Habkost
@ 2014-06-25 2:39 ` Alexey Kardashevskiy
2014-06-25 5:22 ` Michael S. Tsirkin
2014-06-25 4:57 ` Michael S. Tsirkin
2014-06-25 7:34 ` Michael S. Tsirkin
6 siblings, 1 reply; 25+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-25 2:39 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel, Michael S. Tsirkin
Cc: Peter Maydell, Marcel Apfelbaum, Markus Armbruster,
Paul Mackerras, Anthony Liguori, Igor Mammedov, Paolo Bonzini,
Andreas Färber, Alexander Graf
On 06/25/2014 04:02 AM, 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.
>
> The last patch is a proposal to simply eliminate the PC-specific compat props
> macros, because we don't really need them today. All compat properties we have
> can be on global QEMU-version-specific lists, because PC-specific properties are
> not going to affect other machine-types anyway.
The idea is cool, the separation of which properties go to boards.h and
which stay in hw/i386/pc_piix.c is not clear though (PIIX is unlikely to be
wanted somewhere else but virtio is).
>
> Eduardo Habkost (4):
> q35: Move q35-specific compat macros to pc_q35.c
> pc: Eliminate nesting of common PC_COMPAT_* macros
> machine: Introduce QEMU_COMPAT_* macros
> [RFC] Eliminate PC-specific compat_props
>
> hw/i386/pc_piix.c | 31 +++++--
> hw/i386/pc_q35.c | 19 +++++
> include/hw/boards.h | 207 +++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 224 ---------------------------------------------------
> 4 files changed, 251 insertions(+), 230 deletions(-)
--
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros
2014-06-25 2:39 ` [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Alexey Kardashevskiy
@ 2014-06-25 5:22 ` Michael S. Tsirkin
0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 5:22 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Peter Maydell, Eduardo Habkost, Marcel Apfelbaum,
Markus Armbruster, qemu-devel, Paul Mackerras, Anthony Liguori,
Igor Mammedov, Paolo Bonzini, Andreas Färber,
Alexander Graf
On Wed, Jun 25, 2014 at 12:39:39PM +1000, Alexey Kardashevskiy wrote:
> On 06/25/2014 04:02 AM, 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.
> >
> > The last patch is a proposal to simply eliminate the PC-specific compat props
> > macros, because we don't really need them today. All compat properties we have
> > can be on global QEMU-version-specific lists, because PC-specific properties are
> > not going to affect other machine-types anyway.
>
> The idea is cool, the separation of which properties go to boards.h and
> which stay in hw/i386/pc_piix.c is not clear though (PIIX is unlikely to be
> wanted somewhere else but virtio is).
virtio compat < 2.0 is not useful to non x86 either.
>
> >
> > Eduardo Habkost (4):
> > q35: Move q35-specific compat macros to pc_q35.c
> > pc: Eliminate nesting of common PC_COMPAT_* macros
> > machine: Introduce QEMU_COMPAT_* macros
> > [RFC] Eliminate PC-specific compat_props
> >
> > hw/i386/pc_piix.c | 31 +++++--
> > hw/i386/pc_q35.c | 19 +++++
> > include/hw/boards.h | 207 +++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/i386/pc.h | 224 ---------------------------------------------------
> > 4 files changed, 251 insertions(+), 230 deletions(-)
>
>
>
>
>
> --
> Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
` (4 preceding siblings ...)
2014-06-25 2:39 ` [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Alexey Kardashevskiy
@ 2014-06-25 4:57 ` Michael S. Tsirkin
2014-06-25 7:34 ` Michael S. Tsirkin
6 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 4:57 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 03:02:00PM -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.
>
> The last patch is a proposal to simply eliminate the PC-specific compat props
> macros, because we don't really need them today. All compat properties we have
> can be on global QEMU-version-specific lists, because PC-specific properties are
> not going to affect other machine-types anyway.
How was this tested?
I would suggest compiling qemu before and after this
patch. stripped objects should be identical.
> Eduardo Habkost (4):
> q35: Move q35-specific compat macros to pc_q35.c
> pc: Eliminate nesting of common PC_COMPAT_* macros
> machine: Introduce QEMU_COMPAT_* macros
> [RFC] Eliminate PC-specific compat_props
>
> hw/i386/pc_piix.c | 31 +++++--
> hw/i386/pc_q35.c | 19 +++++
> include/hw/boards.h | 207 +++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 224 ---------------------------------------------------
> 4 files changed, 251 insertions(+), 230 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros
2014-06-24 18:02 [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
` (5 preceding siblings ...)
2014-06-25 4:57 ` Michael S. Tsirkin
@ 2014-06-25 7:34 ` Michael S. Tsirkin
2014-06-25 13:50 ` Eduardo Habkost
6 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2014-06-25 7:34 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 03:02:00PM -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.
>
> The last patch is a proposal to simply eliminate the PC-specific compat props
> macros, because we don't really need them today. All compat properties we have
> can be on global QEMU-version-specific lists, because PC-specific properties are
> not going to affect other machine-types anyway.
So global properties are really not so special.
They just need to be specified with <X>.<Y>=<Z> syntax.
Can we just make them properties of a QEMU object?
I would like to make it possible for management to
ask "what's the default value for that property with that
machine type".
Need to add API to make this painless, with not more lines of code
than currently.
> Eduardo Habkost (4):
> q35: Move q35-specific compat macros to pc_q35.c
> pc: Eliminate nesting of common PC_COMPAT_* macros
> machine: Introduce QEMU_COMPAT_* macros
> [RFC] Eliminate PC-specific compat_props
>
> hw/i386/pc_piix.c | 31 +++++--
> hw/i386/pc_q35.c | 19 +++++
> include/hw/boards.h | 207 +++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 224 ---------------------------------------------------
> 4 files changed, 251 insertions(+), 230 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros
2014-06-25 7:34 ` Michael S. Tsirkin
@ 2014-06-25 13:50 ` Eduardo Habkost
0 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2014-06-25 13:50 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 10:34:24AM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 24, 2014 at 03:02:00PM -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.
> >
> > The last patch is a proposal to simply eliminate the PC-specific compat props
> > macros, because we don't really need them today. All compat properties we have
> > can be on global QEMU-version-specific lists, because PC-specific properties are
> > not going to affect other machine-types anyway.
>
> So global properties are really not so special.
> They just need to be specified with <X>.<Y>=<Z> syntax.
> Can we just make them properties of a QEMU object?
>
> I would like to make it possible for management to
> ask "what's the default value for that property with that
> machine type".
>
> Need to add API to make this painless, with not more lines of code
> than currently.
>
We don't even have an API for asking for class property info (with no
machine-type-specific data) yet. I keep asking if we are going to move
towards some day. But the current plan is unclear to me, and the only
available method today seems to be "just create an object and ask for
its properties" (the same seems to be necessary even for listing the
available property _names_).
There has been some discussion about it at:
http://marc.info/?l=qemu-devel&m=139170340408156
but the conclusion was unclear.
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread