Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
> Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
>
> This patch enables pci device assignment based on VT-d support.
> When a device is assigned to the guest, the guest memory is pinned
> and the mapping is updated in the VT-d IOMMU.
>
I am afraid there still some compatible problem...
> Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> ---
> arch/x86/kvm/Makefile |3 +
> arch/x86/kvm/vtd.c | 203
[snip]
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a97157c..5cfc21a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -35,6 +35,7 @@
> #include
> #include
> #include
> +#include
This broken external kernel modules before 2.6.27... If we wrapped it
with CONFIG_DMAR, it would also broken the commit before the patch
checked in and after DMAR enabled in kernel... Need a version number
judgement?
> diff --git a/include/asm-x86/kvm_host.h
> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -354,6 +354,7 @@ struct kvm_arch{
>*/
> struct list_head active_mmu_pages;
> struct list_head assigned_dev_head;
> + struct dmar_domain *intel_iommu_domain;
Need wrapped by CONFIG_DMAR?
--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
Yang, Sheng wrote:
On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
This patch enables pci device assignment based on VT-d support.
When a device is assigned to the guest, the guest memory is pinned
and the mapping is updated in the VT-d IOMMU.
I am afraid there still some compatible problem...
Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
---
arch/x86/kvm/Makefile |3 +
arch/x86/kvm/vtd.c | 203
[snip]
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a97157c..5cfc21a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
This broken external kernel modules before 2.6.27... If we wrapped it
with CONFIG_DMAR, it would also broken the commit before the patch
checked in and after DMAR enabled in kernel... Need a version number
judgement?
kernel patches should not consider external module issues. That keeps
the code clean (at the expense of making the external module's
maintainer's life mode difficult, but that's their problem).
diff --git a/include/asm-x86/kvm_host.h
b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -354,6 +354,7 @@ struct kvm_arch{
*/
struct list_head active_mmu_pages;
struct list_head assigned_dev_head;
+ struct dmar_domain *intel_iommu_domain;
Need wrapped by CONFIG_DMAR?
I guess we can keep this, one pointer is not that expensive. But we
should make sure all the iommu functions are available when iommu is
unconfigured.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
On Wednesday 13 August 2008 17:46:03 Avi Kivity wrote:
> Yang, Sheng wrote:
> > On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
> >> Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
> >>
> >> This patch enables pci device assignment based on VT-d support.
> >> When a device is assigned to the guest, the guest memory is
> >> pinned and the mapping is updated in the VT-d IOMMU.
> >
> > I am afraid there still some compatible problem...
> >
> >> Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> >> Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> >> Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> >> ---
> >> arch/x86/kvm/Makefile |3 +
> >> arch/x86/kvm/vtd.c | 203
> >
> > [snip]
> >
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index a97157c..5cfc21a 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -35,6 +35,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >
> > This broken external kernel modules before 2.6.27... If we
> > wrapped it with CONFIG_DMAR, it would also broken the commit
> > before the patch checked in and after DMAR enabled in kernel...
> > Need a version number judgement?
>
> kernel patches should not consider external module issues. That
> keeps the code clean (at the expense of making the external
> module's maintainer's life mode difficult, but that's their
> problem).
Yeah, thanks for point it out. It's indeed complicate to consider this
kind of issues... :)
And I think now our aptitude towards external modules is not
encouraging? For after this patch, we can discard
external-modules-compat.h as well. :)
--
regards
Yang, Sheng
> >> diff --git a/include/asm-x86/kvm_host.h
> >> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
> >> --- a/include/asm-x86/kvm_host.h
> >> +++ b/include/asm-x86/kvm_host.h
> >> @@ -354,6 +354,7 @@ struct kvm_arch{
> >> */
> >>struct list_head active_mmu_pages;
> >>struct list_head assigned_dev_head;
> >> + struct dmar_domain *intel_iommu_domain;
> >
> > Need wrapped by CONFIG_DMAR?
>
> I guess we can keep this, one pointer is not that expensive. But
> we should make sure all the iommu functions are available when
> iommu is unconfigured.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
>
> This patch enables pci device assignment based on VT-d support.
> When a device is assigned to the guest, the guest memory is pinned and
> the mapping is updated in the VT-d IOMMU.
>
> Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> ---
> arch/x86/kvm/Makefile |3 +
> arch/x86/kvm/vtd.c | 203
> arch/x86/kvm/x86.c |
> 10 ++
> include/asm-x86/kvm_host.h |3 +
> include/linux/kvm_host.h | 32 +++
> virt/kvm/kvm_main.c|9 ++-
> 6 files changed, 259 insertions(+), 1 deletions(-)
> create mode 100644 arch/x86/kvm/vtd.c
> +int kvm_iommu_map_guest(struct kvm *kvm,
> + struct kvm_assigned_dev_kernel *assigned_dev)
> +{
> + struct pci_dev *pdev = NULL;
> + int rc;
> +
> + if (!intel_iommu_found()) {
> + printk(KERN_ERR "intel iommu not found\n");
> + return -ENODEV;
> + }
> +
> + printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> +assigned_dev->host_busnr,
> +PCI_SLOT(assigned_dev->host_devfn),
> +PCI_FUNC(assigned_dev->host_devfn));
> +
> + pdev = assigned_dev->dev;
> +
> + if (pdev == NULL) {
> + if (kvm->arch.intel_iommu_domain) {
> + intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> + kvm->arch.intel_iommu_domain = NULL;
> + }
> + return -ENODEV;
> + }
> +
> + kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> +int kvm_iommu_unmap_guest(struct kvm *kvm)
> +{
> + struct kvm_assigned_dev_kernel *entry;
> + struct pci_dev *pdev = NULL;
> + struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> +
> + /* check if iommu exists and in use */
> + if (!domain)
> + return 0;
> +
> + list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> + printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> +entry->host_busnr,
> +PCI_SLOT(entry->host_devfn),
> +PCI_FUNC(entry->host_devfn));
> +
> + for_each_pci_dev(pdev) {
> + if ((pdev->bus->number == entry->host_busnr) &&
> + (pdev->devfn == entry->host_devfn))
> + break;
> + }
> +
> + if (pdev == NULL)
> + return -ENODEV;
> +
> + /* detach kvm dmar domain */
> + intel_iommu_detach_dev(domain,
> +pdev->bus->number, pdev->devfn);
> + }
> + kvm_iommu_unmap_memslots(kvm);
> + intel_iommu_domain_exit(domain);
> + return 0;
> +}
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a97157c..5cfc21a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -35,6 +35,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
>
> list_add(&match->list, &kvm->arch.assigned_dev_head);
>
> + /* currenlty iommu is required for handling dma */
> + r = kvm_iommu_map_guest(kvm, match);
> + if (r)
> + goto out_list_del;
> +
This will break pvdma support. Please check if intel-iommu is found and only
then bail out of here.
Even though pvdma is out-of-tree, it doesn't make sense to restrict our usage
here. AMD IOMMU support will need this to be handled in the right way as
well.
> out:
> mutex_unlock(&kvm->lock);
> return r;
> +out_list_del:
> + list_del(&match->list);
> + pci_release_regions(dev);
> out_disable:
> pci_disable_device(dev);
> out_put:
> @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
>
> void kvm_arch_destroy_vm(struct kvm *kvm)
> {
> + kvm_iommu_unmap_guest(kvm);
Likewise, check if intel-iommu is found as a first step in this function.
As part of getting AMD-iommu and pvdma support in, we should have generic
function names, but that can be done later.
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index a18aaad..b703890 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
> void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
>
> +#ifdef CONFIG_DMAR
> +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> + unsigned long npages);
> +int kvm_iommu_map_guest(struct kvm *kvm,
> + struct kvm_assigned_dev_kernel *assigned_dev);
> +int kvm_iommu_unmap_guest(struct kvm *kvm);
> +#else /* CONFIG_DMAR */
> +static inline
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
> >
> > This patch enables pci device assignment based on VT-d support.
> > When a device is assigned to the guest, the guest memory is pinned and
> > the mapping is updated in the VT-d IOMMU.
> >
> > Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> > Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> > Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> > ---
> > arch/x86/kvm/Makefile |3 +
> > arch/x86/kvm/vtd.c | 203
> > arch/x86/kvm/x86.c |
> > 10 ++
> > include/asm-x86/kvm_host.h |3 +
> > include/linux/kvm_host.h | 32 +++
> > virt/kvm/kvm_main.c|9 ++-
> > 6 files changed, 259 insertions(+), 1 deletions(-)
> > create mode 100644 arch/x86/kvm/vtd.c
>
> > +int kvm_iommu_map_guest(struct kvm *kvm,
> > + struct kvm_assigned_dev_kernel *assigned_dev)
> > +{
> > + struct pci_dev *pdev = NULL;
> > + int rc;
> > +
> > + if (!intel_iommu_found()) {
> > + printk(KERN_ERR "intel iommu not found\n");
> > + return -ENODEV;
> > + }
> > +
> > + printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > + assigned_dev->host_busnr,
> > + PCI_SLOT(assigned_dev->host_devfn),
> > + PCI_FUNC(assigned_dev->host_devfn));
> > +
> > + pdev = assigned_dev->dev;
> > +
> > + if (pdev == NULL) {
> > + if (kvm->arch.intel_iommu_domain) {
> > + intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > + kvm->arch.intel_iommu_domain = NULL;
> > + }
> > + return -ENODEV;
> > + }
> > +
> > + kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
>
> > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > +{
> > + struct kvm_assigned_dev_kernel *entry;
> > + struct pci_dev *pdev = NULL;
> > + struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > +
> > + /* check if iommu exists and in use */
> > + if (!domain)
> > + return 0;
> > +
> > + list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > + printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > + entry->host_busnr,
> > + PCI_SLOT(entry->host_devfn),
> > + PCI_FUNC(entry->host_devfn));
> > +
> > + for_each_pci_dev(pdev) {
> > + if ((pdev->bus->number == entry->host_busnr) &&
> > + (pdev->devfn == entry->host_devfn))
> > + break;
> > + }
> > +
> > + if (pdev == NULL)
> > + return -ENODEV;
> > +
> > + /* detach kvm dmar domain */
> > + intel_iommu_detach_dev(domain,
> > + pdev->bus->number, pdev->devfn);
> > + }
> > + kvm_iommu_unmap_memslots(kvm);
> > + intel_iommu_domain_exit(domain);
> > + return 0;
> > +}
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index a97157c..5cfc21a 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -35,6 +35,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> >
> > list_add(&match->list, &kvm->arch.assigned_dev_head);
> >
> > + /* currenlty iommu is required for handling dma */
> > + r = kvm_iommu_map_guest(kvm, match);
> > + if (r)
> > + goto out_list_del;
> > +
>
> This will break pvdma support. Please check if intel-iommu is found and only
> then bail out of here.
>
> Even though pvdma is out-of-tree, it doesn't make sense to restrict our usage
> here. AMD IOMMU support will need this to be handled in the right way as
> well.
>
> > out:
> > mutex_unlock(&kvm->lock);
> > return r;
> > +out_list_del:
> > + list_del(&match->list);
> > + pci_release_regions(dev);
> > out_disable:
> > pci_disable_device(dev);
> > out_put:
> > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> >
> > void kvm_arch_destroy_vm(struct kvm *kvm)
> > {
> > + kvm_iommu_unmap_guest(kvm);
>
> Likewise, check if intel-iommu is found as a first step in this function.
>
> As part of getting AMD-iommu and pvdma support in, we should have generic
> function names, but that can be done later.
>
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index a18aaad..b703890 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> > int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
> > void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
> >
> > +#ifdef CONFIG_DMAR
> > +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> > + unsigne
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Thursday 21 Aug 2008 16:35:57 Ben-Ami Yassour wrote:
> On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> > * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > > Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
> > >
> > > This patch enables pci device assignment based on VT-d support.
> > > When a device is assigned to the guest, the guest memory is pinned and
> > > the mapping is updated in the VT-d IOMMU.
> > >
> > > Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> > > Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> > > Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> > > ---
> > > arch/x86/kvm/Makefile |3 +
> > > arch/x86/kvm/vtd.c | 203
> > > arch/x86/kvm/x86.c
> > > | 10 ++
> > > include/asm-x86/kvm_host.h |3 +
> > > include/linux/kvm_host.h | 32 +++
> > > virt/kvm/kvm_main.c|9 ++-
> > > 6 files changed, 259 insertions(+), 1 deletions(-)
> > > create mode 100644 arch/x86/kvm/vtd.c
> > >
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > + struct kvm_assigned_dev_kernel *assigned_dev)
> > > +{
> > > + struct pci_dev *pdev = NULL;
> > > + int rc;
> > > +
> > > + if (!intel_iommu_found()) {
> > > + printk(KERN_ERR "intel iommu not found\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > > +assigned_dev->host_busnr,
> > > +PCI_SLOT(assigned_dev->host_devfn),
> > > +PCI_FUNC(assigned_dev->host_devfn));
> > > +
> > > + pdev = assigned_dev->dev;
> > > +
> > > + if (pdev == NULL) {
> > > + if (kvm->arch.intel_iommu_domain) {
> > > + intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > > + kvm->arch.intel_iommu_domain = NULL;
> > > + }
> > > + return -ENODEV;
> > > + }
> > > +
> > > + kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> > >
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > + struct kvm_assigned_dev_kernel *entry;
> > > + struct pci_dev *pdev = NULL;
> > > + struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > > +
> > > + /* check if iommu exists and in use */
> > > + if (!domain)
> > > + return 0;
> > > +
> > > + list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > > + printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > > +entry->host_busnr,
> > > +PCI_SLOT(entry->host_devfn),
> > > +PCI_FUNC(entry->host_devfn));
> > > +
> > > + for_each_pci_dev(pdev) {
> > > + if ((pdev->bus->number == entry->host_busnr) &&
> > > + (pdev->devfn == entry->host_devfn))
> > > + break;
> > > + }
> > > +
> > > + if (pdev == NULL)
> > > + return -ENODEV;
> > > +
> > > + /* detach kvm dmar domain */
> > > + intel_iommu_detach_dev(domain,
> > > +pdev->bus->number, pdev->devfn);
> > > + }
> > > + kvm_iommu_unmap_memslots(kvm);
> > > + intel_iommu_domain_exit(domain);
> > > + return 0;
> > > +}
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index a97157c..5cfc21a 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -35,6 +35,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > #include
> > > #include
> > > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm
> > > *kvm,
> > >
> > > list_add(&match->list, &kvm->arch.assigned_dev_head);
> > >
> > > + /* currenlty iommu is required for handling dma */
> > > + r = kvm_iommu_map_guest(kvm, match);
> > > + if (r)
> > > + goto out_list_del;
> > > +
> >
> > This will break pvdma support. Please check if intel-iommu is found and
> > only then bail out of here.
> >
> > Even though pvdma is out-of-tree, it doesn't make sense to restrict our
> > usage here. AMD IOMMU support will need this to be handled in the right
> > way as well.
> >
> > > out:
> > > mutex_unlock(&kvm->lock);
> > > return r;
> > > +out_list_del:
> > > + list_del(&match->list);
> > > + pci_release_regions(dev);
> > > out_disable:
> > > pci_disable_device(dev);
> > > out_put:
> > > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> > >
> > > void kvm_arch_destroy_vm(struct kvm *kvm)
> > > {
> > > + kvm_iommu_unmap_guest(kvm);
> >
> > Likewise, check if intel-iommu is found as a first step in this function.
> >
> > As part of getting AMD-iommu and pvdma support in, we should have generic
> > function names, but that can be done later.
> >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index a18aaad..b703890 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> > > int k
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Thursday 21 Aug 2008 16:35:57 Ben-Ami Yassour wrote:
> On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> > * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > > Based on a patch by: Kay, Allen M <[EMAIL PROTECTED]>
> > >
> > > This patch enables pci device assignment based on VT-d support.
> > > When a device is assigned to the guest, the guest memory is pinned and
> > > the mapping is updated in the VT-d IOMMU.
> > >
> > > Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
> > > Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
> > > Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> > > ---
> > > arch/x86/kvm/Makefile |3 +
> > > arch/x86/kvm/vtd.c | 203
> > > arch/x86/kvm/x86.c
> > > | 10 ++
> > > include/asm-x86/kvm_host.h |3 +
> > > include/linux/kvm_host.h | 32 +++
> > > virt/kvm/kvm_main.c|9 ++-
> > > 6 files changed, 259 insertions(+), 1 deletions(-)
> > > create mode 100644 arch/x86/kvm/vtd.c
> > >
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > + struct kvm_assigned_dev_kernel *assigned_dev)
> > > +{
> > > + struct pci_dev *pdev = NULL;
> > > + int rc;
> > > +
> > > + if (!intel_iommu_found()) {
> > > + printk(KERN_ERR "intel iommu not found\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > > +assigned_dev->host_busnr,
> > > +PCI_SLOT(assigned_dev->host_devfn),
> > > +PCI_FUNC(assigned_dev->host_devfn));
> > > +
> > > + pdev = assigned_dev->dev;
> > > +
> > > + if (pdev == NULL) {
> > > + if (kvm->arch.intel_iommu_domain) {
> > > + intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > > + kvm->arch.intel_iommu_domain = NULL;
> > > + }
> > > + return -ENODEV;
> > > + }
> > > +
> > > + kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> > >
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > + struct kvm_assigned_dev_kernel *entry;
> > > + struct pci_dev *pdev = NULL;
> > > + struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > > +
> > > + /* check if iommu exists and in use */
> > > + if (!domain)
> > > + return 0;
> > > +
> > > + list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > > + printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > > +entry->host_busnr,
> > > +PCI_SLOT(entry->host_devfn),
> > > +PCI_FUNC(entry->host_devfn));
> > > +
> > > + for_each_pci_dev(pdev) {
> > > + if ((pdev->bus->number == entry->host_busnr) &&
> > > + (pdev->devfn == entry->host_devfn))
> > > + break;
> > > + }
> > > +
> > > + if (pdev == NULL)
> > > + return -ENODEV;
> > > +
> > > + /* detach kvm dmar domain */
> > > + intel_iommu_detach_dev(domain,
> > > +pdev->bus->number, pdev->devfn);
> > > + }
> > > + kvm_iommu_unmap_memslots(kvm);
> > > + intel_iommu_domain_exit(domain);
> > > + return 0;
> > > +}
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index a97157c..5cfc21a 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -35,6 +35,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > #include
> > > #include
> > > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm
> > > *kvm,
> > >
> > > list_add(&match->list, &kvm->arch.assigned_dev_head);
> > >
> > > + /* currenlty iommu is required for handling dma */
> > > + r = kvm_iommu_map_guest(kvm, match);
> > > + if (r)
> > > + goto out_list_del;
> > > +
> >
> > This will break pvdma support. Please check if intel-iommu is found and
> > only then bail out of here.
> >
> > Even though pvdma is out-of-tree, it doesn't make sense to restrict our
> > usage here. AMD IOMMU support will need this to be handled in the right
> > way as well.
> >
> > > out:
> > > mutex_unlock(&kvm->lock);
> > > return r;
> > > +out_list_del:
> > > + list_del(&match->list);
> > > + pci_release_regions(dev);
> > > out_disable:
> > > pci_disable_device(dev);
> > > out_put:
> > > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> > >
> > > void kvm_arch_destroy_vm(struct kvm *kvm)
> > > {
> > > + kvm_iommu_unmap_guest(kvm);
> >
> > Likewise, check if intel-iommu is found as a first step in this function.
> >
> > As part of getting AMD-iommu and pvdma support in, we should have generic
> > function names, but that can be done later.
> >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index a18aaad..b703890 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> > > int k
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
Amit Shah wrote:
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index d9ef7d3..2956e35 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
> __u32 flags;
> };
>
> +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> +
> #endif
>
>
>
(1 >> 0)?
This is a userspace inteface, so use a generic name like iommu. We also
need a KVM_CAP so userspace can check whether an iommu is present or not.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
> Amit Shah wrote:
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index d9ef7d3..2956e35 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
> > __u32 flags;
> > };
> >
> > +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> > +
> > #endif
>
> (1 >> 0)?
I kept the 1st field reserved for no particular implementation in mind as of
now.
> This is a userspace inteface, so use a generic name like iommu. We also
> need a KVM_CAP so userspace can check whether an iommu is present or not.
We could have multiple hardware IOMMU implementations, like Intel's VT-d and
AMD's IOMMU.
Also, is KVM_CAP_foo needed for this? This is the only #define that'll be used
and we can simply do something like
#ifdef KVM_DEV_ASSIGN_USE_VTD
flags |= KVM_DEV_ASSIGN_USE_VTD
#endif
?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
Amit Shah wrote:
* On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
Amit Shah wrote:
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d9ef7d3..2956e35 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -495,4 +495,6 @@ struct kvm_assigned_irq {
__u32 flags;
};
+#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
+
#endif
(1 >> 0)?
I kept the 1st field reserved for no particular implementation in mind as of
now.
Why?
This is a userspace inteface, so use a generic name like iommu. We also
need a KVM_CAP so userspace can check whether an iommu is present or not.
We could have multiple hardware IOMMU implementations, like Intel's VT-d and
AMD's IOMMU.
Not in userspace. Userspace sees either iommu or no iommu; it doesn't
care about the iommu model.
Also, is KVM_CAP_foo needed for this? This is the only #define that'll be used
and we can simply do something like
#ifdef KVM_DEV_ASSIGN_USE_VTD
flags |= KVM_DEV_ASSIGN_USE_VTD
#endif
?
That only detects if the headers have the flag, not if the kernel
actually supports it (and whether there is an iommu in the host). We
need run-time detection.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Saturday 23 Aug 2008 14:58:50 Avi Kivity wrote: > Amit Shah wrote: > > Also, is KVM_CAP_foo needed for this? This is the only #define that'll be > > used and we can simply do something like > > > > #ifdef KVM_DEV_ASSIGN_USE_VTD > > flags |= KVM_DEV_ASSIGN_USE_VTD > > #endif > > > > ? > > That only detects if the headers have the flag, not if the kernel > actually supports it (and whether there is an iommu in the host). We > need run-time detection. Which means we expose KVM_CAP_IOMMU only if one was detected? How to do this correctly? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
Amit Shah wrote: That only detects if the headers have the flag, not if the kernel actually supports it (and whether there is an iommu in the host). We need run-time detection. Which means we expose KVM_CAP_IOMMU only if one was detected? Yes. How to do this correctly? I don't know. Can't the VT-d api tell you if an iommu exists? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] KVM: Device assignemnt with VT-d
Avi Kivity wrote: > Amit Shah wrote: >>> That only detects if the headers have the flag, not if the kernel >>> actually supports it (and whether there is an iommu in the host). >>> We need run-time detection. >>> >> >> Which means we expose KVM_CAP_IOMMU only if one was detected? > > Yes. > >> How to do this >> correctly? >> > > I don't know. Can't the VT-d api tell you if an iommu exists? I think KVM_CAP_IOMMU is good. At the same time, there is only one iommu at most, and userspace doesn't care about the iommu model. It's easy to know whether an iommu exists. There is a VT-d api (intel_iommu_found()) to know if VT-d exists. Other iommu implementation can easily have this api too. Randy (Weidong) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
* On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
> Amit Shah wrote:
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index d9ef7d3..2956e35 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
> > __u32 flags;
> > };
> >
> > +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> > +
> > #endif
>
> (1 >> 0)?
>
> This is a userspace inteface, so use a generic name like iommu. We also
> need a KVM_CAP so userspace can check whether an iommu is present or not.
How about this?
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 29f94ba..38ab48b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -277,10 +277,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
list_add(&match->list, &kvm->arch.assigned_dev_head);
- /* currenlty iommu is required for handling dma */
- r = kvm_iommu_map_guest(kvm, match);
- if (r)
- goto out_list_del;
+ if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
+ r = kvm_iommu_map_guest(kvm, match);
+ if (r)
+ goto out_list_del;
+ }
out:
mutex_unlock(&kvm->lock);
@@ -1154,6 +1155,9 @@ int kvm_dev_ioctl_check_extension(long ext)
case KVM_CAP_PV_MMU:
r = !tdp_enabled;
break;
+ case KVM_CAP_IOMMU:
+ r = intel_iommu_found();
+ break;
default:
r = 0;
break;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d9ef7d3..f2eafe1 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -384,6 +384,7 @@ struct kvm_trace_rec {
#define KVM_CAP_COALESCED_MMIO 15
#define KVM_CAP_SYNC_MMU 16 /* Changes to host mmap are reflected in guest
*/
#define KVM_CAP_DEVICE_ASSIGNMENT 17
+#define KVM_CAP_IOMMU 18
/*
* ioctls for VM fds
@@ -495,4 +496,6 @@ struct kvm_assigned_irq {
__u32 flags;
};
+#define KVM_DEV_ASSIGN_ENABLE_IOMMU(1 << 0)
+
#endif
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] KVM: Device assignemnt with VT-d
Amit Shah wrote: (1 >> 0)? This is a userspace inteface, so use a generic name like iommu. We also need a KVM_CAP so userspace can check whether an iommu is present or not. How about this? Looks fine. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
