1: don't pass bridge devices to domain_context_mapping_one() 2: adjust log messages in domain_context_mapping_one() Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
When passed a non-NULL pdev, the function does an owner check when it finds an already existing context mapping. Bridges, however, don't get passed through to guests, and hence their owner is always going to be Dom0, leading to the assigment of all but one of the function of multi- function PCI devices behind bridges to fail. Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Note: This was reported as an apparent regression from XSA-302 / -306. So far I haven't been able to figure out how the code would have worked before, i.e. to me it looks like a pre-existing problem. This leaves the risk of the change here papering over another issue. --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1499,7 +1499,7 @@ static int domain_context_mapping(struct break; ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn, - pci_get_pdev(seg, bus, devfn)); + NULL); /* * Devices behind PCIe-to-PCI/PCIx bridge may generate different @@ -1509,7 +1509,7 @@ static int domain_context_mapping(struct if ( !ret && pdev_type(seg, bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE && (secbus != pdev->bus || pdev->devfn != 0) ) ret = domain_context_mapping_one(domain, drhd->iommu, secbus, 0, - pci_get_pdev(seg, secbus, 0)); + NULL); break; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com> > Sent: Tuesday, January 7, 2020 3:38 PM > > When passed a non-NULL pdev, the function does an owner check when it > finds an already existing context mapping. Bridges, however, don't get > passed through to guests, and hence their owner is always going to be > Dom0, leading to the assigment of all but one of the function of multi- > function PCI devices behind bridges to fail. > > Reported-by: Marek Marczykowski-Górecki > <marmarek@invisiblethingslab.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Note: This was reported as an apparent regression from XSA-302 / -306. > So far I haven't been able to figure out how the code would have > worked before, i.e. to me it looks like a pre-existing problem. > This leaves the risk of the change here papering over another > issue. > > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -1499,7 +1499,7 @@ static int domain_context_mapping(struct > break; > > ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn, > - pci_get_pdev(seg, bus, devfn)); > + NULL); the relationship between NULL and a bridge device is not obvious by just looking at the code. Can you at least add some comment here, or possibly be clearer by having a mapping_bridge wrapper? > > /* > * Devices behind PCIe-to-PCI/PCIx bridge may generate different > @@ -1509,7 +1509,7 @@ static int domain_context_mapping(struct > if ( !ret && pdev_type(seg, bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE > && > (secbus != pdev->bus || pdev->devfn != 0) ) > ret = domain_context_mapping_one(domain, drhd->iommu, secbus, > 0, > - pci_get_pdev(seg, secbus, 0)); > + NULL); > > break; > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Add missing newlines, use %pd, and drop exclamation marks. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1308,10 +1308,9 @@ int domain_context_mapping_one( if ( pdev->domain != domain ) { printk(XENLOG_G_INFO VTDPREFIX - "d%d: %04x:%02x:%02x.%u owned by d%d!", - domain->domain_id, - seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - pdev->domain ? pdev->domain->domain_id : -1); + "%pd: %04x:%02x:%02x.%u owned by %pd\n", + domain, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), + pdev->domain); res = -EINVAL; } } @@ -1323,16 +1322,15 @@ int domain_context_mapping_one( if ( cdomain < 0 ) { printk(XENLOG_G_WARNING VTDPREFIX - "d%d: %04x:%02x:%02x.%u mapped, but can't find owner!\n", - domain->domain_id, - seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); + "%pd: %04x:%02x:%02x.%u mapped, but can't find owner\n", + domain, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); res = -EINVAL; } else if ( cdomain != domain->domain_id ) { printk(XENLOG_G_INFO VTDPREFIX - "d%d: %04x:%02x:%02x.%u already mapped to d%d!", - domain->domain_id, + "%pd: %04x:%02x:%02x.%u already mapped to d%d\n", + domain, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), cdomain); res = -EINVAL; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com> > Sent: Tuesday, January 7, 2020 3:38 PM > > Add missing newlines, use %pd, and drop exclamation marks. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.