pc: reduce duplication in compat machine types
PATCH 1/1 pc: reduce duplication in compat machine types
Make it easier to add compat properties, by adding macros for properties duplicated across machine types.
Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and compared to sorted original.
Lightly tested on x86_64.
---
I've put the above on my branch as it's needed for fixing balloon pci class. Let me know if there are any objections.
hw/pc_piix.c | 288 +++++++++++++++------------------------------------------- 1 files changed, 73 insertions(+), 215 deletions(-)
hw/pc_piix.c +73 -215
@@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = { .is_default = 1, }; +#define PC_COMPAT_1_0 \ + {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } +static QEMUMachine pc_machine_v1_0 = { .name ="pc-1.0" , .desc ="Standard PC" , .init = pc_init_pci, .max_cpus =255 , .compat_props = (GlobalProperty []) { - { - .driver ="pc-sysfw" , - .property ="rom_only" , - .value =stringify (1 ), - }, { - .driver ="isa-fdc" , - .property ="check_media_rate" , - .value ="off" , - }, +PC_COMPAT_1_0 , {/* end of list */ } }, }; +#define PC_COMPAT_0_15 \ + PC_COMPAT_1_0 +static QEMUMachine pc_machine_v0_15 = { .name ="pc-0.15" , .desc ="Standard PC" , .init = pc_init_pci, .max_cpus =255 , .compat_props = (GlobalProperty []) { - { - .driver ="pc-sysfw" , - .property ="rom_only" , - .value =stringify (1 ), - }, { - .driver ="isa-fdc" , - .property ="check_media_rate" , - .value ="off" , - }, +PC_COMPAT_0_15 , {/* end of list */ } }, }; +#define PC_COMPAT_0_14 \ + PC_COMPAT_0_15,\ + {\ + .driver = "virtio-blk-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-serial-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-net-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-balloon-pci",\ + .property = "event_idx",\ + .value = "off",\ + } +static QEMUMachine pc_machine_v0_14 = { .name ="pc-0.14" , .desc ="Standard PC" , .init = pc_init_pci, .max_cpus =255 , .compat_props = (GlobalProperty []) { +PC_COMPAT_0_14 , { .driver ="qxl" , .property ="revision" ,
@@ -424,42 +443,30 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "qxl-vga", .property = "revision", .value = stringify(2), - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, {/* end of list */ } }, }; +#define PC_COMPAT_0_13 \ + PC_COMPAT_0_14,\ + {\ + .driver = "PCI",\ + .property = "command_serr_enable",\ + .value = "off",\ + },{\ + .driver = "AC97",\ + .property = "use_broken_id",\ + .value = stringify(1),\ + } +static QEMUMachine pc_machine_v0_13 = { .name ="pc-0.13" , .desc ="Standard PC" , .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { +PC_COMPAT_0_13 , { .driver ="virtio-9p-pci" , .property ="vectors" ,
@@ -472,59 +479,31 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "vmware-svga", .property = "rombar", .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, {/* end of list */ } }, }; +#define PC_COMPAT_0_12 \ + PC_COMPAT_0_13,\ + {\ + .driver = "virtio-serial-pci",\ + .property = "max_ports",\ + .value = stringify(1),\ + },{\ + .driver = "virtio-serial-pci",\ + .property = "vectors",\ + .value = stringify(0),\ + } +static QEMUMachine pc_machine_v0_12 = { .name ="pc-0.12" , .desc ="Standard PC" , .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { +PC_COMPAT_0_12 , { - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver ="VGA" , .property ="rombar" , .value =stringify (0 ),
@@ -532,63 +511,27 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "vmware-svga", .property = "rombar", .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, {/* end of list */ } } }; +#define PC_COMPAT_0_11 \ + PC_COMPAT_0_12,\ + {\ + .driver = "virtio-blk-pci",\ + .property = "vectors",\ + .value = stringify(0),\ + } +static QEMUMachine pc_machine_v0_11 = { .name ="pc-0.11" , .desc ="Standard PC, qemu 0.11" , .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { +PC_COMPAT_0_11 , { - .driver = "virtio-blk-pci", - .property = "vectors", - .value = stringify(0), - },{ - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver ="ide-drive" , .property ="ver" , .value ="0.11" ,
@@ -596,43 +539,6 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "scsi-disk", .property = "ver", .value = "0.11", - },{ - .driver = "PCI", - .property = "rombar", - .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, {/* end of list */ } }
@@ -644,6 +550,7 @@ static QEMUMachine pc_machine_v0_10 = { .init = pc_init_pci_no_kvmclock, .max_cpus = 255, .compat_props = (GlobalProperty []) { +PC_COMPAT_0_11 , { .driver ="virtio-blk-pci" , .property ="class" ,
@@ -653,22 +560,10 @@ static QEMUMachine pc_machine_v0_10 = { .property = "class", .value = stringify(PCI_CLASS_DISPLAY_OTHER), },{ - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "virtio-net-pci", .property = "vectors", .value = stringify(0), },{ - .driver = "virtio-blk-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "ide-drive", .property = "ver", .value = "0.10",
@@ -676,43 +571,6 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "scsi-disk", .property = "ver", .value = "0.10", - },{ - .driver = "PCI", - .property = "rombar", - .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, {/* end of list */ } },
Signature · 1 line
1.7.9.111.gf3fb0
Quoted 14 lines · Michael S. Tsirkin
Make it easier to add compat properties, by adding macros for properties duplicated across machine types.
Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and compared to sorted original.
Lightly tested onx86_64.hw/pc_piix.c +1
-0+#define PC_COMPAT_1_0 \
hw/pc_piix.c static QEMUMachine pc_machine_v1_1 = { · line 385
Quoted 10 lines
hw/pc_piix.c +10
-0+ {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } +
Hmm. how about
Quoted 6 lines
static QEMUMachine pc_machine_v1_0 = {
.name = "pc-1.0",
.desc = "Standard PC",
.init = pc_init_pci,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {hw/pc_piix.c static QEMUMachine pc_machine_v1_1 = { · line 392
Quoted 10 lines
hw/pc_piix.c +1
-9- { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), - }, { - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, + PC_COMPAT_1_0,
+ .base_machine = &pc_machine_v1_1;
Then it would be easier to define machines differentially.
Quoted 2 lines
{ /* end of list */ }
},Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 45 lines · Avi Kivity
Quoted 14 lines · Michael S. Tsirkin
Make it easier to add compat properties, by adding macros for properties duplicated across machine types.
Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and compared to sorted original.
Lightly tested onx86_64.hw/pc_piix.c +1
-0+#define PC_COMPAT_1_0 \hw/pc_piix.c static QEMUMachine pc_machine_v1_1 = { · line 385
Quoted 10 lines
hw/pc_piix.c +10
-0+ {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } +Hmm. how about
Quoted 6 lines
static QEMUMachine pc_machine_v1_0 = { .name = "pc-1.0", .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) {hw/pc_piix.c static QEMUMachine pc_machine_v1_1 = { · line 392
Quoted 10 lines
hw/pc_piix.c +1
-9- { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), - }, { - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, + PC_COMPAT_1_0,
Quoted 2 lines
+ .base_machine =
&pc_machine_v1_1;
It's a lot of work, and the result won't be easier to modify or debug than it already is, IMO.
Quoted 1 line
Then it would be easier to define machines differentially.
We add new 1 machine each release, so I'm not worried about adding them easily, adding properies easily happens between releases so I want to make is simpler.
Quoted 6 lines
Quoted 2 lines
{ /* end of list */ } },Signature · 1 line
error compiling committee.c: too many arguments to function
Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
Quoted 12 lines
Make it easier to add compat properties, by adding macros for properties duplicated across machine types.
Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and compared to sorted original.
Lightly tested onx86_64.
Quoted 6 lines
Signed-off-by Michael S. Tsirkin <[email protected]>---
I've put the above on my branch as it's needed for fixing balloon pci class. Let me know if there are any objections.
Better reusing code is certainly a good idea. I wonder though if we might ever get into a situation where things change back and forth. I'd hope that properties are processed top to bottom so that, e.g., PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited through 1_0.
Andreas
Quoted 42 lines
hw/pc_piix.c | 288 +++++++++++++++------------------------------------------- 1 files changed, 73 insertions(+), 215 deletions(-)hw/pc_piix.c +12
-9@@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = { .is_default = 1, }; +#define PC_COMPAT_1_0 \ + {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } +static QEMUMachine pc_machine_v1_0 = { .name ="pc-1.0" , .desc ="Standard PC" , .init = pc_init_pci, .max_cpus =255 , .compat_props = (GlobalProperty []) { - { - .driver ="pc-sysfw" , - .property ="rom_only" , - .value =stringify (1 ), - }, { - .driver ="isa-fdc" , - .property ="check_media_rate" , - .value ="off" , - }, +PC_COMPAT_1_0 , {/* end of list */ } }, };
[snip]
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 20 lines · Andreas Färber
Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
Quoted 12 lines
Make it easier to add compat properties, by adding macros for properties duplicated across machine types.
Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and compared to sorted original.
Lightly tested onx86_64.Quoted 6 lines
Signed-off-by Michael S. Tsirkin <[email protected]>---
I've put the above on my branch as it's needed for fixing balloon pci class. Let me know if there are any objections.
Quoted 7 lines
Better reusing code is certainly a good idea. I wonder though if we might ever get into a situation where things change back and forth. I'd hope that properties are processed top to bottom so that, e.g.,
PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited through 1_0.Andreas
There are some properties like that currently.
For example, see ide-drive "ver" property or
VGA rombar property. I'm not sure that's intentional but I kept it as is.
In that case we can simply keep it out of
the common macros, that's what I did. There's no rule that says we must have everything in there.
Quoted 47 lines
Quoted 42 lines
hw/pc_piix.c | 288 +++++++++++++++------------------------------------------- 1 files changed, 73 insertions(+), 215 deletions(-)hw/pc_piix.c +12
-9@@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = { .is_default = 1, }; +#define PC_COMPAT_1_0 \ + {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } +static QEMUMachine pc_machine_v1_0 = { .name ="pc-1.0" , .desc ="Standard PC" , .init = pc_init_pci, .max_cpus =255 , .compat_props = (GlobalProperty []) { - { - .driver ="pc-sysfw" , - .property ="rom_only" , - .value =stringify (1 ), - }, { - .driver ="isa-fdc" , - .property ="check_media_rate" , - .value ="off" , - }, +PC_COMPAT_1_0 , {/* end of list */ } }, };[snip]
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg