Re: [PATCH 2/4][VTD] modifications to intel-iommu.c.

[Avi Kivity <avi@qumranet.com>]

Kay, Allen M wrote:
> Modification to intel-iommu.c to support vt-d page table and context
> table mapping in kvm.  Mods to dmar.c and iova.c are due to header file
> moves to include/linux.
>   

> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index f941f60..a58a5b0 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -26,8 +26,8 @@
>  
>  #include <linux/pci.h>
>  #include <linux/dmar.h>
> -#include "iova.h"
> -#include "intel-iommu.h"
> +#include <linux/iova.h>
> +#include <linux/intel-iommu.h>

This should have been done in the file movement patch to avoid breaking 
the build. 

>
>  
> +void kvm_intel_iommu_domain_exit(struct dmar_domain *domain)

This should be a generic API, not a kvm specific one.

> +{
> +    u64 end;
> +
> +    /* Domain 0 is reserved, so dont process it */
> +    if (!domain)
> +        return;

'domain' here is a pointer, not an identifier.

>
> +int kvm_intel_iommu_context_mapping(
> +    struct dmar_domain *domain, struct pci_dev *pdev)
> +{
> +    int rc;
> +    rc = domain_context_mapping(domain, pdev);
> +    return rc;
> +}
> +EXPORT_SYMBOL_GPL(kvm_intel_iommu_context_mapping);

What does the return value mean?

> +
> +int kvm_intel_iommu_page_mapping(
> +    struct dmar_domain *domain, dma_addr_t iova,
> +    u64 hpa, size_t size, int prot)
> +{
> +    int rc;
> +    rc = domain_page_mapping(domain, iova, hpa, size, prot);
> +    return rc;
> +}
> +EXPORT_SYMBOL_GPL(kvm_intel_iommu_page_mapping);

The function name makes it sound like it's retrieving information.  If 
it does something, put a verb in there.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.