RE: [PATCH] kvm: Enable device LTR/OBFF capibility before doingguest device assignment

[Hao, Xudong]

> -----Original Message-----
> From: Avi Kivity [mailto:avi@xxxxxxxxxx]
> Sent: Sunday, May 06, 2012 11:34 PM
> To: Xudong Hao
> Cc: mtosatti@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> Zhang, Xiantao; Hao, Xudong; Alex Williamson
> Subject: Re: [PATCH] kvm: Enable device LTR/OBFF capibility before doing guest
> device assignment
> 
> On 05/06/2012 06:24 PM, Xudong Hao wrote:
> > Enable device LTR/OBFF capibility before do device assignment, so that guest
> can benefit from them.
> 
> cc += Alex
> 
> > @@ -166,6 +166,10 @@ int kvm_assign_device(struct kvm *kvm,
> >     if (pdev == NULL)
> >         return -ENODEV;
> >
> > +   /* Enable some device capibility before do device assignment,
> > +    * so that guest can benefit from them.
> > +    */
> > +   kvm_iommu_enable_dev_caps(pdev);
> >     r = iommu_attach_device(domain, &pdev->dev);
> 
> Suppose we fail here.  Do we need to disable_dev_caps()?
> 
I don't think so. When a device will be assigned to guest, it's be owned by a pci-stub driver, attach_device fail here do not affect everything. If host want to use it, host device driver has its own enable/disable dev_caps.
 
> >     if (r) {
> >         printk(KERN_ERR "assign device %x:%x:%x.%x failed",
> > @@ -228,6 +232,7 @@ int kvm_deassign_device(struct kvm *kvm,
> >         PCI_SLOT(assigned_dev->host_devfn),
> >         PCI_FUNC(assigned_dev->host_devfn));
> >
> > +   kvm_iommu_disable_dev_caps(pdev);
> >     return 0;
> >  }
> >
> > @@ -351,3 +356,30 @@ int kvm_iommu_unmap_guest(struct kvm *kvm)
> >     iommu_domain_free(domain);
> >     return 0;
> >  }
> > +
> > +static void kvm_iommu_enable_dev_caps(struct pci_dev *pdev)
> > +{
> > +   /* set default value */
> > +   unsigned long type = PCI_EXP_OBFF_SIGNAL_ALWAYS;
> > +   int snoop_lat_ns = 1024, nosnoop_lat_ns = 1024;
> 
> Where does this magic number come from?
> 
The number is the max value that register support, set it as default here, we did not have any device here, and we do not know what's the proper value, so it set a default value firstly.


> > +
> > +   /* LTR(Latency tolerance reporting) allows devices to send
> > +    * messages to the root complex indicating their latency
> > +    * tolerance for snooped & unsnooped memory transactions.
> > +    */
> > +   pci_enable_ltr(pdev);
> > +   pci_set_ltr(pdev, snoop_lat_ns, nosnoop_lat_ns);
> > +
> > +   /* OBFF (optimized buffer flush/fill), where supported,
> > +    * can help improve energy efficiency by giving devices
> > +    * information about when interrupts and other activity
> > +    * will have a reduced power impact.
> > +    */
> > +   pci_enable_obff(pdev, type);
> > +}
> > +
> > +static void kvm_iommu_disable_dev_caps(struct pci_dev *pdev)
> > +{
> > +   pci_disble_obff(pdev);
> > +   pci_disble_ltr(pdev);
> > +}
> 
> Do we need to communicate something about these capabilities to the guest?
> 

I guess you means that here host don't know if guest want to enable them, right? 
The ltr/obff new feature are supposed to enabled by guest if platform and device supported.

> --
> error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/