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.

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

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



Reply via email to