Re: [Qemu-devel] [PATCH 09/11] ACPI: move PRST OperationRegion into SSDT

[Michael S. Tsirkin]

On Mon, Dec 16, 2013 at 10:53:24PM +0100, Igor Mammedov wrote:
> On Mon, 16 Dec 2013 22:13:30 +0100
> Laszlo Ersek <address@hidden> wrote:
> 
> > On 12/16/13 21:38, Igor Mammedov wrote:
> > > On Mon, 16 Dec 2013 21:30:14 +0200
> > > "Michael S. Tsirkin" <address@hidden> wrote:
> > > 
> > >> On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote:
> > >>> .. and report range used by it to OSPM via _CRS.
> > >>> PRST is needed in SSDT since its base will depend on
> > >>> chipset and will be dynamically set by QEMU.
> > >>> Also move PRSC() method along with PRST since cross
> > >>> table reference to PRST doesn't work.
> > >>
> > >> Could you clarify this last sentence?
> > >> I don't mind where it is but I'd like to know
> > >> where does the limitation come from.
> > > It's empiric deduction so far I haven't found such limitation in spec yet.
> > > iasl builds tables just fine but neither linux nor windows were able to 
> > > find
> > > Operation region from SSDT when loading DSDT, failing whole table loading
> > > process. Decompiling DSDT/SSDT tables in guest shows that region is in
> > > expected scope but OSPM refuses to see it when referenced outside SSDT.
> > 
> > There seem to be four things here:
> > - the OperationRegion definition,
> > - its external declaration,
> > - the Field() declaration,
> > - use of fields.
> > 
> > I think referencing an OperationRegion defined in another table should
> > work (by way of External). I suspect the tricky part is with Field():
>                   ^^^ it looks like it should work and decompiled tables
> look fine as well but it unfortunately doesn't.
> 
> > 
> >     The fields are parts of the object named by RegionName, but their
> >     names appear in the same scope as the Field term.
> > 
> > So,
> > - maybe moving PRST only, and leaving the definition of PRS (as part of
> > Field()) together with PRSC would suffice,
> That was the first thing I've tried.
> 
> > - or, after moving the definition of PRS (as part of Field()) together
> > with PRST to another table, all references to PRS (in the PRSC method)
> > would have to be qualified. (But I guess this is what you tried.)
> yep, that didn't work too.
> 
> I'm not fun of moving a bunch of code around, but looks like there is
> no other way. I'd be happy to try if there are any other suggestions.

I'm not sure we need to spend too much time on it.
Just clarifying that 'doesn't work' means 'builds but
makes OSPM fail to resolve the OperationRegion,
even though the spec makes it look like this should work'
in the commit log should be enough.

> > 
> > Laszlo
> > 
> 
> 
> -- 
> Regards,
>   Igor