Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity <avi@redhat.com>]

On 05/20/2012 03:59 PM, Gleb Natapov wrote:
> > > > > > 
> > > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > > > > > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > > > > > 
> > > > > We better not talk to fwcfg after OSPM is started since this is firmware
> > > > > confing interface.
> > > > 
> > > > Why not?  The OS isn't going to talk to it, so we can have a driver in ACPI.
> > > > 
> > > The OS is going to talk to it since the OS is the one who interprets
> > > AML. 
> > 
> > I meant, not directly.  So the driver in ACPI has exclusive access.
> > 
> What's the difference?

ACPI is firmware, not OS.

> > > We may want to disable fwcfg after OS bootup at all in the feature.
> > > Who knows what kind of sensitive information we may want to pass by it
> > > in the feature? May be something TPM related? 
> > 
> > fwcfg is for passing information to the guest.  If you want to hide
> > something from the guest, just don't put it in fwcfg.
> > 
> Where to put it if we want to pass it to a firmware, but not an OS.
> That was the point of fwcfg. If you want to pass something to a guest OS
> use virtio-serial.

See above.

> > > And I do not see any advantage
> > > of using fwcfg from AML.
> > 
> > It's an alternative to patching AML.  Sure it takes some effort to write
> > the driver, but afterwards we can modify the guest behaviour more
> > easily.  One possible client is -M old, so you can revert to previous
> > behaviour depending on fwcfg data.
> -M old is easy to support with the current patch. You just set new
> properties to compatibility values. The code is written with this in
> mind. And this is not an alternative to patching AML as I am trying to
> explain to you below. You can eliminate patching of s4 value, but that's
> it, you still need to patch out _S3/_S4 names.

What about

  If (Fcfg(...)) {
        Method()...
  }

?

(i.e.. define the method conditionally at runtime)

>  
> > 
> > (we don't need a driver in AML to avoid patching, we can have AML talk
> > to the bios and the bios drive fwcfg; but I think we'll find uses for a
> > driver).
> I am not sure what you mean. AML can't talk to the bios. It can read
> values that bios put somewhere. 

That's what I meant - communicate through memory.

> I do not see advantage of this method
> and it requires patching still.

For the existence of the names?  Yes, if we can't avoid it it's a
problem.  But if we can avoid patching, we should.

> > 
> > >
> > > > >  Regardless, presence of _S3 name or method is all
> > > > > that needed for OS enabling S3 option. If _S3 is defined as a method it
> > > > > has to return Package() otherwise iasl refuses to compile it.
> > > > 
> > > > Can't we Return (Package (...) { ... }) or equivalent? 
> > > > 
> > > We can, how does it help?
> > >
> > 
> > The contents of the package can be determined at runtime.
> > 
> And? _S3 name should not exists at all in order to disable S3, not return
> something different.
>

See above.

-- 
error compiling committee.c: too many arguments to function