[PATCH] KVM: Fix signature of kvm_iommu_map_pages stub

[PATCH] KVM: Fix signature of kvm_iommu_map_pages stub

[Jan Kiszka]

Breaks otherwise if CONFIG_IOMMU_API is not set.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 include/linux/kvm_host.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 0b89d00..866ed30 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -483,8 +483,7 @@ int kvm_deassign_device(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *assigned_dev);
 #else /* CONFIG_IOMMU_API */
 static inline int kvm_iommu_map_pages(struct kvm *kvm,
-				      gfn_t base_gfn,
-				      unsigned long npages)
+				      struct kvm_memory_slot *slot)
 {
 	return 0;
 }
-- 
1.7.1

Re: [PATCH] KVM: Fix signature of kvm_iommu_map_pages stub

[Jan Kiszka]

Am 14.10.2010 13:59, Jan Kiszka wrote:
> Breaks otherwise if CONFIG_IOMMU_API is not set.

Actually, it only broke a special local version. It doesn't break with
current KVM due to

[__kvm_set_memory_region:]
#ifdef CONFIG_DMAR
	/* map the pages in iommu page table */
	if (npages) {
		r = kvm_iommu_map_pages(kvm, &new);
		if (r)
			goto out_free;
	}
#endif

And CONFIG_IOMMU_API is set when CONFIG_DMAR is enabled. But do we only
need this call on Intel?

Jan

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  include/linux/kvm_host.h |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0b89d00..866ed30 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -483,8 +483,7 @@ int kvm_deassign_device(struct kvm *kvm,
>  			struct kvm_assigned_dev_kernel *assigned_dev);
>  #else /* CONFIG_IOMMU_API */
>  static inline int kvm_iommu_map_pages(struct kvm *kvm,
> -				      gfn_t base_gfn,
> -				      unsigned long npages)
> +				      struct kvm_memory_slot *slot)
>  {
>  	return 0;
>  }

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [PATCH] KVM: Fix signature of kvm_iommu_map_pages stub

[Marcelo Tosatti]

On Thu, Oct 14, 2010 at 02:43:51PM +0200, Jan Kiszka wrote:
> Am 14.10.2010 13:59, Jan Kiszka wrote:
> > Breaks otherwise if CONFIG_IOMMU_API is not set.
> 
> Actually, it only broke a special local version. It doesn't break with
> current KVM due to
> 
> [__kvm_set_memory_region:]
> #ifdef CONFIG_DMAR
> 	/* map the pages in iommu page table */
> 	if (npages) {
> 		r = kvm_iommu_map_pages(kvm, &new);
> 		if (r)
> 			goto out_free;
> 	}
> #endif
> 
> And CONFIG_IOMMU_API is set when CONFIG_DMAR is enabled. But do we only
> need this call on Intel?

On CONFIG_AMD_IOMMU too. It should be conditional on CONFIG_IOMMU_API.

Re: [PATCH] KVM: Fix signature of kvm_iommu_map_pages stub

[Jan Kiszka]

Am 18.10.2010 15:04, Marcelo Tosatti wrote:
> On Thu, Oct 14, 2010 at 02:43:51PM +0200, Jan Kiszka wrote:
>> Am 14.10.2010 13:59, Jan Kiszka wrote:
>>> Breaks otherwise if CONFIG_IOMMU_API is not set.
>>
>> Actually, it only broke a special local version. It doesn't break with
>> current KVM due to
>>
>> [__kvm_set_memory_region:]
>> #ifdef CONFIG_DMAR
>> 	/* map the pages in iommu page table */
>> 	if (npages) {
>> 		r = kvm_iommu_map_pages(kvm, &new);
>> 		if (r)
>> 			goto out_free;
>> 	}
>> #endif
>>
>> And CONFIG_IOMMU_API is set when CONFIG_DMAR is enabled. But do we only
>> need this call on Intel?
> 
> On CONFIG_AMD_IOMMU too. It should be conditional on CONFIG_IOMMU_API.
> 

Then we can simply drop the #ifdef here as kvm_iommu_map_pages is
stubbed for !CONFIG_IOMMU_API.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux