drivers/xen/xen-pciback/xenbus.c | 39 +++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-)
Check if the device is a bridge.
If it is a bridge, iterate over all its child devices and export them.
Log error if the export fails for any particular device logging details.
Export error string is split across lines as I could see several
other such occurrences in the file.
Please let me know if I should change it in some way.
Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
---
drivers/xen/xen-pciback/xenbus.c | 39 +++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index b11e401f1b1e..d15271d33ad6 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -258,14 +258,37 @@ static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
xen_register_device_domain_owner(dev, pdev->xdev->otherend_id);
}
- /* TODO: It'd be nice to export a bridge and have all of its children
- * get exported with it. This may be best done in xend (which will
- * have to calculate resource usage anyway) but we probably want to
- * put something in here to ensure that if a bridge gets given to a
- * driver domain, that all devices under that bridge are not given
- * to other driver domains (as he who controls the bridge can disable
- * it and stop the other devices from working).
- */
+ /* Check if the device is a bridge and export all its children */
+ if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
+ struct pci_dev *child = NULL;
+
+ /* Iterate over all the devices in this bridge */
+ list_for_each_entry(child, &dev->subordinate->devices,
+ bus_list) {
+ dev_dbg(&pdev->xdev->dev,
+ "exporting child device %04x:%02x:%02x.%d\n",
+ child->domain, child->bus->number,
+ PCI_SLOT(child->devfn),
+ PCI_FUNC(child->devfn));
+
+ err = xen_pcibk_export_device(pdev,
+ child->domain,
+ child->bus->number,
+ PCI_SLOT(child->devfn),
+ PCI_FUNC(child->devfn),
+ devid);
+ if (err) {
+ dev_err(&pdev->xdev->dev,
+ "failed to export child device : "
+ "%04x:%02x:%02x.%d\n",
+ child->domain,
+ child->bus->number,
+ PCI_SLOT(child->devfn),
+ PCI_FUNC(child->devfn));
+ goto out;
+ }
+ }
+ }
out:
return err;
}
--
2.34.1Hi Abhinav,
kernel test robot noticed the following build warnings:
[auto build test WARNING on xen-tip/linux-next]
[also build test WARNING on linus/master v6.10-rc3 next-20240607]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Abhinav-Jain/xen-xen-pciback-Export-a-bridge-and-all-its-children-as-per-TODO/20240610-024623
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link: https://lore.kernel.org/r/20240609184410.53500-1-jain.abhinav177%40gmail.com
patch subject: [PATCH] xen: xen-pciback: Export a bridge and all its children as per TODO
config: x86_64-randconfig-006-20240610 (https://download.01.org/0day-ci/archive/20240610/202406101933.49pM50Ii-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/202406101933.49pM50Ii-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406101933.49pM50Ii-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/xen/xen-pciback/xenbus.c:262:21: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
262 | if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
| ^ ~~~~~~~~~~~~~~~~~~~~
drivers/xen/xen-pciback/xenbus.c:262:21: note: use '&' for a bitwise operation
262 | if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
| ^~
| &
drivers/xen/xen-pciback/xenbus.c:262:21: note: remove constant to silence this warning
262 | if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/xen/xen-pciback/xenbus.c:270:12: error: no member named 'domain' in 'struct pci_dev'
270 | child->domain, child->bus->number,
| ~~~~~ ^
include/linux/dev_printk.h:163:47: note: expanded from macro 'dev_dbg'
163 | dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk'
129 | _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/xen/xen-pciback/xenbus.c:275:20: error: no member named 'domain' in 'struct pci_dev'
275 | child->domain,
| ~~~~~ ^
drivers/xen/xen-pciback/xenbus.c:284:13: error: no member named 'domain' in 'struct pci_dev'
284 | child->domain,
| ~~~~~ ^
include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
1 warning and 3 errors generated.
vim +262 drivers/xen/xen-pciback/xenbus.c
225
226 static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
227 int domain, int bus, int slot, int func,
228 int devid)
229 {
230 struct pci_dev *dev;
231 int err = 0;
232
233 dev_dbg(&pdev->xdev->dev, "exporting dom %x bus %x slot %x func %x\n",
234 domain, bus, slot, func);
235
236 dev = pcistub_get_pci_dev_by_slot(pdev, domain, bus, slot, func);
237 if (!dev) {
238 err = -EINVAL;
239 xenbus_dev_fatal(pdev->xdev, err,
240 "Couldn't locate PCI device "
241 "(%04x:%02x:%02x.%d)! "
242 "perhaps already in-use?",
243 domain, bus, slot, func);
244 goto out;
245 }
246
247 err = xen_pcibk_add_pci_dev(pdev, dev, devid,
248 xen_pcibk_publish_pci_dev);
249 if (err)
250 goto out;
251
252 dev_info(&dev->dev, "registering for %d\n", pdev->xdev->otherend_id);
253 if (xen_register_device_domain_owner(dev,
254 pdev->xdev->otherend_id) != 0) {
255 dev_err(&dev->dev, "Stealing ownership from dom%d.\n",
256 xen_find_device_domain_owner(dev));
257 xen_unregister_device_domain_owner(dev);
258 xen_register_device_domain_owner(dev, pdev->xdev->otherend_id);
259 }
260
261 /* Check if the device is a bridge and export all its children */
> 262 if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
263 struct pci_dev *child = NULL;
264
265 /* Iterate over all the devices in this bridge */
266 list_for_each_entry(child, &dev->subordinate->devices,
267 bus_list) {
268 dev_dbg(&pdev->xdev->dev,
269 "exporting child device %04x:%02x:%02x.%d\n",
270 child->domain, child->bus->number,
271 PCI_SLOT(child->devfn),
272 PCI_FUNC(child->devfn));
273
274 err = xen_pcibk_export_device(pdev,
275 child->domain,
276 child->bus->number,
277 PCI_SLOT(child->devfn),
278 PCI_FUNC(child->devfn),
279 devid);
280 if (err) {
281 dev_err(&pdev->xdev->dev,
282 "failed to export child device : "
283 "%04x:%02x:%02x.%d\n",
284 child->domain,
285 child->bus->number,
286 PCI_SLOT(child->devfn),
287 PCI_FUNC(child->devfn));
288 goto out;
289 }
290 }
291 }
292 out:
293 return err;
294 }
295
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Abhinav,
kernel test robot noticed the following build errors:
[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on linus/master v6.10-rc3 next-20240607]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Abhinav-Jain/xen-xen-pciback-Export-a-bridge-and-all-its-children-as-per-TODO/20240610-024623
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link: https://lore.kernel.org/r/20240609184410.53500-1-jain.abhinav177%40gmail.com
patch subject: [PATCH] xen: xen-pciback: Export a bridge and all its children as per TODO
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240610/202406101511.hTO5m855-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/202406101511.hTO5m855-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406101511.hTO5m855-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/xen/xenbus.h:37,
from drivers/xen/xen-pciback/xenbus.c:15:
drivers/xen/xen-pciback/xenbus.c: In function 'xen_pcibk_export_device':
>> drivers/xen/xen-pciback/xenbus.c:270:38: error: 'struct pci_dev' has no member named 'domain'
270 | child->domain, child->bus->number,
| ^~
include/linux/dev_printk.h:129:48: note: in definition of macro 'dev_printk'
129 | _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/xen/xen-pciback/xenbus.c:268:25: note: in expansion of macro 'dev_dbg'
268 | dev_dbg(&pdev->xdev->dev,
| ^~~~~~~
drivers/xen/xen-pciback/xenbus.c:275:60: error: 'struct pci_dev' has no member named 'domain'
275 | child->domain,
| ^~
drivers/xen/xen-pciback/xenbus.c:284:46: error: 'struct pci_dev' has no member named 'domain'
284 | child->domain,
| ^~
include/linux/dev_printk.h:110:37: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/xen/xen-pciback/xenbus.c:281:33: note: in expansion of macro 'dev_err'
281 | dev_err(&pdev->xdev->dev,
| ^~~~~~~
vim +270 drivers/xen/xen-pciback/xenbus.c
225
226 static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
227 int domain, int bus, int slot, int func,
228 int devid)
229 {
230 struct pci_dev *dev;
231 int err = 0;
232
233 dev_dbg(&pdev->xdev->dev, "exporting dom %x bus %x slot %x func %x\n",
234 domain, bus, slot, func);
235
236 dev = pcistub_get_pci_dev_by_slot(pdev, domain, bus, slot, func);
237 if (!dev) {
238 err = -EINVAL;
239 xenbus_dev_fatal(pdev->xdev, err,
240 "Couldn't locate PCI device "
241 "(%04x:%02x:%02x.%d)! "
242 "perhaps already in-use?",
243 domain, bus, slot, func);
244 goto out;
245 }
246
247 err = xen_pcibk_add_pci_dev(pdev, dev, devid,
248 xen_pcibk_publish_pci_dev);
249 if (err)
250 goto out;
251
252 dev_info(&dev->dev, "registering for %d\n", pdev->xdev->otherend_id);
253 if (xen_register_device_domain_owner(dev,
254 pdev->xdev->otherend_id) != 0) {
255 dev_err(&dev->dev, "Stealing ownership from dom%d.\n",
256 xen_find_device_domain_owner(dev));
257 xen_unregister_device_domain_owner(dev);
258 xen_register_device_domain_owner(dev, pdev->xdev->otherend_id);
259 }
260
261 /* Check if the device is a bridge and export all its children */
262 if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
263 struct pci_dev *child = NULL;
264
265 /* Iterate over all the devices in this bridge */
266 list_for_each_entry(child, &dev->subordinate->devices,
267 bus_list) {
268 dev_dbg(&pdev->xdev->dev,
269 "exporting child device %04x:%02x:%02x.%d\n",
> 270 child->domain, child->bus->number,
271 PCI_SLOT(child->devfn),
272 PCI_FUNC(child->devfn));
273
274 err = xen_pcibk_export_device(pdev,
275 child->domain,
276 child->bus->number,
277 PCI_SLOT(child->devfn),
278 PCI_FUNC(child->devfn),
279 devid);
280 if (err) {
281 dev_err(&pdev->xdev->dev,
282 "failed to export child device : "
283 "%04x:%02x:%02x.%d\n",
284 child->domain,
285 child->bus->number,
286 PCI_SLOT(child->devfn),
287 PCI_FUNC(child->devfn));
288 goto out;
289 }
290 }
291 }
292 out:
293 return err;
294 }
295
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 09.06.2024 20:44, Abhinav Jain wrote:
> Check if the device is a bridge.
> If it is a bridge, iterate over all its child devices and export them.
> Log error if the export fails for any particular device logging details.
> Export error string is split across lines as I could see several
> other such occurrences in the file.
> Please let me know if I should change it in some way.
>
> Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
> ---
> drivers/xen/xen-pciback/xenbus.c | 39 +++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> index b11e401f1b1e..d15271d33ad6 100644
> --- a/drivers/xen/xen-pciback/xenbus.c
> +++ b/drivers/xen/xen-pciback/xenbus.c
> @@ -258,14 +258,37 @@ static int xen_pcibk_export_device(struct xen_pcibk_device *pdev,
> xen_register_device_domain_owner(dev, pdev->xdev->otherend_id);
> }
>
> - /* TODO: It'd be nice to export a bridge and have all of its children
> - * get exported with it. This may be best done in xend (which will
> - * have to calculate resource usage anyway) but we probably want to
> - * put something in here to ensure that if a bridge gets given to a
> - * driver domain, that all devices under that bridge are not given
> - * to other driver domains (as he who controls the bridge can disable
> - * it and stop the other devices from working).
> - */
> + /* Check if the device is a bridge and export all its children */
> + if ((dev->hdr_type && PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) {
Besides it wanting to be & here, it feels as if such a change can't be done
standalone in pciback. It likely needs adjustments in the tool stack (even
if that's not send anymore) and possibly also arrangements in the hypervisor
(to correctly deal with bridges when handed to other than Dom0).
> + struct pci_dev *child = NULL;
> +
> + /* Iterate over all the devices in this bridge */
> + list_for_each_entry(child, &dev->subordinate->devices,
> + bus_list) {
> + dev_dbg(&pdev->xdev->dev,
> + "exporting child device %04x:%02x:%02x.%d\n",
> + child->domain, child->bus->number,
> + PCI_SLOT(child->devfn),
> + PCI_FUNC(child->devfn));
> +
> + err = xen_pcibk_export_device(pdev,
> + child->domain,
> + child->bus->number,
> + PCI_SLOT(child->devfn),
> + PCI_FUNC(child->devfn),
> + devid);
> + if (err) {
> + dev_err(&pdev->xdev->dev,
> + "failed to export child device : "
> + "%04x:%02x:%02x.%d\n",
> + child->domain,
> + child->bus->number,
> + PCI_SLOT(child->devfn),
> + PCI_FUNC(child->devfn));
> + goto out;
Hmm, and leaving things in partially-exported state?
Jan
> + }
> + }
> + }
> out:
> return err;
> }
© 2016 - 2024 Red Hat, Inc.