On Mon, May 16, 2016 at 09:44:28AM -0600, Alex Williamson wrote: > On Mon, 16 May 2016 17:58:18 +0800 > Peter Xu <pet...@redhat.com> wrote: > > > On Mon, May 16, 2016 at 12:21:54PM +0300, Michael S. Tsirkin wrote: > > [...] > > > > "Legacy PCI bus, override requester ID with the bridge's BDF > > > > upstream. The root complex of legacy PCI system can only get > > > > requester ID from directly attached devices (including bridges). > > > > > > When do legacy pci systems use requester id at all? > > > PCI spec does not mention this concept. > > > > I see some descriptions about this in vt-d spec, e.g., chap > > 3.9.2. Maybe somewhere else too, but I cannot remember. In the spec, > > it is told something like: > > > > "...the source-id in the DMA requests is the requester-id of the > > bridge device..." > > > > Similar thing on interrupt remapping desc in 5.1.1. > > > > Actually I am curious about how generic PCI system delivers > > requester ID (if there is)... For PCIe, we have encoded TLP header, > > and requester ID is filled in the specific field of the header. > > However for legacy PCI system, all the data is transmitted via the > > parallel interface (no matter 32/64 bits) and I found no place that > > the requester ID can be included. I was assuming there is some way > > for the root complex to know it (when request comes, the root > > complex should be able to know where the request come from, or say, > > its connected BDF). Never digger into the details, or am I wrong? > > There's no such thing as a requester ID on conventional PCI. We should > probably be making use of pci_bus_is_express() to determine whether we > have a valid requester ID and error if we hit pci_bus_is_root() and we > still don't have an express bus. And as MST says, testing for > bus number zero is not a valid test for the root bus. Thanks, > > Alex
Well MSI code sticks the requester ID in the MSI message unconditionally. It's typically later ignored by the the PC machine type though. > > > > > > > If > > > > devices are attached under specific bridge (no matter > > > > > > should be "no matter if" > > > > > > > there are one > > > > or more bridges), only the requester ID of the bridge that directly > > > > > > should be "that is directly" > > > > Will fix above two. > > > > > > > > > attached to the root complex can be recognized." > > > > > > > > > > > > > > > + result = pci_get_bdf(dev); > > > > > > > > > > Won't dev be NULL for a root bus? > > > > > > > > Should not. The above while() is checking whether dev's parent bus > > > > number (N) is zero, > > > > > > OK but from pci perspective it's not a given that it's zero. > > > I think it isn't for pci expander. > > > BTW did you try this with expander bridges? > > > > Nop... I used the same test in as in v1 (Radim's one, with IR > > patchset applied, since until now IR seems the only one that uses > > this field), since I found it hard to cover all the combinations > > (include different PCI/PCIX/PCIe buses, PCI/PCIe devices, and all > > kinds of topologies, etc.). Do you think I should do thorough tests > > for this change? If so, do you have suggestion on which test cases I > > should (at least) cover? > > > > > > > > > and reach here only if it's non-zero. Here, dev > > > > is already re-used to store the PCIDevice struct for the bus device, > > > > whose secondary bus number is N (as checked in the while > > > > condition). So it should never be the root pci bus (which has > > > > so-called secondary bus number 0). > > > > > > Pls don't make this assumption. If you want to know > > > whether it's a root, call pci_bus_is_root. > > > > Ah, yes, I should use that. > > > > Thanks, > > > > -- peterx
