Re: [RFC PATCH 1/7] common/pv-iommu: Add stub hypercall for PV-IOMMU

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

On Wed, Feb 10, 2016 at 10:10:29AM +0000, Malcolm Crossley wrote:
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> --
> Cc: jbeulich@suse.com
> Cc: andrew.cooper3@citrix.com
> Cc: ian.campbell@citrix.com
> Cc: keir@xen.org
> Cc: tim@xen.org
> Cc: xen-devel@lists.xen.org
> ---
>  xen/arch/x86/x86_64/compat/entry.S |  2 ++
>  xen/arch/x86/x86_64/entry.S        |  2 ++
>  xen/common/Makefile                |  1 +
>  xen/common/pv_iommu.c              | 38 ++++++++++++++++++++++++++++++++++++++
>  xen/include/public/xen.h           |  1 +
>  5 files changed, 44 insertions(+)
>  create mode 100644 xen/common/pv_iommu.c
> 
> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
> index 3088aa7..53a1689 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -436,6 +436,7 @@ ENTRY(compat_hypercall_table)
>          .quad do_tmem_op
>          .quad do_ni_hypercall           /* reserved for XenClient */
>          .quad do_xenpmu_op              /* 40 */
> +        .quad do_iommu_op
>          .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
>          .quad compat_ni_hypercall
>          .endr
> @@ -487,6 +488,7 @@ ENTRY(compat_hypercall_args_table)
>          .byte 1 /* do_tmem_op               */
>          .byte 0 /* reserved for XenClient   */
>          .byte 2 /* do_xenpmu_op             */  /* 40 */
> +        .byte 2 /* do_iommu_op              */
>          .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
>          .byte 0 /* compat_ni_hypercall      */
>          .endr
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index 94a54aa..fee7191 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -769,6 +769,7 @@ ENTRY(hypercall_table)
>          .quad do_tmem_op
>          .quad do_ni_hypercall       /* reserved for XenClient */
>          .quad do_xenpmu_op          /* 40 */
> +        .quad do_iommu_op
>          .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
>          .quad do_ni_hypercall
>          .endr
> @@ -820,6 +821,7 @@ ENTRY(hypercall_args_table)
>          .byte 1 /* do_tmem_op           */
>          .byte 0 /* reserved for XenClient */
>          .byte 2 /* do_xenpmu_op         */  /* 40 */
> +        .byte 2 /* do_iommu_op          */
>          .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
>          .byte 0 /* do_ni_hypercall      */
>          .endr
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 6e82b33..b498589 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -25,6 +25,7 @@ obj-y += notifier.o
>  obj-y += page_alloc.o
>  obj-$(CONFIG_HAS_PDX) += pdx.o
>  obj-y += preempt.o
> +obj-y += pv_iommu.o

Perhaps have an Kconfig entry for it?

Also you seem to be missing ARM code?

>  obj-y += random.o
>  obj-y += rangeset.o
>  obj-y += radix-tree.o
> diff --git a/xen/common/pv_iommu.c b/xen/common/pv_iommu.c
> new file mode 100644
> index 0000000..304fccf
> --- /dev/null
> +++ b/xen/common/pv_iommu.c
> @@ -0,0 +1,38 @@
> +/******************************************************************************
> + * common/pv_iommu.c
> + * 
> + * Paravirtualised IOMMU functionality
> + * 
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + * 
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + * 
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/guest_access.h>
> +
> +#define ret_t long

? What is wrong with just using 'long'?

> +
> +ret_t do_iommu_op(XEN_GUEST_HANDLE_PARAM(void) arg, unsigned int count)
> +{
> +    return -ENOSYS;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 7b629b1..ff50e7a 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -102,6 +102,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_tmem_op              38
>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
>  #define __HYPERVISOR_xenpmu_op            40
> +#define __HYPERVISOR_iommu_op             41

I would think there would be an pv_iommu.h header file as well?
>  
>  /* Architecture-specific hypercall definitions. */
>  #define __HYPERVISOR_arch_0               48
> -- 
> 1.7.12.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel