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

[Igor Mammedov]

On Thu, 12 Mar 2020 20:48:48 +0200
Liran Alon <address@hidden> wrote:

> On 12/03/2020 18:35, Igor Mammedov wrote:
> > On Thu, 12 Mar 2020 14:55:50 +0200
> > Liran Alon <address@hidden> 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?
In this case I'd drop flag.

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