| Message ID | 1462948831-931-2-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 11, 2016 at 02:40:31PM +0800, Peter Xu wrote: > When there are devices under PCI bridge (or bridges), PCI requester ID > should be the one that hooked on the root PCI bus, not the PCI device > itself. > > Signed-off-by: Peter Xu <peterx@redhat.com> I think this is only correct for pci bridges, and wrong for pci express bridges. How exactly do you test this? > --- > hw/pci/msi.c | 2 +- > hw/pci/pci.c | 9 +++++++++ > include/hw/pci/pci.h | 2 ++ > 3 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/msi.c b/hw/pci/msi.c > index e0e64c2..1719716 100644 > --- a/hw/pci/msi.c > +++ b/hw/pci/msi.c > @@ -315,7 +315,7 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg) > { > MemTxAttrs attrs = {}; > > - attrs.requester_id = pci_requester_id(dev); > + attrs.requester_id = pci_requester_id_recursive(dev); > address_space_stl_le(&dev->bus_master_as, msg.address, msg.data, > attrs, NULL); > } > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index bb605ef..c14299b 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2498,6 +2498,15 @@ PCIDevice *pci_get_function_0(PCIDevice *pci_dev) > } > } > > +uint16_t pci_requester_id_recursive(PCIDevice *dev) > +{ > + while (pci_bus_num(dev->bus)) { > + /* This is not on root PCI bus, we find its parent */ > + dev = dev->bus->parent_dev; > + } > + return pci_requester_id(dev); > +} > + > static const TypeInfo pci_device_type_info = { > .name = TYPE_PCI_DEVICE, > .parent = TYPE_DEVICE, > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index ef6ba51..4cb5b50 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -776,4 +776,6 @@ extern const VMStateDescription vmstate_pci_device; > .offset = vmstate_offset_pointer(_state, _field, PCIDevice), \ > } > > +uint16_t pci_requester_id_recursive(PCIDevice *dev); > + > #endif > -- > 2.4.11
On Wed, May 11, 2016 at 04:53:54PM +0300, Michael S. Tsirkin wrote: > On Wed, May 11, 2016 at 02:40:31PM +0800, Peter Xu wrote: > > When there are devices under PCI bridge (or bridges), PCI requester ID > > should be the one that hooked on the root PCI bus, not the PCI device > > itself. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > I think this is only correct for pci bridges, and wrong for pci express bridges. > > How exactly do you test this? I was using Radim's test case: bin=x86_64-softmmu/qemu-system-x86_64 $bin -machine q35,iommu=on,intremap=on,kernel-irqchip=split \ -smp cpus=2 -m 1024 -enable-kvm \ -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ -netdev user,id=user.0,hostfwd=tcp::5555-:22 \ -device e1000,netdev=user.0,bus=pci.2,addr=0x1 \ -drive file=/var/lib/libvirt/images/vm1.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,scsi=off,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 This does not boot on v6 series, but can boot with the patch mentioned. Do you know where I can find any document on related topics? Thanks in advance. -- peterx
On Thu, 12 May 2016 10:40:57 +0800 Peter Xu <peterx@redhat.com> wrote: > On Wed, May 11, 2016 at 04:53:54PM +0300, Michael S. Tsirkin wrote: > > On Wed, May 11, 2016 at 02:40:31PM +0800, Peter Xu wrote: > > > When there are devices under PCI bridge (or bridges), PCI requester ID > > > should be the one that hooked on the root PCI bus, not the PCI device > > > itself. > > > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > > > I think this is only correct for pci bridges, and wrong for pci express bridges. > > > > How exactly do you test this? > > I was using Radim's test case: > > bin=x86_64-softmmu/qemu-system-x86_64 > $bin -machine q35,iommu=on,intremap=on,kernel-irqchip=split \ > -smp cpus=2 -m 1024 -enable-kvm \ > -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ > -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ > -netdev user,id=user.0,hostfwd=tcp::5555-:22 \ > -device e1000,netdev=user.0,bus=pci.2,addr=0x1 \ > -drive file=/var/lib/libvirt/images/vm1.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ > -device virtio-blk-pci,scsi=off,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 > > This does not boot on v6 series, but can boot with the patch > mentioned. > > Do you know where I can find any document on related topics? PCI Express to PCI/PCI-X Bridge Specification rev 1.0 2.3 Assignment of Requester ID and Tag by the Bridge PCIe-to-PCI bridges assign a requester ID composed of the secondary bus number with devfn = 0. Although often on real hardware, the root complex PCI bridge uses the actual bridge requester ID even though it's actually a PCIe bridge. Linux assume that if a bridge has a PCIe capability with type PCIe-to-PCI/X bridge we use the secondary bus requester ID, if it has a PCIe capability with type PCI/X-to-PCIe, we use the bridge requester ID. If it does not have a PCIe capability we use the bridge ID except for a few quirked devices known to use the secondary bus ID. Yay standards! Thanks, Alex
On Wed, May 11, 2016 at 09:22:03PM -0600, Alex Williamson wrote: [...] > PCI Express to PCI/PCI-X Bridge Specification rev 1.0 > 2.3 Assignment of Requester ID and Tag by the Bridge > > PCIe-to-PCI bridges assign a requester ID composed of the secondary bus > number with devfn = 0. Although often on real hardware, the root > complex PCI bridge uses the actual bridge requester ID even though > it's actually a PCIe bridge. Linux assume that if a bridge has a PCIe > capability with type PCIe-to-PCI/X bridge we use the secondary bus > requester ID, if it has a PCIe capability with type PCI/X-to-PCIe, we > use the bridge requester ID. If it does not have a PCIe capability we > use the bridge ID except for a few quirked devices known to use the > secondary bus ID. Yay standards! Thanks, Thanks Alex! I have found pci and pci-to-pci bridge specs, which seems useful to me. However, I still cannot find pcie-to-pci bridge spec online (as you have mentioned above). Is that only for registered users? -- peterx
diff --git a/hw/pci/msi.c b/hw/pci/msi.c index e0e64c2..1719716 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -315,7 +315,7 @@ void msi_send_message(PCIDevice *dev, MSIMessage msg) { MemTxAttrs attrs = {}; - attrs.requester_id = pci_requester_id(dev); + attrs.requester_id = pci_requester_id_recursive(dev); address_space_stl_le(&dev->bus_master_as, msg.address, msg.data, attrs, NULL); } diff --git a/hw/pci/pci.c b/hw/pci/pci.c index bb605ef..c14299b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2498,6 +2498,15 @@ PCIDevice *pci_get_function_0(PCIDevice *pci_dev) } } +uint16_t pci_requester_id_recursive(PCIDevice *dev) +{ + while (pci_bus_num(dev->bus)) { + /* This is not on root PCI bus, we find its parent */ + dev = dev->bus->parent_dev; + } + return pci_requester_id(dev); +} + static const TypeInfo pci_device_type_info = { .name = TYPE_PCI_DEVICE, .parent = TYPE_DEVICE, diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index ef6ba51..4cb5b50 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -776,4 +776,6 @@ extern const VMStateDescription vmstate_pci_device; .offset = vmstate_offset_pointer(_state, _field, PCIDevice), \ } +uint16_t pci_requester_id_recursive(PCIDevice *dev); + #endif
When there are devices under PCI bridge (or bridges), PCI requester ID should be the one that hooked on the root PCI bus, not the PCI device itself. Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/pci/msi.c | 2 +- hw/pci/pci.c | 9 +++++++++ include/hw/pci/pci.h | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-)