On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
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.
AML is a data provided by firmware. AML's runtime is different from firmware's.
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()... }
?
syntax error, unexpected PARSEOP_IF
(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.
What's the benefit? The patching is still needed. You need to pass address of OperationRegion() to AML. You can do it either by patching or by creating OperationRegion() code dynamically.
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.
If we can, we should, but we can't as far as I see. The patching was here long before this patch.
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.
Does not work for me, can you send me a patch that works?
-- Gleb.