From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Marcel Apfelbaum" <marcel.a@redhat.com>,
"Alexey Kardashevskiy" <aik@ozlabs.ru>,
qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
"Paul Mackerras" <paulus@samba.org>,
"Anthony Liguori" <aliguori@amazon.com>,
"Igor Mammedov" <imammedo@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Andreas Färber" <afaerber@suse.de>,
"Alexander Graf" <agraf@suse.com>
Subject: Re: [Qemu-devel] [PATCH 2/4] pc: Eliminate nesting of common PC_COMPAT_* macros
Date: Wed, 25 Jun 2014 08:13:12 +0300 [thread overview]
Message-ID: <20140625051312.GC21297@redhat.com> (raw)
In-Reply-To: <1403632924-16603-3-git-send-email-ehabkost@redhat.com>
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
next prev parent reply other threads:[~2014-06-25 5:13 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
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 19:23 ` Marcel Apfelbaum
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 [this message]
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-24 23:01 ` Eduardo Habkost
2014-06-25 5:20 ` Michael S. Tsirkin
2014-06-25 6:46 ` Marcel Apfelbaum
2014-06-25 7:18 ` Michael S. Tsirkin
2014-06-25 7:20 ` Marcel Apfelbaum
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
2014-06-25 13:25 ` Eduardo Habkost
2014-06-25 14:12 ` Michael S. Tsirkin
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
2014-06-25 4:57 ` Michael S. Tsirkin
2014-06-25 7:34 ` Michael S. Tsirkin
2014-06-25 13:50 ` Eduardo Habkost
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140625051312.GC21297@redhat.com \
--to=mst@redhat.com \
--cc=afaerber@suse.de \
--cc=agraf@suse.com \
--cc=aik@ozlabs.ru \
--cc=aliguori@amazon.com \
--cc=armbru@redhat.com \
--cc=ehabkost@redhat.com \
--cc=imammedo@redhat.com \
--cc=marcel.a@redhat.com \
--cc=paulus@samba.org \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.