Re: [RFC PATCH 6/7] common/pv-iommu: Add foreign ops to PV-IOMMU interface

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Feb 10, 2016 at 10:10:34AM +0000, Malcolm Crossley wrote:
> Currently the ops are X86 only due to a dependency on the
> X86 only B2M implementation.
> 
> Foreign ops conform to draft D of PV-IOMMU design.
> 
> XSM control been implemented to allow full security control of
> these priviledged operatins.

But you missed the decleration of them in access_vectors, the
default policy in xen.te and also the implementation in the hooks.c

And that should all be part of the code that adds the new hypercall.

> 
> Page references and page locking are taken before using B2M
> interface which is mandated by the B2M interface itself.
> 
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> --
> Cc: dgdegra@tycho.nsa.gov
> Cc: jbeulich@suse.com
> Cc: ian.campbell@citrix.com
> Cc: keir@xen.org
> Cc: tim@xen.org
> Cc: andrew.cooper3@citrix.com
> Cc: xen-devel@lists.xen.org
> ---
>  xen/common/pv_iommu.c         | 269 ++++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/pv-iommu.h |  22 ++++
>  xen/include/xsm/dummy.h       |   6 +
>  xen/include/xsm/xsm.h         |   6 +
>  xen/xsm/dummy.c               |   1 +
>  5 files changed, 304 insertions(+)
> 
> diff --git a/xen/common/pv_iommu.c b/xen/common/pv_iommu.c
> index 91485f3..bbf1a21 100644
> --- a/xen/common/pv_iommu.c
> +++ b/xen/common/pv_iommu.c
> @@ -21,7 +21,12 @@
>  #include <asm/event.h>
>  #include <xen/guest_access.h>
>  #include <public/pv-iommu.h>
> +#include <xsm/xsm.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/m2b.h>
> +#include <asm/setup.h>
> +#endif
>  #define ret_t long
>  
>  static int get_paged_frame(unsigned long gfn, unsigned long *frame,
> @@ -79,6 +84,7 @@ void do_iommu_sub_op(struct pv_iommu_op *op)
>  {
>      struct domain *d = current->domain;
>      struct domain *rd = NULL;
> +    int ret;
>  
>      /* Only order 0 pages supported */
>      if ( IOMMU_get_page_order(op->flags) != 0 )
> @@ -183,7 +189,270 @@ void do_iommu_sub_op(struct pv_iommu_op *op)
>              op->status = 0;
>              break;
>          }
> +#ifdef CONFIG_X86

Why not move all of this in arch/x86/pv_iommu.c code that would
implement per-arch code?

> +        case IOMMUOP_map_foreign_page:

Didn't look at the rest of the code - will when you repost.