Re: [Xen-devel] [PATCH 3/3] iommu: add xc_iommu_map_batch

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 18:00 +0000 on 15 Nov (1289844022), Jean Guyader wrote:
> diff -r 48256256333b xen/arch/x86/domctl.c
> --- a/xen/arch/x86/domctl.c   Mon Nov 15 11:54:43 2010 +0000
> +++ b/xen/arch/x86/domctl.c   Mon Nov 15 17:45:09 2010 +0000
> @@ -953,6 +953,44 @@
>      }
>      break;
>  
> +    case XEN_DOMCTL_iommu_map_batch:
> +    {
> +        struct domain *d;
> +        unsigned long gfn = domctl->u.iommu_map_batch.gfn;
> +        unsigned long nr = domctl->u.iommu_map_batch.nr;
> +        unsigned long mfn;
> +        int i = 0;
> +
> +        printk("iommu_map_batch: domid_to:%d gfn:0x%lx size:0x%lx\n",
> +                domctl->domain, gfn, nr);

This should be gdprintk, at most.

> +
> +        ret = -EINVAL;
> +        if ( (gfn + nr - 1) < gfn) /* wrap? */
> +            break;
> +
> +        ret = -ESRCH;
> +        if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
> +            break;

ITYM rcu_lock_target_domain_by_id, otherwise there are no privilege
checks on this call.  It needs xcp hooks too. 

> +        if (d != current->domain)
> +            domain_pause(d);
> +        for ( i = 0; i < nr; i++)
> +        {
> +            ret = -EFAULT;
> +            if (copy_from_guest_offset(&mfn, domctl->u.iommu_map_batch.mfns, 
> i, 1))

The hypercall interface defines the mfn list as uint64s but you're
copying them as unsigned longs.  That'll break on 32-bit builds.

> +                break;
> +            if ((ret = iommu_set_entry(d, gfn + i, mfn, 
> IOMMUF_readable|IOMMUF_writable, 1)) != 0)

For HVM guests, don't you want to keep the p2m in sync as well?

> +                break;
> +        }
> +        if (i)
> +            iommu_flush_iotlb(d, gfn, nr - ( nr - i));
> +        if (d != current->domain)
> +            domain_unpause(d);
> +
> +        rcu_unlock_domain(d);
> +    }
> +    break;
> +
>      case XEN_DOMCTL_memory_mapping:
>      {
>          struct domain *d;
> diff -r 48256256333b xen/include/public/domctl.h
> --- a/xen/include/public/domctl.h     Mon Nov 15 11:54:43 2010 +0000
> +++ b/xen/include/public/domctl.h     Mon Nov 15 17:45:09 2010 +0000
> @@ -517,6 +517,13 @@
>  typedef struct xen_domctl_memory_mapping xen_domctl_memory_mapping_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_memory_mapping_t);
>  
> +struct xen_domctl_iommu_map_batch {
> +    uint64_aligned_t gfn;
> +    uint64_aligned_t nr;
> +    XEN_GUEST_HANDLE_64(uint64) mfns;
> +};
> +typedef struct xen_domctl_iommu_map_batch xen_domctl_iommu_map_batch_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_iommu_map_batch_t);

Needs a comment describing what this hypercall does. 

>  /* Bind machine I/O port range -> HVM I/O port range. */
>  /* XEN_DOMCTL_ioport_mapping */
> @@ -868,6 +875,7 @@
>  #define XEN_DOMCTL_getpageframeinfo3             61
>  #define XEN_DOMCTL_setvcpuextstate               62
>  #define XEN_DOMCTL_getvcpuextstate               63
> +#define XEN_DOMCTL_iommu_map_batch               64
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -907,6 +915,7 @@
>          struct xen_domctl_assign_device     assign_device;
>          struct xen_domctl_bind_pt_irq       bind_pt_irq;
>          struct xen_domctl_memory_mapping    memory_mapping;
> +        struct xen_domctl_iommu_map_batch   iommu_map_batch;
>          struct xen_domctl_ioport_mapping    ioport_mapping;
>          struct xen_domctl_pin_mem_cacheattr pin_mem_cacheattr;
>          struct xen_domctl_ext_vcpucontext   ext_vcpucontext;

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel


-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel