Re: [PATCH v7 1/7] public / x86: introduce __HYPERCALL_iommu_op

From: "Jan Beulich" <JBeulich@suse.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v7 1/7] public / x86: introduce __HYPERCALL_iommu_op
Date: Fri, 09 Nov 2018 04:23:39 -0700	[thread overview]
Message-ID: <5BE56E3B02000078001FA21D@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <20181015103559.22075-2-paul.durrant@citrix.com>

>>> On 15.10.18 at 12:35, <paul.durrant@citrix.com> wrote:
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
>  obj-y += grant_table.o
>  obj-y += guestcopy.o
>  obj-bin-y += gunzip.init.o
> +obj-$(CONFIG_X86) += iommu_op.o

I'm afraid I didn't notice this placement in earlier versions. Doesn't
the new hypercalls as a prereq take host IOMMU functionality? If
so, why would this not live in e.g. drivers/passthrough/ (under
this or perhaps a different name)?

Also now that we have full PV and HVM separation - is the
hypercall meant to be usable by both types of guests from the
very beginning? Otherwise conditionals will need to be
introduced above (expressed by the common list model if at all
possible) or in the source file.

> --- /dev/null
> +++ b/xen/common/iommu_op.c
> @@ -0,0 +1,193 @@
> +/******************************************************************************
> + * iommu_op.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/>.
> + *
> + * Copyright (C) 2018 Citrix Systems Inc
> + */
> +
> +#include <xen/event.h>
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +
> +static void iommu_op(xen_iommu_op_t *op)
> +{
> +    switch ( op->op )
> +    {
> +    default:
> +        op->status = -EOPNOTSUPP;
> +        break;
> +    }
> +}
> +
> +int do_one_iommu_op(xen_iommu_op_buf_t *buf)

static? const?

> +#ifdef CONFIG_COMPAT
> +
> +int compat_one_iommu_op(compat_iommu_op_buf_t *buf)

Same here.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel