| Message ID | 1403632924-16603-5-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
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" >
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.
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
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?
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.
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"
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(-)