| Message ID | 1595263380-209956-1-git-send-email-ashok.raj@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | PCI/ATS: PASID and PRI are only enumerated in PF devices. | expand |
Hi Ashok,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pci/next]
[also build test ERROR on iommu/next linux/master linus/master v5.8-rc6 next-20200720]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ashok-Raj/PCI-ATS-PASID-and-PRI-are-only-enumerated-in-PF-devices/20200721-004510
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-kexec (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/iommu/intel/iommu.c: In function 'dmar_insert_one_dev_info':
>> drivers/iommu/intel/iommu.c:2557:8: error: implicit declaration of function 'pci_pri_supported'; did you mean 'pci_ats_supported'? [-Werror=implicit-function-declaration]
2557 | pci_pri_supported(pdev))
| ^~~~~~~~~~~~~~~~~
| pci_ats_supported
cc1: some warnings being treated as errors
vim +2557 drivers/iommu/intel/iommu.c
2504
2505 static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
2506 int bus, int devfn,
2507 struct device *dev,
2508 struct dmar_domain *domain)
2509 {
2510 struct dmar_domain *found = NULL;
2511 struct device_domain_info *info;
2512 unsigned long flags;
2513 int ret;
2514
2515 info = alloc_devinfo_mem();
2516 if (!info)
2517 return NULL;
2518
2519 if (!dev_is_real_dma_subdevice(dev)) {
2520 info->bus = bus;
2521 info->devfn = devfn;
2522 info->segment = iommu->segment;
2523 } else {
2524 struct pci_dev *pdev = to_pci_dev(dev);
2525
2526 info->bus = pdev->bus->number;
2527 info->devfn = pdev->devfn;
2528 info->segment = pci_domain_nr(pdev->bus);
2529 }
2530
2531 info->ats_supported = info->pasid_supported = info->pri_supported = 0;
2532 info->ats_enabled = info->pasid_enabled = info->pri_enabled = 0;
2533 info->ats_qdep = 0;
2534 info->dev = dev;
2535 info->domain = domain;
2536 info->iommu = iommu;
2537 info->pasid_table = NULL;
2538 info->auxd_enabled = 0;
2539 INIT_LIST_HEAD(&info->auxiliary_domains);
2540
2541 if (dev && dev_is_pci(dev)) {
2542 struct pci_dev *pdev = to_pci_dev(info->dev);
2543
2544 if (ecap_dev_iotlb_support(iommu->ecap) &&
2545 pci_ats_supported(pdev) &&
2546 dmar_find_matched_atsr_unit(pdev))
2547 info->ats_supported = 1;
2548
2549 if (sm_supported(iommu)) {
2550 if (pasid_supported(iommu)) {
2551 int features = pci_pasid_features(pdev);
2552 if (features >= 0)
2553 info->pasid_supported = features | 1;
2554 }
2555
2556 if (info->ats_supported && ecap_prs(iommu->ecap) &&
> 2557 pci_pri_supported(pdev))
2558 info->pri_supported = 1;
2559 }
2560 }
2561
2562 spin_lock_irqsave(&device_domain_lock, flags);
2563 if (dev)
2564 found = find_domain(dev);
2565
2566 if (!found) {
2567 struct device_domain_info *info2;
2568 info2 = dmar_search_domain_by_dev_info(info->segment, info->bus,
2569 info->devfn);
2570 if (info2) {
2571 found = info2->domain;
2572 info2->dev = dev;
2573 }
2574 }
2575
2576 if (found) {
2577 spin_unlock_irqrestore(&device_domain_lock, flags);
2578 free_devinfo_mem(info);
2579 /* Caller must free the original domain */
2580 return found;
2581 }
2582
2583 spin_lock(&iommu->lock);
2584 ret = domain_attach_iommu(domain, iommu);
2585 spin_unlock(&iommu->lock);
2586
2587 if (ret) {
2588 spin_unlock_irqrestore(&device_domain_lock, flags);
2589 free_devinfo_mem(info);
2590 return NULL;
2591 }
2592
2593 list_add(&info->link, &domain->devices);
2594 list_add(&info->global, &device_domain_list);
2595 if (dev)
2596 dev->archdata.iommu = info;
2597 spin_unlock_irqrestore(&device_domain_lock, flags);
2598
2599 /* PASID table is mandatory for a PCI device in scalable mode. */
2600 if (dev && dev_is_pci(dev) && sm_supported(iommu)) {
2601 ret = intel_pasid_alloc_table(dev);
2602 if (ret) {
2603 dev_err(dev, "PASID table allocation failed\n");
2604 dmar_remove_one_dev_info(dev);
2605 return NULL;
2606 }
2607
2608 /* Setup the PASID entry for requests without PASID: */
2609 spin_lock(&iommu->lock);
2610 if (hw_pass_through && domain_type_is_si(domain))
2611 ret = intel_pasid_setup_pass_through(iommu, domain,
2612 dev, PASID_RID2PASID);
2613 else if (domain_use_first_level(domain))
2614 ret = domain_setup_first_level(iommu, domain, dev,
2615 PASID_RID2PASID);
2616 else
2617 ret = intel_pasid_setup_second_level(iommu, domain,
2618 dev, PASID_RID2PASID);
2619 spin_unlock(&iommu->lock);
2620 if (ret) {
2621 dev_err(dev, "Setup RID2PASID failed\n");
2622 dmar_remove_one_dev_info(dev);
2623 return NULL;
2624 }
2625 }
2626
2627 if (dev && domain_context_mapping(domain, dev)) {
2628 dev_err(dev, "Domain context map failed\n");
2629 dmar_remove_one_dev_info(dev);
2630 return NULL;
2631 }
2632
2633 return domain;
2634 }
2635
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Ashok,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pci/next]
[also build test ERROR on iommu/next linux/master linus/master v5.8-rc6 next-20200720]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ashok-Raj/PCI-ATS-PASID-and-PRI-are-only-enumerated-in-PF-devices/20200721-004510
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/pci/ats.c:471:6: warning: no previous prototype for 'pci_pri_supported' [-Wmissing-prototypes]
471 | bool pci_pri_supported(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~
drivers/pci/ats.c: In function 'pci_pri_supported':
>> drivers/pci/ats.c:474:30: error: 'struct pci_dev' has no member named 'pri_cap'; did you mean 'pcie_cap'?
474 | return !!(pci_physfn(pdev)->pri_cap);
| ^~~~~~~
| pcie_cap
>> drivers/pci/ats.c:475:1: warning: control reaches end of non-void function [-Wreturn-type]
475 | }
| ^
vim +474 drivers/pci/ats.c
463
464 /**
465 * pci_pri_supported - Check if PRI is supported.
466 * @pdev: PCI device structure
467 *
468 * Returns false when no PRI capability is present.
469 * Returns true if PRI feature is supported and enabled
470 */
> 471 bool pci_pri_supported(struct pci_dev *pdev)
472 {
473 /* VFs share the PF PRI configuration */
> 474 return !!(pci_physfn(pdev)->pri_cap);
> 475 }
476 EXPORT_SYMBOL_GPL(pci_pri_supported);
477
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Mon, Jul 20, 2020 at 09:43:00AM -0700, Ashok Raj wrote: > PASID and PRI capabilities are only enumerated in PF devices. VF devices > do not enumerate these capabilites. IOMMU drivers also need to enumerate > them before enabling features in the IOMMU. Extending the same support as > PASID feature discovery (pci_pasid_features) for PRI. > > Signed-off-by: Ashok Raj <ashok.raj@intel.com> Hi Ashok, When you update this for the 0-day implicit declaration thing, can you update the subject to say what the patch *does*, as opposed to what it is solving? Also, no need for a period at the end. Does this fix a regression? Is it associated with a commit that we could add as a "Fixes:" tag so we know how far back to try to apply to stable kernels? > To: Bjorn Helgaas <bhelgaas@google.com> > To: Joerg Roedel <joro@8bytes.com> > To: Lu Baolu <baolu.lu@intel.com> > Cc: stable@vger.kernel.org > Cc: linux-pci@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: Ashok Raj <ashok.raj@intel.com> > Cc: iommu@lists.linux-foundation.org > --- > drivers/iommu/intel/iommu.c | 2 +- > drivers/pci/ats.c | 14 ++++++++++++++ > include/linux/pci-ats.h | 1 + > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index d759e7234e98..276452f5e6a7 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -2560,7 +2560,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, > } > > if (info->ats_supported && ecap_prs(iommu->ecap) && > - pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI)) > + pci_pri_supported(pdev)) > info->pri_supported = 1; > } > } > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index b761c1f72f67..ffb4de8c5a77 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -461,6 +461,20 @@ int pci_pasid_features(struct pci_dev *pdev) > } > EXPORT_SYMBOL_GPL(pci_pasid_features); > > +/** > + * pci_pri_supported - Check if PRI is supported. > + * @pdev: PCI device structure > + * > + * Returns false when no PRI capability is present. > + * Returns true if PRI feature is supported and enabled > + */ > +bool pci_pri_supported(struct pci_dev *pdev) > +{ > + /* VFs share the PF PRI configuration */ > + return !!(pci_physfn(pdev)->pri_cap); > +} > +EXPORT_SYMBOL_GPL(pci_pri_supported); > + > #define PASID_NUMBER_SHIFT 8 > #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) > /** > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > index f75c307f346d..073d57292445 100644 > --- a/include/linux/pci-ats.h > +++ b/include/linux/pci-ats.h > @@ -28,6 +28,7 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs); > void pci_disable_pri(struct pci_dev *pdev); > int pci_reset_pri(struct pci_dev *pdev); > int pci_prg_resp_pasid_required(struct pci_dev *pdev); > +bool pci_pri_supported(struct pci_dev *pdev); > #endif /* CONFIG_PCI_PRI */ > > #ifdef CONFIG_PCI_PASID > -- > 2.7.4 >
Hi Bjorn On Tue, Jul 21, 2020 at 09:54:01AM -0500, Bjorn Helgaas wrote: > On Mon, Jul 20, 2020 at 09:43:00AM -0700, Ashok Raj wrote: > > PASID and PRI capabilities are only enumerated in PF devices. VF devices > > do not enumerate these capabilites. IOMMU drivers also need to enumerate > > them before enabling features in the IOMMU. Extending the same support as > > PASID feature discovery (pci_pasid_features) for PRI. > > > > Signed-off-by: Ashok Raj <ashok.raj@intel.com> > > Hi Ashok, > > When you update this for the 0-day implicit declaration thing, can you > update the subject to say what the patch *does*, as opposed to what it > is solving? Also, no need for a period at the end. Yes, will update and resend. Goofed up a couple things, i'll update those as well. > > Does this fix a regression? Is it associated with a commit that we > could add as a "Fixes:" tag so we know how far back to try to apply > to stable kernels? Yes, but the iommu files moved location and git fixes tags only generates for a few handful of commits and doesn't show the old ones. Cheers, Ashok
On Thu, Jul 23, 2020 at 10:38:19AM -0700, Raj, Ashok wrote: > Hi Bjorn > > On Tue, Jul 21, 2020 at 09:54:01AM -0500, Bjorn Helgaas wrote: > > On Mon, Jul 20, 2020 at 09:43:00AM -0700, Ashok Raj wrote: > > > PASID and PRI capabilities are only enumerated in PF devices. VF devices > > > do not enumerate these capabilites. IOMMU drivers also need to enumerate > > > them before enabling features in the IOMMU. Extending the same support as > > > PASID feature discovery (pci_pasid_features) for PRI. > > > > > > Signed-off-by: Ashok Raj <ashok.raj@intel.com> > > > > Hi Ashok, > > > > When you update this for the 0-day implicit declaration thing, can you > > update the subject to say what the patch *does*, as opposed to what it > > is solving? Also, no need for a period at the end. > > Yes, will update and resend. Goofed up a couple things, i'll update those > as well. > > > Does this fix a regression? Is it associated with a commit that we > > could add as a "Fixes:" tag so we know how far back to try to apply > > to stable kernels? > > Yes, Does that mean "yes, this fixes a regression"? > but the iommu files moved location and git fixes tags only generates > for a few handful of commits and doesn't show the old ones. Not sure how to interpret the rest of this. I'm happy to include the SHA1 of the original commit that added the regression, even if the file has moved since then. Bjorn
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index d759e7234e98..276452f5e6a7 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -2560,7 +2560,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, } if (info->ats_supported && ecap_prs(iommu->ecap) && - pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI)) + pci_pri_supported(pdev)) info->pri_supported = 1; } } diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index b761c1f72f67..ffb4de8c5a77 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -461,6 +461,20 @@ int pci_pasid_features(struct pci_dev *pdev) } EXPORT_SYMBOL_GPL(pci_pasid_features); +/** + * pci_pri_supported - Check if PRI is supported. + * @pdev: PCI device structure + * + * Returns false when no PRI capability is present. + * Returns true if PRI feature is supported and enabled + */ +bool pci_pri_supported(struct pci_dev *pdev) +{ + /* VFs share the PF PRI configuration */ + return !!(pci_physfn(pdev)->pri_cap); +} +EXPORT_SYMBOL_GPL(pci_pri_supported); + #define PASID_NUMBER_SHIFT 8 #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) /** diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h index f75c307f346d..073d57292445 100644 --- a/include/linux/pci-ats.h +++ b/include/linux/pci-ats.h @@ -28,6 +28,7 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs); void pci_disable_pri(struct pci_dev *pdev); int pci_reset_pri(struct pci_dev *pdev); int pci_prg_resp_pasid_required(struct pci_dev *pdev); +bool pci_pri_supported(struct pci_dev *pdev); #endif /* CONFIG_PCI_PRI */ #ifdef CONFIG_PCI_PASID
PASID and PRI capabilities are only enumerated in PF devices. VF devices do not enumerate these capabilites. IOMMU drivers also need to enumerate them before enabling features in the IOMMU. Extending the same support as PASID feature discovery (pci_pasid_features) for PRI. Signed-off-by: Ashok Raj <ashok.raj@intel.com> To: Bjorn Helgaas <bhelgaas@google.com> To: Joerg Roedel <joro@8bytes.com> To: Lu Baolu <baolu.lu@intel.com> Cc: stable@vger.kernel.org Cc: linux-pci@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: Ashok Raj <ashok.raj@intel.com> Cc: iommu@lists.linux-foundation.org --- drivers/iommu/intel/iommu.c | 2 +- drivers/pci/ats.c | 14 ++++++++++++++ include/linux/pci-ats.h | 1 + 3 files changed, 16 insertions(+), 1 deletion(-)