> On 30-Jun-2023, at 4:10 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>
> On Fri, Jun 30, 2023 at 04:07:32PM +0530, Ani Sinha wrote:
>>
>>
>>> On 30-Jun-2023, at 3:30 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>>>
>>> On Fri, Jun 30, 2023 at 02:52:52PM +0530, Ani Sinha wrote:
>>>>
>>>>
>>>>> On 30-Jun-2023, at 2:13 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>>>>>
>>>>> On Fri, Jun 30, 2023 at 02:06:59PM +0530, Ani Sinha wrote:
>>>>>>
>>>>>>
>>>>>>> On 30-Jun-2023, at 2:02 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>>>>>>>
>>>>>>> On Fri, Jun 30, 2023 at 01:11:33PM +0530, Ani Sinha wrote:
>>>>>>>>>
>>>>>>>>> Thus the check for unoccupied function 0 needs to use pci_is_vf()
>>>>>>>>> instead of checking ARI capability, and that can happen in
>>>>>>>>> do_pci_register_device().
>>>>>>>>>
>>>>>>>>>> Also where do you propose we move the check?
>>>>>>>>>
>>>>>>>>> In pci_qdev_realize(), somewhere after pc->realize() and before
>>>>>>>>> option ROM loading.
>>>>>>>>
>>>>>>>> Hmm, I tried this. The issue here is something like this would be now
>>>>>>>> allowed since the PF has ARI capability:
>>>>>>>>
>>>>>>>> -device pcie-root-port,id=p -device igb,bus=p,addr=0x2.0x0
>>>>>>>>
>>>>>>>> The above should not be allowed and when used, we do not see the igb
>>>>>>>> ethernet device from the guest OS.
>>>>>>>
>>>>>>> I think it's allowed because it expects you to hotplug function 0 later,
>>>>>>
>>>>>> This is about the igb device being plugged into the non-zero slot of the
>>>>>> pci-root-port. The guest OS ignores it.
>>>>>
>>>>> yes but if you later add a device with ARI and with next field pointing
>>>>> slot 2 guest will suddently find both.
>>>>
>>>> Hmm, I tried this:
>>>>
>>>> -device pcie-root-port,id=p \
>>>> -device igb,bus=p,addr=0x2.0x0 \
>>>> -device igb,bus=p,addr=0x0.0x0 \
>>>>
>>>> The guest only found the second igb device not the first. You can try too.
>>>
>>> Because next parameter in pcie_ari_init does not match.
>>
>> OK send me a command line that I can test it with. I can’t come up with a
>> case that actually works in practice.
>
>
> you need to patch igb to pass 2 as next parameter.
> maybe add a property to make it easier to play with.
Yes but without patching, could I not change the device parameters like
-device pcie-root-port,id=p \
-device igb,bus=p,addr=0x1.0x0 \
-device igb,bus=p,addr=0x0.0x0 \
I tried the above and it did not work.
>
>>>
>>>
>>>>>
>>>>>>> no?
>>>>>>>
>>>>>>> I am quite worried about all this work going into blocking
>>>>>>> what we think is disallowed configurations. We should have
>>>>>>> maybe blocked them originally, but now that we didn't
>>>>>>> there's a non zero chance of regressions,
>>>>>>
>>>>>> Sigh,
>>>>>
>>>>> There's value in patches 1-4 I think - the last patch helped you find
>>>>> these. so there's value in this work.
>>>>>
>>>>>> no medals here for being brave :-)
>>>>>
>>>>> Try removing support for a 3.5mm jack next. Oh wait ...
>>>>
>>>> Indeed. Everyone uses bluetooth these days. I for one is happy that the
>>>> jack is gone (and they were bold enough to do it while Samsung and others
>>>> still carry the useless port ) :-)
>>>>
>>>>>
>>>>>>> and the benefit
>>>>>>> is not guaranteed.
>>>>>>>
>>>>>>> --
>>>>>>> MST