Re: [Xen-devel] [PATCH v3.1 07/15] xen/x86: do the PCI scan unconditionally

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 11/03/2016 07:35 AM, Jan Beulich wrote:
> > > > On 03.11.16 at 11:58, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Oct 31, 2016 at 10:47:15AM -0600, Jan Beulich wrote:
> > > > > > On 29.10.16 at 10:59, <roger.pau@xxxxxxxxxx> wrote:
> > > > --- a/xen/arch/x86/setup.c
> > > > +++ b/xen/arch/x86/setup.c
> > > > @@ -1491,6 +1491,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > > >
> > > >      early_msi_init();
> > > >
> > > > +    scan_pci_devices();
> > > > +
> > > >      iommu_setup();    /* setup iommu if available */
> > > >
> > > >      smp_prepare_cpus(max_cpus);
> > > > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > > > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > > > @@ -219,7 +219,8 @@ int __init amd_iov_detect(void)
> > > >
> > > >      if ( !amd_iommu_perdev_intremap )
> > > >          printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not 
> > > > recommended (see XSA-36)!\n");
> > > > -    return scan_pci_devices();
> > > > +
> > > > +    return 0;
> > > >  }
> > > I'm relatively certain that I did point out on a prior version that the
> > > error handling here gets lost. At the very least the commit message
> > > should provide a reason for doing so; even better would be if there
> > > was no behavioral change (other than the point in time where this
> > > happens slightly changing).
> > Behaviour here is different on Intel or AMD hardware, on Intel failure to
> > scan the PCI bus will not be fatal, and the IOMMU will be enabled anyway. On
> > AMD OTOH failure to scan the PCI bus will cause the IOMMU to be disabled.
> > I expect we should be able to behave equally for both Intel and AMD, so
> > which one should be used?
> I'm afraid I have to defer to the vendor IOMMU maintainers for
> that one, as I don't know the reason for the difference in behavior.
> An aspect that may play into here is that for AMD the IOMMU is
> represented by a PCI device, while for Intel it's just a part of one
> of the core chipset devices.
That's probably the reason although it looks like the only failure that 
scan_pci_devices() can return is -ENOMEM, in which case disabling IOMMU 
may not be the biggest problem.
-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel