Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT

[Igor Mammedov]

On Tue, 20 Jan 2015 12:35:47 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> On Tue, Jan 20, 2015 at 10:59:43AM +0100, Igor Mammedov wrote:
> > On Mon, 19 Jan 2015 21:29:57 +0200
> > "Michael S. Tsirkin" <address@hidden> wrote:
> > 
> > > On Mon, Jan 19, 2015 at 06:26:55PM +0100, Paolo Bonzini wrote:
> > > > 
> > > > 
> > > > On 19/01/2015 18:14, Igor Mammedov wrote:
> > > > > I'm fine with moving "SMC out of the per-machine-type AML", should be
> > > > > a separate patch anyway. But patch-able SMC being in DSDT is our 
> > > > > mistake
> > > > > that we allowed it to slip there and should be better moved to SSDT 
> > > > > rather
> > > > > than staying in DSDT and making thing more complex.
> > > > > It's also candidate for trimming, i.e. dropping it from tables 
> > > > > altogether
> > > > > if device is not present in QEMU, same applies to _S[34] Packages when
> > > > > respective features are disabled and to PEVT device template.
> > > > 
> > > > Yes, trimming is better than putting it in the DSDT, at least for simple
> > > > devices such as SMC and pvpanic.
> > So are we dropping 1-2/4 from this series?
> > I need to know on top of what to rebase. I'll take care of moving SMC to 
> > SSDT.
> > 
> > > > 
> > > > >> > 
> > > > >>>> > >> simpler.  However, it also complicates backwards 
> > > > >>>> > >> compatibility, so
> > > > >>>> > >> merge it with the DSDT.
> > > > >>> > > What are these complications?
> > > > >> > 
> > > > >> > The complication arises if we want to make the SSDT exactly the 
> > > > >> > same for
> > > > >> > all QEMU versions, given a (machine type, command line) pair.  
> > > > >> > Then you
> > > > >> > either cannot do any change to ssdt-misc, or you have to keep 
> > > > >> > different
> > > > >> > copies for each machine type.
> > > > > With resizable ROM blobs in master, there shouldn't be an issue with
> > > > > migration in new QEMU versions if size of SSDT changes.
> > > > 
> > > > There is only a very small issue that remains (the RSDP pointer is wrong
> > > > if the size changes),
> > > 
> > > Yes - for new machine types I'll send a patch to put it
> > > in memory.
> > > For old ones - there's a race, and it's painful to fix.  If we do want
> > > to try fixing it, one solution is to fail migration if attempted before
> > > rsdp is shadowed. Useful?
> > There were my patches on list that move RSDT at the start of blob,
> > which fixes issue for new machine types.
> 
> I don't see the point - IMO for new machine types, we can just put RSDP
> in a memory region, have it migrated.
you mean to put it into ROM blob, that should will cover not only migration
issue but also reboot after bridge hotplug, since updated RSDP will be used.

> 
> > That patches however
> > weren't doing good job for old machine types. I can respin that series
> > fixing new machines and we can fix old machines in separate patch later.
> 
> I don't think it's worth it since I don't see an easy way for old
> machine types.  A harder way would be to allow rom files to share an MR.
> We could then stick RSDP at the tail of the MR, and look for it on
> incoming migration: if there, fix it up.
> 
> Needs reworking of rom_add_blob API.
> 
> > > 
> > > > so we probably should apply anyway the patch of
> > > > mine that allows the DSDT size to change; and we probably should pay
> > > > attention to SSDT, and version it.
> > > > 
> > > > ("Let's just ignore the SSDT" was exactly what I feared when I disagreed
> > > > with putting in resizable ROM blobs first.  But now that it's in, I
> > > > cannot really argue otherwise).
> > > 
> > > I don't have a strong opinion here. you guys arrive
> > > at a rough consensus.:w
> > > 
> > > 
> > > > > So question is if we still need SSDT version-ing and per machine type
> > > > > SSDT compatibility? /it's better not to do version-ing at all if it 
> > > > > could
> > > > > be avoided, due to maintenance headache it brings along/
> > > > 
> > > > I'm okay with re-evaluating that after your patches go in.
> > > > 
> > > > Paolo