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

[Gleb Natapov <gleb@redhat.com>]

On Sun, May 20, 2012 at 03:47:02PM +0300, Avi Kivity wrote:
> On 05/20/2012 03:36 PM, Gleb Natapov wrote:
> > On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote:
> > > On 05/20/2012 03:15 PM, Gleb Natapov wrote:
> > > > On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> > > > > On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > > > > > QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> > > > > > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > > > > > option was added that passes supported system states and what value should
> > > > > > guest use to enter each state. States are passed in 6 byte array. Each
> > > > > > byte represents one system state. If byte at offset X has its MSB set
> > > > > > it means that system state X is supported and to enter it guest should
> > > > > > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > > > > > values that work in backwards compatible way there.
> > > > > >
> > > > > 
> > > > > 
> > > > > 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?

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

> > 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.
 
> 
> (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. I do not see advantage of this method
and it requires patching still.

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

--
			Gleb.