On Thu, May 05, 2022 at 05:14:14PM +0200, Jan Beulich wrote: > On 05.05.2022 17:00, Roger Pau Monné wrote: > > On Fri, Apr 29, 2022 at 03:05:32PM +0200, Jan Beulich wrote: > >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments > >> unless you have verified the sender and know the content is safe. > >> > >> IOMMU code mapping / unmapping devices and interrupts will misbehave if > >> a wrong command line option declared a function "phantom" when there's a > >> real device at that position. Warn about this and adjust the specified > >> stride (in the worst case ignoring the option altogether). > >> > >> Requested-by: Andrew Cooper <[email protected]> > >> Signed-off-by: Jan Beulich <[email protected]> > > > > Reviewed-by: Roger Pau Monné <[email protected]> > > Thanks. > > > FWIW, I would be fine with just discarding the stride option if one of > > the phantom devices happen to report vendor/device IDs on the config > > space. > > Well, I thought I'd try a best-effort adjustment rather than simply > ignoring an option. > > >> --- a/xen/drivers/passthrough/pci.c > >> +++ b/xen/drivers/passthrough/pci.c > >> @@ -451,7 +451,24 @@ static struct pci_dev *alloc_pdev(struct > >> phantom_devs[i].slot == PCI_SLOT(devfn) && > >> phantom_devs[i].stride > PCI_FUNC(devfn) ) > >> { > >> - pdev->phantom_stride = phantom_devs[i].stride; > >> + pci_sbdf_t sbdf = pdev->sbdf; > >> + unsigned int stride = phantom_devs[i].stride; > >> + > >> + while ( (sbdf.fn += stride) > PCI_FUNC(devfn) ) > >> + { > >> + if ( pci_conf_read16(sbdf, PCI_VENDOR_ID) == > >> 0xffff && > >> + pci_conf_read16(sbdf, PCI_DEVICE_ID) == > >> 0xffff ) > >> + continue; > >> + stride <<= 1; > >> + printk(XENLOG_WARNING > >> + "%pp looks to be a real device; > >> bumping %04x:%02x:%02x stride to %u\n", > >> + &sbdf, phantom_devs[i].seg, > >> + phantom_devs[i].bus, > >> phantom_devs[i].slot, > > > > Can't you use pdev->sbdf here? > > No - sbdf was altered from pdev->sbdf (and is also shorter to use), > and for the 2nd item I'm intentionally omitting the function part > (to match the command line option).
Sorry, should have been clearer. My question was to use pdev->sbdf for the second instance. I see now that you don't print the function, so that's fine. Thanks, Roger.
