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

[Jon Doron <arilou@gmail.com>]

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