On Tue, 4 Jul 2023 20:50:49 +0900 Akihiko Odaki <akihiko.od...@daynix.com> wrote:
> On 2023/07/04 20:38, Igor Mammedov wrote: > > On Sat, 1 Jul 2023 16:28:30 +0900 > > Akihiko Odaki <akihiko.od...@daynix.com> wrote: > > > >> On 2023/07/01 0:29, Michael S. Tsirkin wrote: > >>> On Fri, Jun 30, 2023 at 08:36:38PM +0900, Akihiko Odaki wrote: > >>>> On 2023/06/30 19:37, 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. > >>>> > >>>> I don't think there is one because the code for PCI multifunction does > >>>> not > >>>> care ARI. In my opinion, we need yet another check to make non-SR-IOV > >>>> multifunction and ARI capability mutually exclusive; if a function has > >>>> the > >>>> ARI capability and it is not a VF, an attempt to assign non-zero function > >>>> number for it should fail. > > > > is it stated somewhere in spec(s) that ARI and !SR-IOV are mutually > > exclusive? > > > >>> > >>> Why is that? My understanding is that ARI capable devices should also > >>> set the multifunction bit in the header. It's not terribly clear from > >>> the spec though. > >> > >> Something like the following will not work properly with ARI-capable > >> device (think of a as an ARI-capable device): > >> -device a,addr=0x1.0x0,multifunction=on -device a,addr=0x1.0x1 > > (I had a crazy idea, to use it like that so we could put more devices > > on port without resorting to adding extra bridges) > > > > Can you elaborate some more why it won't work? > > It won't work because the ARI next function number field is fixed. In > this case, the field of the Function at 0x1.0x0 should point to 0x1.0x1, > but it doesn't. As the result, the Function at 0x1.0x1 won't be recognized. > > It's more problematic if some of the Functions are ARI-capable but > others are not. In my understanding, all Functions in a ARI-capable > device need to have ARI capability, but that's not enforced. that doesn't look to me as an real issue but rather as a QEMU problem that we could fix and handle it properly. > >> This is because the next function numbers advertised with ARI are not > >> updated with the multifunction configuration, but they are hardcoded in > >> the device implementation. In this sense, the traditional (non-SR/IOV) > >> multifunction mechanism QEMU has will not work with ARI-capable devices. > >> > >>> > >>>> But it should be a distinct check as it will need to check the function > >>>> number bits. > >>>> > >>>>> > >>>>>> > >>>>>> > >>>>>>>> > >>>>>>>>>> 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 ) :-) > >>>> > >>>> Hello from a guy using a shiny M2 Macbook Air carrying the legacy jack > >>>> with > >>>> a 100-yen earphone. Even people who ported Linux to this machine spent > >>>> efforts to get the jack to work on Linux ;) > >>>> > >>>>>>> > >>>>>>>> > >>>>>>>>>> and the benefit > >>>>>>>>>> is not guaranteed. > >>>>>>>>>> > >>>>>>>>>> -- > >>>>>>>>>> MST > >>>>> > >>> > >> > > >
