Qemu: Add Xen vIOMMU support

[Qemu-devel] [RFC PATCH 2/4] Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen

Posted by Lan Tianyu 8 years, 10 months ago

From: Chao Gao <chao.gao@intel.com>

Since adding a dynamic sysbus device is forbidden, so choose TYPE_DEVICE
as parent class.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 hw/xen/Makefile.objs |   1 +
 hw/xen/xen_viommu.c  | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 hw/xen/xen_viommu.c

diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index d367094..e37d808 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -3,3 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
 
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_graphics.o xen_pt_msi.o
+obj-$(CONFIG_XEN) += xen_viommu.o
diff --git a/hw/xen/xen_viommu.c b/hw/xen/xen_viommu.c
new file mode 100644
index 0000000..9bf9158
--- /dev/null
+++ b/hw/xen/xen_viommu.c
@@ -0,0 +1,116 @@
+/*
+ * Xen virtual IOMMU (virtual VT-D)
+ *
+ * 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 "qemu/osdep.h"
+#include "qemu/error-report.h"
+
+#include "hw/qdev-core.h"
+#include "hw/sysbus.h"
+#include "hw/xen/xen.h"
+#include "hw/xen/xen_backend.h"
+
+#define TYPE_XEN_VIOMMU_DEVICE "xen_viommu"
+#define  XEN_VIOMMU_DEVICE(obj) \
+    OBJECT_CHECK(XenVIOMMUState, (obj), TYPE_XEN_VIOMMU_DEVICE)
+
+typedef struct XenVIOMMUState XenVIOMMUState;
+
+struct XenVIOMMUState {
+    DeviceState dev;
+    uint32_t id;
+    uint64_t cap;
+    uint64_t base_addr;
+};
+
+static void xen_viommu_realize(DeviceState *dev, Error **errp)
+{
+    int rc;
+    uint64_t cap;
+    char *dom;
+    char viommu_path[1024];
+    XenVIOMMUState *s = XEN_VIOMMU_DEVICE(dev);
+
+    s->id = -1;
+    
+    /* Read vIOMMU attributes from Xenstore. */
+    dom = xs_get_domain_path(xenstore, xen_domid);
+    snprintf(viommu_path, sizeof(viommu_path), "%s/viommu", dom);
+    rc = xenstore_read_uint64(viommu_path, "base_addr", &s->base_addr);  
+    if (rc) {
+        error_report("Can't get base address of vIOMMU");
+        exit(1);
+    }
+
+    rc = xenstore_read_uint64(viommu_path, "cap", &s->cap);
+    if (rc) {
+        error_report("Can't get capabilities of vIOMMU");
+        exit(1);
+    }
+
+    rc = xc_viommu_query_cap(xen_xc, xen_domid, &cap);
+    if (rc) {
+        exit(1);
+    }
+
+
+    if ((cap & s->cap) != cap) {
+        error_report("xen: Unsupported capability %lx", s->cap);
+        exit(1);
+    }
+
+    rc = xc_viommu_create(xen_xc, xen_domid, s->base_addr, s->cap, &s->id);
+    if (rc) {
+        s->id = -1;
+        error_report("xen: failed(%d) to create viommu ", rc);
+        exit(1);
+    }
+}
+
+static void xen_viommu_instance_finalize(Object *o)
+{
+    int rc;
+    XenVIOMMUState *s = XEN_VIOMMU_DEVICE(o);
+
+    if (s->id != -1) {
+        rc = xc_viommu_destroy(xen_xc, xen_domid, s->id); 
+        if (rc) {
+            error_report("xen: failed(%d) to destroy viommu ", rc);
+        }
+    }
+}
+
+static void xen_viommu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->hotpluggable = false;
+    dc->realize = xen_viommu_realize;
+}
+
+static const TypeInfo xen_viommu_info = {
+    .name              = TYPE_XEN_VIOMMU_DEVICE,
+    .parent            = TYPE_SYS_BUS_DEVICE,
+    .instance_size     = sizeof(XenVIOMMUState),
+    .instance_finalize = xen_viommu_instance_finalize,
+    .class_init        = xen_viommu_class_init,
+};
+
+static void xen_viommu_register_types(void)
+{
+    type_register_static(&xen_viommu_info); 
+}
+
+type_init(xen_viommu_register_types);
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 2/4] Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen

Posted by Anthony PERARD 8 years, 10 months ago

Hi,

On Fri, Mar 17, 2017 at 07:29:15PM +0800, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> Since adding a dynamic sysbus device is forbidden, so choose TYPE_DEVICE
> as parent class.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  hw/xen/Makefile.objs |   1 +
>  hw/xen/xen_viommu.c  | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>  create mode 100644 hw/xen/xen_viommu.c
> 
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index d367094..e37d808 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -3,3 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
>  
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_graphics.o xen_pt_msi.o
> +obj-$(CONFIG_XEN) += xen_viommu.o
> diff --git a/hw/xen/xen_viommu.c b/hw/xen/xen_viommu.c
> new file mode 100644
> index 0000000..9bf9158
> --- /dev/null
> +++ b/hw/xen/xen_viommu.c
> @@ -0,0 +1,116 @@
> +/*
> + * Xen virtual IOMMU (virtual VT-D)
> + *
> + * 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 "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +
> +#include "hw/qdev-core.h"
> +#include "hw/sysbus.h"
> +#include "hw/xen/xen.h"
> +#include "hw/xen/xen_backend.h"
> +
> +#define TYPE_XEN_VIOMMU_DEVICE "xen_viommu"
> +#define  XEN_VIOMMU_DEVICE(obj) \
> +    OBJECT_CHECK(XenVIOMMUState, (obj), TYPE_XEN_VIOMMU_DEVICE)
> +
> +typedef struct XenVIOMMUState XenVIOMMUState;
> +
> +struct XenVIOMMUState {
> +    DeviceState dev;
> +    uint32_t id;
> +    uint64_t cap;
> +    uint64_t base_addr;
> +};
> +
> +static void xen_viommu_realize(DeviceState *dev, Error **errp)
> +{
> +    int rc;
> +    uint64_t cap;
> +    char *dom;
> +    char viommu_path[1024];
> +    XenVIOMMUState *s = XEN_VIOMMU_DEVICE(dev);
> +
> +    s->id = -1;
> +    
> +    /* Read vIOMMU attributes from Xenstore. */
> +    dom = xs_get_domain_path(xenstore, xen_domid);
> +    snprintf(viommu_path, sizeof(viommu_path), "%s/viommu", dom);
> +    rc = xenstore_read_uint64(viommu_path, "base_addr", &s->base_addr);  

Could these informations (base_addr and cap) be read from the command
line instead of via xenstore?
Any reason for these informations to be on xenstore?

> +    if (rc) {
> +        error_report("Can't get base address of vIOMMU");

I think error_setg should be used instead of error_report.

> +        exit(1);

Also, exit should be remove, and return instead. error_setg would be
enough to signal that the device can not work.

> +    }
> +
> +    rc = xenstore_read_uint64(viommu_path, "cap", &s->cap);
> +    if (rc) {
> +        error_report("Can't get capabilities of vIOMMU");
> +        exit(1);
> +    }
> +
> +    rc = xc_viommu_query_cap(xen_xc, xen_domid, &cap);

Since xc_viommu_* seems to be new, you should use
xendevicemodel_viommu_* instead, also you will need to define a stub for
them to be able to compile QEMU against older version of Xen.


The patch looks good otherwise.

Thanks,

-- 
Anthony PERARD

Re: [Qemu-devel] [RFC PATCH 2/4] Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen

Posted by Chao Gao 8 years, 10 months ago

On Thu, Mar 30, 2017 at 05:24:52PM +0100, Anthony PERARD wrote:
>Hi,
>
>On Fri, Mar 17, 2017 at 07:29:15PM +0800, Lan Tianyu wrote:
>> From: Chao Gao <chao.gao@intel.com>
>> 
>> Since adding a dynamic sysbus device is forbidden, so choose TYPE_DEVICE
>> as parent class.
>> 
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  hw/xen/Makefile.objs |   1 +
>>  hw/xen/xen_viommu.c  | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 117 insertions(+)
>>  create mode 100644 hw/xen/xen_viommu.c
>> 
>> +static void xen_viommu_realize(DeviceState *dev, Error **errp)
>> +{
>> +    int rc;
>> +    uint64_t cap;
>> +    char *dom;
>> +    char viommu_path[1024];
>> +    XenVIOMMUState *s = XEN_VIOMMU_DEVICE(dev);
>> +
>> +    s->id = -1;
>> +    
>> +    /* Read vIOMMU attributes from Xenstore. */
>> +    dom = xs_get_domain_path(xenstore, xen_domid);
>> +    snprintf(viommu_path, sizeof(viommu_path), "%s/viommu", dom);
>> +    rc = xenstore_read_uint64(viommu_path, "base_addr", &s->base_addr);  
>
>Could these informations (base_addr and cap) be read from the command
>line instead of via xenstore?
>Any reason for these informations to be on xenstore?

Actually, we passed both via command line at first. We just concerned
whether it was ok to pass a address through command line since
we find no device does the similar thing.

>
>> +    if (rc) {
>> +        error_report("Can't get base address of vIOMMU");
>
>I think error_setg should be used instead of error_report.
>
>> +        exit(1);
>
>Also, exit should be remove, and return instead. error_setg would be
>enough to signal that the device can not work.
>
>> +    }
>> +
>> +    rc = xenstore_read_uint64(viommu_path, "cap", &s->cap);
>> +    if (rc) {
>> +        error_report("Can't get capabilities of vIOMMU");
>> +        exit(1);
>> +    }
>> +
>> +    rc = xc_viommu_query_cap(xen_xc, xen_domid, &cap);
>
>Since xc_viommu_* seems to be new, you should use
>xendevicemodel_viommu_* instead, also you will need to define a stub for
>them to be able to compile QEMU against older version of Xen.

Will follow your suggestions above.

Thanks
Chao

>
>
>The patch looks good otherwise.
>
>Thanks,
>
>-- 
>Anthony PERARD

[Qemu-devel] [RFC PATCH 1/4] I440: Allow adding sysbus devices with -device on I440
[Qemu-devel] [RFC PATCH 2/4] Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen
[Qemu-devel] [RFC PATCH 3/4] xen-pt: bind/unbind interrupt remapping format MSI
[Qemu-devel] [RFC PATCH 4/4] msi: taking interrupt format into consideration during judging a pirq is binded with a event channel