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

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

Ah yes, so it is.  Sorry.
 -George

On Tue, Sep 13, 2011 at 11:53 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> 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
>

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