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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jan 20, 2015 at 01:41:16PM +0100, Igor Mammedov wrote:
> On Tue, 20 Jan 2015 12:35:47 +0200
> "Michael S. Tsirkin" <mst@redhat.com> 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" <mst@redhat.com> 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.

Exactly.
We can reuse the original rom blob but it's tricky given
existing APIs.

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