| Message ID | 1376316104-11269-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 12.08.2013 16:01, schrieb Anthony Liguori: > This breaks migration and is unneeded with modern SeaBIOS. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > v1 -> v2 > - Still load external DSDT for q35 Care to extend $subject with "for i440fx" then before applying? :) Regards, Andreas > --- > hw/i386/pc_piix.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 95c45b8..311574a 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, > OBJECT(icc_bridge), NULL); > > pc_cpus_init(cpu_model, icc_bridge); > - pc_acpi_init("acpi-dsdt.aml"); > > if (kvm_enabled() && kvmclock_enabled) { > kvmclock_create(); >
On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote: > This breaks migration and is unneeded with modern SeaBIOS. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> Hmm don't we want to keep it around for machine types 1.4.0 and 1.5.0? By the way, copy stable as well? Loading it unconditonally is a cross version migration bug that we probably want to fix on stable branch - disabling for 1.3.0 and older. > --- > v1 -> v2 > - Still load external DSDT for q35 > --- > hw/i386/pc_piix.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 95c45b8..311574a 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, > OBJECT(icc_bridge), NULL); > > pc_cpus_init(cpu_model, icc_bridge); > - pc_acpi_init("acpi-dsdt.aml"); > > if (kvm_enabled() && kvmclock_enabled) { > kvmclock_create(); > -- > 1.8.0
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote: >> This breaks migration and is unneeded with modern SeaBIOS. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > Hmm don't we want to keep it around for machine types > 1.4.0 and 1.5.0? Hrm, why? Regards, Anthony Liguori > > By the way, copy stable as well? > Loading it unconditonally is a cross > version migration bug that we probably want to fix > on stable branch - disabling for 1.3.0 and older. > >> --- >> v1 -> v2 >> - Still load external DSDT for q35 >> --- >> hw/i386/pc_piix.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 95c45b8..311574a 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, >> OBJECT(icc_bridge), NULL); >> >> pc_cpus_init(cpu_model, icc_bridge); >> - pc_acpi_init("acpi-dsdt.aml"); >> >> if (kvm_enabled() && kvmclock_enabled) { >> kvmclock_create(); >> -- >> 1.8.0
On Mon, Aug 12, 2013 at 10:20:41AM -0500, Anthony Liguori wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote: > >> This breaks migration and is unneeded with modern SeaBIOS. > >> > >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > > > Hmm don't we want to keep it around for machine types > > 1.4.0 and 1.5.0? > > Hrm, why? Well this modifies the contents of ROM which is loaded from FW CFG and isn't migrated, correct? If guest is migrated from 1.6 to 1.5 while it is loading the ROM, it will likely get a corrupted table. > Regards, > > Anthony Liguori > > > > > By the way, copy stable as well? > > Loading it unconditonally is a cross > > version migration bug that we probably want to fix > > on stable branch - disabling for 1.3.0 and older. > > > >> --- > >> v1 -> v2 > >> - Still load external DSDT for q35 > >> --- > >> hw/i386/pc_piix.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > >> index 95c45b8..311574a 100644 > >> --- a/hw/i386/pc_piix.c > >> +++ b/hw/i386/pc_piix.c > >> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, > >> OBJECT(icc_bridge), NULL); > >> > >> pc_cpus_init(cpu_model, icc_bridge); > >> - pc_acpi_init("acpi-dsdt.aml"); > >> > >> if (kvm_enabled() && kvmclock_enabled) { > >> kvmclock_create(); > >> -- > >> 1.8.0
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Mon, Aug 12, 2013 at 10:20:41AM -0500, Anthony Liguori wrote: >> "Michael S. Tsirkin" <mst@redhat.com> writes: >> >> > On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote: >> >> This breaks migration and is unneeded with modern SeaBIOS. >> >> >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> > >> > Hmm don't we want to keep it around for machine types >> > 1.4.0 and 1.5.0? >> >> Hrm, why? > > Well this modifies the contents of ROM which > is loaded from FW CFG and isn't migrated, correct? > If guest is migrated from 1.6 to 1.5 > while it is loading the ROM, > it will likely get a corrupted table. No, if you migrate from 1.6 to 1.5 the ROM is untouched. ROMs aren't migrated so it doesn't really matter. If you migrate from 1.5 while reading the ROM contents, then badness can ensue but that's already the case without this patch. Regards, Anthony Liguori > > >> Regards, >> >> Anthony Liguori >> >> > >> > By the way, copy stable as well? >> > Loading it unconditonally is a cross >> > version migration bug that we probably want to fix >> > on stable branch - disabling for 1.3.0 and older. >> > >> >> --- >> >> v1 -> v2 >> >> - Still load external DSDT for q35 >> >> --- >> >> hw/i386/pc_piix.c | 1 - >> >> 1 file changed, 1 deletion(-) >> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> >> index 95c45b8..311574a 100644 >> >> --- a/hw/i386/pc_piix.c >> >> +++ b/hw/i386/pc_piix.c >> >> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, >> >> OBJECT(icc_bridge), NULL); >> >> >> >> pc_cpus_init(cpu_model, icc_bridge); >> >> - pc_acpi_init("acpi-dsdt.aml"); >> >> >> >> if (kvm_enabled() && kvmclock_enabled) { >> >> kvmclock_create(); >> >> -- >> >> 1.8.0
On Mon, Aug 12, 2013 at 02:59:24PM -0500, Anthony Liguori wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Mon, Aug 12, 2013 at 10:20:41AM -0500, Anthony Liguori wrote: > >> "Michael S. Tsirkin" <mst@redhat.com> writes: > >> > >> > On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote: > >> >> This breaks migration and is unneeded with modern SeaBIOS. > >> >> > >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > >> > > >> > Hmm don't we want to keep it around for machine types > >> > 1.4.0 and 1.5.0? > >> > >> Hrm, why? > > > > Well this modifies the contents of ROM which > > is loaded from FW CFG and isn't migrated, correct? > > If guest is migrated from 1.6 to 1.5 > > while it is loading the ROM, > > it will likely get a corrupted table. > > No, if you migrate from 1.6 to 1.5 the ROM is untouched. ROMs aren't > migrated so it doesn't really matter. Clarifying this on IRC, what was meant here is that -acpi is uncommon so bios typically checks and sees nothing in the ACPI FW CFG entry, and this check is atomic. So this works, by luck. > > If you migrate from 1.5 while reading the ROM contents, then badness can > ensue but that's already the case without this patch. > > Regards, > > Anthony Liguori Well there's a difference between can and will :) But yes, some future version will likely break it anyway. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > > > > >> Regards, > >> > >> Anthony Liguori > >> > >> > > >> > By the way, copy stable as well? > >> > Loading it unconditonally is a cross > >> > version migration bug that we probably want to fix > >> > on stable branch - disabling for 1.3.0 and older. > >> > > >> >> --- > >> >> v1 -> v2 > >> >> - Still load external DSDT for q35 > >> >> --- > >> >> hw/i386/pc_piix.c | 1 - > >> >> 1 file changed, 1 deletion(-) > >> >> > >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > >> >> index 95c45b8..311574a 100644 > >> >> --- a/hw/i386/pc_piix.c > >> >> +++ b/hw/i386/pc_piix.c > >> >> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, > >> >> OBJECT(icc_bridge), NULL); > >> >> > >> >> pc_cpus_init(cpu_model, icc_bridge); > >> >> - pc_acpi_init("acpi-dsdt.aml"); > >> >> > >> >> if (kvm_enabled() && kvmclock_enabled) { > >> >> kvmclock_create(); > >> >> -- > >> >> 1.8.0
Applied. Thanks. Regards, Anthony Liguori
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 95c45b8..311574a 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory, OBJECT(icc_bridge), NULL); pc_cpus_init(cpu_model, icc_bridge); - pc_acpi_init("acpi-dsdt.aml"); if (kvm_enabled() && kvmclock_enabled) { kvmclock_create();
This breaks migration and is unneeded with modern SeaBIOS. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- v1 -> v2 - Still load external DSDT for q35 --- hw/i386/pc_piix.c | 1 - 1 file changed, 1 deletion(-)