Re: [PATCH v4 5/6] i386: Hyper-V VMBus ACPI DSDT entry

[Jon Doron <arilou@gmail.com>]

On 14/06/2020, Jon Doron wrote:
>On 28/05/2020, Jon Doron wrote:
>>On 28/05/2020, Igor Mammedov wrote:
>>>On Thu, 28 May 2020 08:26:42 +0300
>>>Jon Doron <arilou@gmail.com> wrote:
>>>
>>>>On 22/05/2020, Igor Mammedow wrote:
>>>>>On Thu, 21 May 2020 18:02:07 +0200
>>>>>Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>
>>>>>>On 13/05/20 17:34, Igor Mammedov wrote:
>>>>>>> I'd rather avoid using random IRQ numbers (considering we are
>>>>>>> dealing with black-box here). So if it's really necessary to have
>>>>>>> IRQ described here, I'd suggest to implement them in device model
>>>>>>> so they would be reserved and QEMU would error out in a sane way if
>>>>>>> IRQ conflict is detected.
>>>>>>
>>>>>>We don't generally detect ISA IRQ conflicts though, do we?
>>>>>
>>>>>that I don't know that's why I'm not suggesting how to do it.
>>>>>The point is hard-coding in AML random IRQs is not right thing to do,
>>>>>(especially with the lack of 'any' spec), as minimum AML should pull
>>>>>it from device model and that probably should be configurable and set
>>>>>by board.
>>>>>
>>>>>Other thing is:
>>>>>I haven't looked at VMBus device model in detail, but DSDT part aren't
>>>>>matching device though (device model is not ISA device hence AML part
>>>>>shouldn't be on in ISA scope), where to put it is open question.
>>>>>There were other issues with AML code, I've commented on, so I was
>>>>>waiting on respin with comments addressed.
>>>>>I don't think that this patch is good enough for merging.
>>>>>
>>>>>
>>>>
>>>>But it seems like the current patch does match what's Microsoft HyperV
>>>>is publishing in it's APCI tables.
>>>>
>>>>I dont think it's correct for us to "fix" Microsoft emulation even if
>>>>it's wrong, since that's what Windows probably expects to see...
>>>>
>>>>I tried looking where Microsoft uses the ACPI tables to identify the
>>>>VMBus but without much luck in order to understand how flexible a change
>>>>would be for the OS to still detect the VMBus device, but in general
>>>>I think "correcting" something that is emulated 1:1 because there is no
>>>>spec is the right way.
>>>
>>>I'd agree, if removing nonsense would break VMBus detection (does it?).
>>>if something is that doesn't make sense but has to stay because it is need
>>>to make windows happy, that's fine , just add annotate is with comment,
>>>so it won't confuse anyone why that code exists there later on.
>>>
>>>I suggest to:
>>>1. try dropping _PS* & _STA as it doesn't actually does anything and _PS3 is plain wrong
>>>2. drop one IRQ, newer hyper-v seems to be doing fine with only one
>>>3. it's not ISA device, I'd suggest to move into _SB scope
>>>4. I don't know much about IRQs but
>>>     git grep DEFINE_PROP_ | grep -i iqr
>>>  yields nothing so I'm not sure if it's acceptable. Typically it's board that assigns
>>>  IRQ and not device, for Sysbus devices (see: sysbus_init_irq/sysbus_connect_irq).
>>>  So I'd leave it upto Paolo or someone else to decide/comment on.
>>>
>>
>>Sounds like a plan, I'll try to come up with the test results
>>(at least for Windows 10 guest which is  what I have setup) and update
>>this thread with the results.
>>
>>-- Jon.
>>
>>>>
>>>>>>
>>>>>>Paolo
>>>>>>
>>>>>
>>>>
>>>
>Hi guys,
>
>Sorry for the delay...
>
>So first ill clarify what was the test, the test was to see the device
>"Microsoft Hyper-V Virtual Machine Bus" in Windows Device Manager under
>"System devices" with a state of "working properly".
>
>It seems like it's ok to drop all the _PS* and _STA.
>
>It seems to be functioning with single IRQ as well, it is worth noting 
>that even when i dropped the entire _CRS (so no IRQs resources are 
>required, the device was still showing that it's functioning, but I 
>suspect this might affect the child devices like hv-net and hv-scsi).
>
>With that said I did run into a small issue I set-up Win10 1903 (aka 
>19H1) and it seems like VMBus now requires to have the following 
>features enabled:
>HV_VP_RUNTIME_AVAILABLE
>HV_TIME_REF_COUNT_AVAILABLE
>HV_SYNIC_AVAILABLE
>HV_SYNTIMERS_AVAILABLE
>HV_APIC_ACCESS_AVAILABLE
>HV_HYPERCALL_AVAILABLE
>HV_VP_INDEX_AVAILABLE
>
>So notice that previously only SYNIC and VPINDEX was needed, now you 
>need the whole thing so you need to run qemu with something like
>-cpu host,hv-relaxed,hv_spinlocks=0x1fff,hv_time,hv-vapic,hv-vpindex,hv-synic,hv-runtime,hv-stimer
>
>The validation was done in winhv!WinHvpCheckPartitionPrivileges .
>
>Paolo I noticed you have done a PULL request, would you like to wait 
>on it and we will submit a version with a single IRQ (selectable by 
>user property) and go with Igor's suggestion dropping _PS* and _STA 
>(though like I said before I prefer to mimic the original HyperV with 
>it's bugs, but I'll leave this decision to you).
>
>Also today VMBus only verifies SYNIC is enabled I'm not sure how but I 
>wonder if we want to some how exports from the CPU which other HV 
>features are enabled so we can verify all the required ones are set, 
>would appreciate if you have any suggestions here.
>
>Cheers,
>-- Jon.

I got the latest DSDT from one of the latest builds 19041

     Device (\_SB.VMOD.VMBS)
     {
         Name (STA, 0x0F)
         Name (_ADR, Zero)  // _ADR: Address
         Name (_DDN, "VMBUS")  // _DDN: DOS Device Name
         Name (_HID, "VMBus")  // _HID: Hardware ID
         Name (_UID, Zero)  // _UID: Unique ID
         Method (_DIS, 0, NotSerialized)  // _DIS: Disable Device
         {
             STA &= 0x0D
         }

         Method (_PS0, 0, NotSerialized)  // _PS0: Power State 0
         {
             STA |= 0x0F
         }

         Method (_STA, 0, NotSerialized)  // _STA: Status
         {
             Return (STA) /* \_SB_.VMOD.VMBS.STA_ */
         }

         Name (_PS3, Zero)  // _PS3: Power State 3
         Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
         {
             IRQ (Edge, ActiveHigh, Exclusive, )
                 {5}
         })
     }

So things looks the same...