Re: [Qemu-devel] [PATCH v2] pc: drop external DSDT loading

[Michael S. Tsirkin]

On Mon, Aug 12, 2013 at 02:59:24PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
> 
> > On Mon, Aug 12, 2013 at 10:20:41AM -0500, Anthony Liguori wrote:
> >> "Michael S. Tsirkin" <address@hidden> 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 <address@hidden>
> >> >
> >> > 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 <address@hidden>

> >
> >
> >> 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