Re: [Xen-devel] [PATCH] xen, vtd: Fix device check for devices behind PCIe-to-PCI bridges

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.11 at 11:12, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
> So what was the verdict on this one?  Is someone going to commit to
> doing a "fake pdev" thing?  If that's not going to happen before the
> 4.2 release, I suggest we take this patch in the mean time.

Isn't this -unstable c/s 23813:5535d7ce2673?

Jan

> On Thu, Sep 1, 2011 at 3:20 PM, George Dunlap
> <george.dunlap@xxxxxxxxxxxxx> wrote:
>> On some systems, requests devices behind a PCIe-to-PCI bridge all
>> appear to the IOMMU as though they come from from slot 0, function
>> 0 on that device; so the mapping code much punch a hole for X:0.0
>> in the IOMMU for such devices.  When punching the hole, if that device
>> has already been mapped once, we simply need to check ownership to
>> make sure it's legal.  To do so, domain_context_mapping_one() will look
>> up the device for the mapping with pci_get_pdev() and look for the owner.
>>
>> However, if there is no device in X:0.0, this look up will fail.
>>
>> Rather than returning -ENODEV in this situation (causing a failure in
>> mapping the device), try to get the domain ownership from the iommu context
>> mapping itself.
>>
>> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>>
>> diff -r 4a4882df5649 -r ede81b0552be xen/drivers/passthrough/vtd/iommu.c
>> --- a/xen/drivers/passthrough/vtd/iommu.c       Wed Aug 31 15:23:49 2011 
>> +0100
>> +++ b/xen/drivers/passthrough/vtd/iommu.c       Thu Sep 01 15:18:18 2011 
> +0100
>> @@ -113,6 +113,27 @@ static int context_set_domain_id(struct
>>     return 0;
>>  }
>>
>> +static int context_get_domain_id(struct context_entry *context,
>> +                                 struct iommu *iommu)
>> +{
>> +    unsigned long dom_index, nr_dom;
>> +    int domid = -1;
>> +
>> +    if (iommu && context)
>> +    {
>> +        nr_dom = cap_ndoms(iommu->cap);
>> +
>> +        dom_index = context_domain_id(*context);
>> +
>> +        if ( dom_index < nr_dom && iommu->domid_map)
>> +            domid = iommu->domid_map[dom_index];
>> +        else
>> +            dprintk(XENLOG_DEBUG VTDPREFIX, "%s: dom_index %lu exceeds 
> nr_dom %lu or iommu has no domid_map\n",
>> +                    __func__, dom_index, nr_dom);
>> +    }
>> +    return domid;
>> +}
>> +
>>  static struct intel_iommu *__init alloc_intel_iommu(void)
>>  {
>>     struct intel_iommu *intel;
>> @@ -1237,7 +1258,6 @@ int domain_context_mapping_one(
>>     struct hvm_iommu *hd = domain_hvm_iommu(domain);
>>     struct context_entry *context, *context_entries;
>>     u64 maddr, pgd_maddr;
>> -    struct pci_dev *pdev = NULL;
>>     int agaw;
>>
>>     ASSERT(spin_is_locked(&pcidevs_lock));
>> @@ -1249,12 +1269,45 @@ int domain_context_mapping_one(
>>     if ( context_present(*context) )
>>     {
>>         int res = 0;
>> +        struct pci_dev *pdev = NULL;
>>
>> +        /* First try to get domain ownership from device structure.  If 
> that's
>> +         * not available, try to read it from the context itself. */
>>         pdev = pci_get_pdev(bus, devfn);
>> -        if (!pdev)
>> -            res = -ENODEV;
>> -        else if (pdev->domain != domain)
>> -            res = -EINVAL;
>> +        if ( pdev )
>> +        {
>> +            if ( pdev->domain != domain )
>> +            {
>> +                dprintk(XENLOG_INFO VTDPREFIX, "d%d: bdf = %x:%x.%x owned 
> by d%d!",
>> +                        domain->domain_id,
>> +                        bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
>> +                        (pdev->domain)
>> +                        ? pdev->domain->domain_id : -1);
>> +                res = -EINVAL;
>> +            }
>> +        }
>> +        else
>> +        {
>> +            int cdomain;
>> +            cdomain = context_get_domain_id(context, iommu);
>> +
>> +            if ( cdomain < 0 )
>> +            {
>> +                dprintk(VTDPREFIX, "d%d: bdf = %x:%x.%x mapped, but can't 
> find owner!\n",
>> +                        domain->domain_id,
>> +                        bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
>> +                res = -EINVAL;
>> +            }
>> +            else if ( cdomain != domain->domain_id )
>> +            {
>> +                dprintk(XENLOG_INFO VTDPREFIX, "d%d: bdf = %x:%x.%x already 
> mapped to d%d!",
>> +                        domain->domain_id,
>> +                        bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
>> +                        cdomain);
>> +                res = -EINVAL;
>> +            }
>> +        }
>> +
>>         unmap_vtd_domain_page(context_entries);
>>         spin_unlock(&iommu->lock);
>>         return res;
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx 
>> http://lists.xensource.com/xen-devel 
>>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel