[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/2] VT-d: Fix vt-d flush timeout issue.



> On 15.12.2015 at 8:30pm, <JBeulich@xxxxxxxx> wrote:
> >>> On 15.12.15 at 13:23, <quan.xu@xxxxxxxxx> wrote:
> >> On 15.12.2015 at 5:17pm, <JBeulich@xxxxxxxx> wrote:
> >> >>> On 15.12.15 at 09:15, <quan.xu@xxxxxxxxx> wrote:
> >> >> On 14.12.2015 at 5:28pm, <JBeulich@xxxxxxxx> wrote:
> >> >> >>> On 12.12.15 at 14:21, <quan.xu@xxxxxxxxx> wrote:
> >> >> > @@ -88,6 +89,16 @@ struct pci_dev {  #define
> >> >> > for_each_pdev(domain,
> >> >> > pdev) \
> >> >> >      list_for_each_entry(pdev, &(domain->arch.pdev_list),
> >> >> > domain_list)
> >> >> >
> >> >> > +static inline void mark_pdev_unassignable(struct pci_dev *pdev) {
> >> >> > +    pdev->info.is_unassignable = 1; }
> >> >> > +
> >> >> > +static inline bool_t is_pdev_unassignable(const struct pci_dev
> >> >> > +*pdev) {
> >> >> > +    return pdev->info.is_unassignable; }
> >> >>
> >> >> Are you aware that we already have a mechanism to prevent
> >> >> assignment (via pci_{ro,hide}_device())? I think at the very least
> >> >> this check should
> >> > consider both
> >> >> variants. Whether fully using the existing mechanism for your
> >> >> purpose is feasible I can't immediately tell (since the ownership
> >> >> change may be problematic at the points where you want the flagging to
> happen).
> >> >
> >> > pci_{ro,hide}_device() is dangerous, and it makes xen crash when I tried 
> >> > it.
> >> > for this case, IMO I think a flag is a better solution.
> >>
> >> I can't really judge on this without know details of the crash. As
> >> said, I'd prefer to have just a single mechanism, and would accept a
> >> second one only with proper justification (i.e. more than "is
> >> dangerous" or "makes xen crash" - after all that may also be a result of 
> >> the
> functions not being used as necessary).
> >
> > Jan, one question, do pci_{ro,hide}_device() work to hide pci devices?
> 
> Not sure I understand the question - the place they're used in they do work, 
> yes.


Copy from pci_hide_device(), which is actually add device to dom_xen and
add pdev->domain_list to dom_xen->arch.pdev_list.

Quite similar, a second one only with proper justification, I can reassign
Device form _domain to dom_xen directly. The below is the how to deal
With device_tlb ( is it acceptable? ). It is working to hide device.





+void device_tlb_invalidate_timeout(struct iommu *iommu, u16 did,
+                                   u16 seg, u8 bus, u8 devfn)
+{
+    struct domain *d;
+    struct pci_dev *pdev;
+    struct hvm_iommu *hd;
+    int rc;
+
+    d = rcu_lock_domain_by_id(iommu->domid_map[did]);
+    ASSERT(d);
+    for_each_pdev(d, pdev)
+        if ( (pdev->seg == seg) &&
+             (pdev->bus == bus) &&
+             (pdev->devfn == devfn) )
+        {
+            if ( pdev->domain )
+            {
+                hd = domain_hvm_iommu(d);
+                rc =  hd->platform_ops->reassign_device(d,
+                      dom_xen, devfn, pdev);
+                if ( rc )
+                    continue;
+            }
+            break;
+        }
+
+    if ( !is_hardware_domain(d) )
+        domain_crash(d);
+    rcu_unlock_domain(d);
+}



Quan

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.