| Message ID | e87fd7753c097333c39526f70a68fae7880a7259.1346069810.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 27, 2012 at 03:20:13PM +0300, Michael S. Tsirkin wrote: > In preparation to adding PV EOI migration for 1.2, > trivially refactor some some compat code > to make it easier to add version specific > cpuid tweaks. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/pc_piix.c | 44 ++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 36 insertions(+), 8 deletions(-) Why can't you clear the cpuid bit in kvm paravirt leaf at hw/pc_piix.c, leaving compat code isolated there?
On Tue, Aug 28, 2012 at 01:23:18PM -0300, Marcelo Tosatti wrote: > On Mon, Aug 27, 2012 at 03:20:13PM +0300, Michael S. Tsirkin wrote: > > In preparation to adding PV EOI migration for 1.2, > > trivially refactor some some compat code > > to make it easier to add version specific > > cpuid tweaks. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/pc_piix.c | 44 ++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 36 insertions(+), 8 deletions(-) > > Why can't you clear the cpuid bit in kvm paravirt leaf at hw/pc_piix.c, > leaving compat code isolated there? This is not how we handle it for compat properties: there we set flag in pc_piix and make devices look at that flag. And it makes sense because what you suggest does not scale: we can not teach pc_piix about quirks of all hardware. It will also scale better if we ever get interested about compatibility and migration for non pc machines.
On Tue, Aug 28, 2012 at 07:31:24PM +0300, Michael S. Tsirkin wrote: > On Tue, Aug 28, 2012 at 01:23:18PM -0300, Marcelo Tosatti wrote: > > On Mon, Aug 27, 2012 at 03:20:13PM +0300, Michael S. Tsirkin wrote: > > > In preparation to adding PV EOI migration for 1.2, > > > trivially refactor some some compat code > > > to make it easier to add version specific > > > cpuid tweaks. > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > hw/pc_piix.c | 44 ++++++++++++++++++++++++++++++++++++-------- > > > 1 file changed, 36 insertions(+), 8 deletions(-) > > > > Why can't you clear the cpuid bit in kvm paravirt leaf at hw/pc_piix.c, > > leaving compat code isolated there? > > This is not how we handle it for compat properties: > there we set flag in pc_piix and make devices look at > that flag. > And it makes sense because what you suggest does not scale: we can not > teach pc_piix about quirks of all hardware. What is there to scale? Old machines are static, they don't change. This way you are putting the burden of compat support in the generic driver code, which has better things to worry about than old machine types. IMO it is better to have all of the compat mess localized. > It will also scale better if we ever get interested about > compatibility and migration for non pc machines. > -- > MST Alright, will wait a couple of days before merging.
On Tue, Aug 28, 2012 at 01:37:18PM -0300, Marcelo Tosatti wrote: > On Tue, Aug 28, 2012 at 07:31:24PM +0300, Michael S. Tsirkin wrote: > > On Tue, Aug 28, 2012 at 01:23:18PM -0300, Marcelo Tosatti wrote: > > > On Mon, Aug 27, 2012 at 03:20:13PM +0300, Michael S. Tsirkin wrote: > > > > In preparation to adding PV EOI migration for 1.2, > > > > trivially refactor some some compat code > > > > to make it easier to add version specific > > > > cpuid tweaks. > > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > > > hw/pc_piix.c | 44 ++++++++++++++++++++++++++++++++++++-------- > > > > 1 file changed, 36 insertions(+), 8 deletions(-) > > > > > > Why can't you clear the cpuid bit in kvm paravirt leaf at hw/pc_piix.c, > > > leaving compat code isolated there? > > > > This is not how we handle it for compat properties: > > there we set flag in pc_piix and make devices look at > > that flag. > > And it makes sense because what you suggest does not scale: we can not > > teach pc_piix about quirks of all hardware. > > What is there to scale? Old machines are static, they don't change. But we keep adding new features in each version and each of them adds new device specific stuff. If we stick all logic in pc_piix.c it will quickly have to know about internals of virtio,cdrom,ide,cpu ... > This > way you are putting the burden of compat support in the generic driver > code, which has better things to worry about than old machine types. > > IMO it is better to have all of the compat mess localized. > > > It will also scale better if we ever get interested about > > compatibility and migration for non pc machines. > > -- > > MST > > Alright, will wait a couple of days before merging.
diff --git a/hw/pc_piix.c b/hw/pc_piix.c index a771d79..008d42f 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -369,6 +369,22 @@ static QEMUMachine pc_machine_v1_2 = { .default_machine_opts = KVM_MACHINE_OPTIONS, }; +static void pc_machine_v1_1_compat(void) +{ +} + +static void pc_init_pci_v1_1(ram_addr_t ram_size, + const char *boot_device, + const char *kernel_filename, + const char *kernel_cmdline, + const char *initrd_filename, + const char *cpu_model) +{ + pc_machine_v1_1_compat(); + pc_init_pci(ram_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model); +} + #define PC_COMPAT_1_1 \ {\ .driver = "virtio-scsi-pci",\ @@ -403,7 +419,7 @@ static QEMUMachine pc_machine_v1_2 = { static QEMUMachine pc_machine_v1_1 = { .name = "pc-1.1", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_v1_1, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -439,7 +455,7 @@ static QEMUMachine pc_machine_v1_1 = { static QEMUMachine pc_machine_v1_0 = { .name = "pc-1.0", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_v1_1, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -455,7 +471,7 @@ static QEMUMachine pc_machine_v1_0 = { static QEMUMachine pc_machine_v0_15 = { .name = "pc-0.15", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_v1_1, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -488,7 +504,7 @@ static QEMUMachine pc_machine_v0_15 = { static QEMUMachine pc_machine_v0_14 = { .name = "pc-0.14", .desc = "Standard PC", - .init = pc_init_pci, + .init = pc_init_pci_v1_1, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -519,10 +535,22 @@ static QEMUMachine pc_machine_v0_14 = { .value = stringify(1),\ } +static void pc_init_pci_v0_13(ram_addr_t ram_size, + const char *boot_device, + const char *kernel_filename, + const char *kernel_cmdline, + const char *initrd_filename, + const char *cpu_model) +{ + pc_machine_v1_1_compat(); + pc_init_pci_no_kvmclock(ram_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model); +} + static QEMUMachine pc_machine_v0_13 = { .name = "pc-0.13", .desc = "Standard PC", - .init = pc_init_pci_no_kvmclock, + .init = pc_init_pci_v0_13, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -560,7 +588,7 @@ static QEMUMachine pc_machine_v0_13 = { static QEMUMachine pc_machine_v0_12 = { .name = "pc-0.12", .desc = "Standard PC", - .init = pc_init_pci_no_kvmclock, + .init = pc_init_pci_v0_13, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -594,7 +622,7 @@ static QEMUMachine pc_machine_v0_12 = { static QEMUMachine pc_machine_v0_11 = { .name = "pc-0.11", .desc = "Standard PC, qemu 0.11", - .init = pc_init_pci_no_kvmclock, + .init = pc_init_pci_v0_13, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) { @@ -616,7 +644,7 @@ static QEMUMachine pc_machine_v0_11 = { static QEMUMachine pc_machine_v0_10 = { .name = "pc-0.10", .desc = "Standard PC, qemu 0.10", - .init = pc_init_pci_no_kvmclock, + .init = pc_init_pci_v0_13, .max_cpus = 255, .default_machine_opts = KVM_MACHINE_OPTIONS, .compat_props = (GlobalProperty[]) {
In preparation to adding PV EOI migration for 1.2, trivially refactor some some compat code to make it easier to add version specific cpuid tweaks. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/pc_piix.c | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-)