From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wzg8a-0006KD-N7 for qemu-devel@nongnu.org; Wed, 25 Jun 2014 01:52:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wzg8V-0002AE-SX for qemu-devel@nongnu.org; Wed, 25 Jun 2014 01:52:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:65365) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wzg8V-0002AA-Kv for qemu-devel@nongnu.org; Wed, 25 Jun 2014 01:52:47 -0400 Date: Wed, 25 Jun 2014 08:53:08 +0300 From: "Michael S. Tsirkin" Message-ID: <20140625055308.GF21297@redhat.com> References: <1403661885-28619-1-git-send-email-ehabkost@redhat.com> <1403661885-28619-2-git-send-email-ehabkost@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1403661885-28619-2-git-send-email-ehabkost@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Peter Maydell , Liu Ping Fan , qemu-devel@nongnu.org On Tue, Jun 24, 2014 at 11:04:44PM -0300, Eduardo Habkost wrote: > For each compat property on PC_Q35_COMPAT_*, there are only two > possibilities: > > * If the device is never instantiated when using a machine other than > pc-q35, then the compat property can be safely added to > PC_COMPAT_*; > * If the device can be instantiated when using a machine other than > pc-q35, that means the other machines also need the compat property > to be set. > That means we don't need separate PC_Q35_COMPAT_* macros at all, today. > > The hpet.hpet-intcap case is interesting: piix and q35 do have something > that emulates different defaults, but the machine-specific default is > applied _after_ compat_props are applied, by simply checking if the > property is zero (which is the real default on the hpet code). > > The hpet.hpet-intcap=0x4 compat property can (should?) be applied to > piix too, because 0x4 was the default on both piix and q35 before the > hpet-intcap property was introduced. True. That's a very good explanation, thanks! > Now, if one day we change the default HPET intcap on one of the PC > machine-types again, we may want to introduce PC_{Q35,I440FX}_COMPAT > macros. But while we don't need that, we can keep the code simple. > > Signed-off-by: Eduardo Habkost > Cc: Liu Ping Fan > Cc: Peter Maydell Small patch, less code, same functionality, what's not to like. Reviewed-by: Michael S. Tsirkin > --- > hw/i386/pc_q35.c | 10 +++++----- > include/hw/i386/pc.h | 55 +++++++++++++++++----------------------------------- > 2 files changed, 23 insertions(+), 42 deletions(-) > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 155db99..36b6ab0 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -361,7 +361,7 @@ static QEMUMachine pc_q35_machine_v2_0 = { > .name = "pc-q35-2.0", > .init = pc_q35_init_2_0, > .compat_props = (GlobalProperty[]) { > - PC_Q35_COMPAT_2_0, > + PC_COMPAT_2_0, > { /* end of list */ } > }, > }; > @@ -373,7 +373,7 @@ static QEMUMachine pc_q35_machine_v1_7 = { > .name = "pc-q35-1.7", > .init = pc_q35_init_1_7, > .compat_props = (GlobalProperty[]) { > - PC_Q35_COMPAT_1_7, > + PC_COMPAT_1_7, > { /* end of list */ } > }, > }; > @@ -385,7 +385,7 @@ static QEMUMachine pc_q35_machine_v1_6 = { > .name = "pc-q35-1.6", > .init = pc_q35_init_1_6, > .compat_props = (GlobalProperty[]) { > - PC_Q35_COMPAT_1_6, > + PC_COMPAT_1_6, > { /* end of list */ } > }, > }; > @@ -395,7 +395,7 @@ static QEMUMachine pc_q35_machine_v1_5 = { > .name = "pc-q35-1.5", > .init = pc_q35_init_1_5, > .compat_props = (GlobalProperty[]) { > - PC_Q35_COMPAT_1_5, > + PC_COMPAT_1_5, > { /* end of list */ } > }, > }; > @@ -409,7 +409,7 @@ static QEMUMachine pc_q35_machine_v1_4 = { > .name = "pc-q35-1.4", > .init = pc_q35_init_1_4, > .compat_props = (GlobalProperty[]) { > - PC_Q35_COMPAT_1_4, > + PC_COMPAT_1_4, > { /* end of list */ } > }, > }; > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 1331d5a..1c0c382 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -294,43 +294,6 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > -#define PC_Q35_COMPAT_2_0 \ > - PC_COMPAT_2_0, \ > - {\ > - .driver = "ICH9-LPC",\ > - .property = "memory-hotplug-support",\ > - .value = "off",\ > - },{\ > - .driver = "xio3130-downstream",\ > - .property = COMPAT_PROP_PCP,\ > - .value = "off",\ > - },{\ > - .driver = "ioh3420",\ > - .property = COMPAT_PROP_PCP,\ > - .value = "off",\ > - } > - > -#define PC_Q35_COMPAT_1_7 \ > - PC_COMPAT_1_7, \ > - PC_Q35_COMPAT_2_0, \ > - {\ > - .driver = "hpet",\ > - .property = HPET_INTCAP,\ > - .value = stringify(4),\ > - } > - > -#define PC_Q35_COMPAT_1_6 \ > - PC_COMPAT_1_6, \ > - PC_Q35_COMPAT_1_7 > - > -#define PC_Q35_COMPAT_1_5 \ > - PC_COMPAT_1_5, \ > - PC_Q35_COMPAT_1_6 > - > -#define PC_Q35_COMPAT_1_4 \ > - PC_COMPAT_1_4, \ > - PC_Q35_COMPAT_1_5 > - > #define PC_COMPAT_2_0 \ > {\ > .driver = "virtio-scsi-pci",\ > @@ -370,6 +333,19 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > .driver = "virtio-net-pci",\ > .property = "guest_announce",\ > .value = "off",\ > + },\ > + {\ > + .driver = "ICH9-LPC",\ > + .property = "memory-hotplug-support",\ > + .value = "off",\ > + },{\ > + .driver = "xio3130-downstream",\ > + .property = COMPAT_PROP_PCP,\ > + .value = "off",\ > + },{\ > + .driver = "ioh3420",\ > + .property = COMPAT_PROP_PCP,\ > + .value = "off",\ > } > > #define PC_COMPAT_1_7 \ > @@ -383,6 +359,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > .driver = "PIIX4_PM",\ > .property = "acpi-pci-hotplug-with-bridge-support",\ > .value = "off",\ > + },\ > + {\ > + .driver = "hpet",\ > + .property = HPET_INTCAP,\ > + .value = stringify(4),\ > } > > #define PC_COMPAT_1_6 \ > -- > 1.9.3