Re: [PATCH] acpi: Add Windows ACPI Emulated Device Table (WAET)

[Liran Alon <liran.alon@oracle.com>]

On 12/03/2020 18:35, Igor Mammedov wrote:
> On Thu, 12 Mar 2020 14:55:50 +0200
> Liran Alon <liran.alon@oracle.com> wrote:
>
>> On 12/03/2020 14:19, Michael S. Tsirkin wrote:
>>> On Thu, Mar 12, 2020 at 01:30:01PM +0200, Liran Alon wrote:
>>>> On 12/03/2020 8:12, Michael S. Tsirkin wrote:
>>>>> On Thu, Mar 12, 2020 at 01:20:02AM +0200, Liran Alon wrote:
>>>>>> But this is just a good practice in general and in the past it was said by
>>>>>> maintainers that this is one of the main reasons that ACPI and SMBIOS
>>>>>> generation have moved from SeaBIOS to QEMU.
>>>>> I think a flag to disable this might make sense though. For example,
>>>>> some guests might behave differently and get broken.
>>>> Right. That's why I think it's a good practice to have this flag and tie it
>>>> to machine-type.
>>> Tying things to the machine type is not what I had in mind.
>>> A separate flag would also be helpful so users can tweak this
>>> for new machine types, too.
>> I think it's unnecessary, given how common WAET ACPI table is exposed by
>> default by other hypervisors.
>>
>> But if you insist, I can add such flag on a separate commit in v2...
>> Where do you want to have such flag? It cannot be a property of some
>> qdev object.
>> So you want to add a new QEMU_OPTION_no_weat in vl.c?
> If it doesn't break any windows guests we probably don't need an option.
> Can you test if old guests are booting fine with new table, to confirm
> that it's fine? (starting with XPsp3)

Old guests boot fine with the new WAET table.
We are running with this table in production for many years with many 
Windows XP guests (and much more esoteric guests)

Just to verify, I've just now run it with a WinXP SP3 VM and it works 
just fine.
So should I remove the flag completely or remain with the current 
functionality I have that makes sure WAET is only exposed on new 
machine-types?

-Liran

>>>> Guest-visible changes shouldn't be exposed to old machine-types.
>>> Well almost any change in qemu is guest visible to some level.
>>> Even optimizations are guest visible.
>>> We made changes in ACPI without versioning in the past but I'm not
>>> opposed to versioning here. However in that case pls do add a bit
>>> of documentation about why this is done here.
>> I remember that maintainers have explicitly specified that ACPI/SMBIOS
>> should not be changed between machine-types.
>> This have been one of the reasons to move ACPI/SMBIOS generation from
>> SeaBIOS to QEMU control.
>>
>> What can of documentation you want me to add and where?
>> The only thing I can say is that I tie it to machine-type because I do
>> not think a given machine-type should suddenly change BIOS exposed info
>> to guest.
>> But that's kinda generic. I haven't found similar documentation in other
>> ACPI-disable flags to copy from (E.g. do_not_add_smb_acpi).
>>
>>> What I am asking about is whether we need a flag to disable
>>> this as part of the stable interface.
>> I personally think not. But if you think otherwise, can you provide
>> guidance of where you suggest to add this flag?
>> As the only place I see fit is adding a new QEMU_OPTION_no_weat.