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