Re: [Xen-devel] Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.13 at 00:27, "Auld, Will" <will.auld@xxxxxxxxx> wrote:
> FYI: PRC is on holiday Thursday and Friday.

Understood, but it's now Thursday the next week, and there was
_still_ no response.

Jan

>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Wednesday, September 18, 2013 5:51 AM
>> To: Zhang, Xiantao
>> Cc: suravee.suthikulpanit@xxxxxxx; stefan.bader@xxxxxxxxxxxxx; Auld,
>> Will; xen-devel
>> Subject: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for
>> host bridges
>> 
>> Xiantao,
>> 
>> despite the title this has implications on VT-d, and hence a review/
>> ack from you would be much appreciated.
>> 
>> Jan
>> 
>> >>> On 12.09.13 at 10:17, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> > The host bridge device (i.e. 0x18 for AMD) does not require IOMMU,
>> and
>> > therefore is not included in the IVRS. The current logic tries to map
>> > all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
>> > following message on AMD sytem.
>> >
>> > (XEN) setup 0000:00:18.0 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.1 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.2 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.3 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.4 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.5 for d0 failed (-19)
>> >
>> > This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE)
>> > which corresponds to PCI class code 0x06 and sub-class 0x00. Then, it
>> > uses this new type to filter when trying to map device to IOMMU.
>> >
>> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
>> > Reported-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>> >
>> > On VT-d refuse (un)mapping host bridges for other than the hardware
>> > domain.
>> >
>> > Coding style cleanup.
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >
>> > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
>> >
>> >          amd_iommu_flush_device(iommu, req_id);
>> >
>> > -        AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
>> > +        AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type
>> = %#x, "
>> >                          "root table = %#"PRIx64", "
>> > -                        "domain = %d, paging mode = %d\n", req_id,
>> > +                        "domain = %d, paging mode = %d\n",
>> > +                        req_id, pdev->type,
>> >                          page_to_maddr(hd->root_table),
>> >                          hd->domain_id, hd->paging_mode);
>> >      }
>> > @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
>> >
>> >      if ( unlikely(!iommu) )
>> >      {
>> > +        /* Filter the bridge devices */
>> > +        if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
>> > +        {
>> > +            AMD_IOMMU_DEBUG("Skipping host bridge
>> %04x:%02x:%02x.%u\n",
>> > +                            pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
>> > +                            PCI_FUNC(bdf));
>> > +            return 0;
>> > +        }
>> > +
>> >          AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
>> >                          pdev->seg, pdev->bus,
>> >                          PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > --- a/xen/drivers/passthrough/pci.c
>> > +++ b/xen/drivers/passthrough/pci.c
>> > @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
>> >          u16 cap;
>> >          u8 sec_bus, sub_bus;
>> >
>> > -        case DEV_TYPE_PCIe_BRIDGE:
>> > -            break;
>> > -
>> >          case DEV_TYPE_PCIe2PCI_BRIDGE:
>> >          case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> >              sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
>> > @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
>> >              break;
>> >
>> >          case DEV_TYPE_PCI:
>> > +        case DEV_TYPE_PCIe_BRIDGE:
>> > +        case DEV_TYPE_PCI_HOST_BRIDGE:
>> >              break;
>> >
>> >          default:
>> > @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
>> >      spin_unlock(&pcidevs_lock);
>> >  }
>> >
>> > +#define PCI_CLASS_BRIDGE_HOST    0x0600
>> >  #define PCI_CLASS_BRIDGE_PCI     0x0604
>> >
>> >  enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn) @@ -720,6 +720,8
>> > @@ enum pdev_type pdev_type(u16 seg, u8 bus
>> >              return DEV_TYPE_PCI2PCIe_BRIDGE;
>> >          }
>> >          return DEV_TYPE_PCIe_BRIDGE;
>> > +    case PCI_CLASS_BRIDGE_HOST:
>> > +        return DEV_TYPE_PCI_HOST_BRIDGE;
>> >
>> >      case 0x0000: case 0xffff:
>> >          return DEV_TYPE_PCI_UNKNOWN;
>> > --- a/xen/drivers/passthrough/vtd/intremap.c
>> > +++ b/xen/drivers/passthrough/vtd/intremap.c
>> > @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
>> >      case DEV_TYPE_PCIe_ENDPOINT:
>> >      case DEV_TYPE_PCIe_BRIDGE:
>> >      case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > +    case DEV_TYPE_PCI_HOST_BRIDGE:
>> >          switch ( pdev->phantom_stride )
>> >          {
>> >          case 1: sq = SQ_13_IGNORE_3; break;
>> > --- a/xen/drivers/passthrough/vtd/iommu.c
>> > +++ b/xen/drivers/passthrough/vtd/iommu.c
>> > @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
>> >
>> >      switch ( pdev->type )
>> >      {
>> > +    case DEV_TYPE_PCI_HOST_BRIDGE:
>> > +        if ( iommu_verbose )
>> > +            dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u map\n",
>> > +                    domain->domain_id, seg, bus,
>> > +                    PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > +        if ( !is_hardware_domain(domain) )
>> > +            return -EPERM;
>> > +        break;
>> > +
>> >      case DEV_TYPE_PCIe_BRIDGE:
>> >      case DEV_TYPE_PCIe2PCI_BRIDGE:
>> >      case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
>> >
>> >      switch ( pdev->type )
>> >      {
>> > +    case DEV_TYPE_PCI_HOST_BRIDGE:
>> > +        if ( iommu_verbose )
>> > +            dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u unmap\n",
>> > +                    domain->domain_id, seg, bus,
>> > +                    PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > +        if ( !is_hardware_domain(domain) )
>> > +            return -EPERM;
>> > +        goto out;
>> > +
>> >      case DEV_TYPE_PCIe_BRIDGE:
>> >      case DEV_TYPE_PCIe2PCI_BRIDGE:
>> >      case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > --- a/xen/include/xen/pci.h
>> > +++ b/xen/include/xen/pci.h
>> > @@ -63,6 +63,7 @@ struct pci_dev {
>> >          DEV_TYPE_PCIe2PCI_BRIDGE,   // PCIe-to-PCI/PCIx bridge
>> >          DEV_TYPE_PCI2PCIe_BRIDGE,   // PCI/PCIx-to-PCIe bridge
>> >          DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
>> > +        DEV_TYPE_PCI_HOST_BRIDGE,   // PCI Host bridge
>> >          DEV_TYPE_PCI,
>> >      } type;
>> >
>> 



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