[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
On 05/12/2022 13:48, Rahul Singh wrote:
Hi Julien,
On 3 Dec 2022, at 9:54 pm, Julien Grall <julien@xxxxxxx> wrote:
Hi Rahul,
On 01/12/2022 16:02, Rahul Singh wrote:
This patch adds basic framework for vIOMMU.
Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
---
xen/arch/arm/domain.c | 17 +++++++
xen/arch/arm/domain_build.c | 3 ++
xen/arch/arm/include/asm/viommu.h | 70 ++++++++++++++++++++++++++++
xen/drivers/passthrough/Kconfig | 6 +++
xen/drivers/passthrough/arm/Makefile | 1 +
xen/drivers/passthrough/arm/viommu.c | 48 +++++++++++++++++++
xen/include/public/arch-arm.h | 4 ++
7 files changed, 149 insertions(+)
create mode 100644 xen/arch/arm/include/asm/viommu.h
create mode 100644 xen/drivers/passthrough/arm/viommu.c
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 38e22f12af..2a85209736 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -37,6 +37,7 @@
#include <asm/tee/tee.h>
#include <asm/vfp.h>
#include <asm/vgic.h>
+#include <asm/viommu.h>
#include <asm/vtimer.h>
#include "vpci.h"
@@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+ if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
+ {
+ dprintk(XENLOG_INFO,
+ "vIOMMU type requested not supported by the platform or
Xen\n");
+ return -EINVAL;
+ }
+
return 0;
}
@@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
if ( (rc = domain_vpci_init(d)) != 0 )
goto fail;
+ if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
+ goto fail;
+
return 0;
fail:
@@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
page_list_head *list)
enum {
PROG_pci = 1,
PROG_tee,
+ PROG_viommu,
PROG_xen,
PROG_page,
PROG_mapping,
@@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
if (ret )
return ret;
+ PROGRESS(viommu):
+ ret = viommu_relinquish_resources(d);
+ if (ret )
+ return ret;
I would have expected us to relinquish the vIOMMU resources *before* we detach
the devices. So can you explain the ordering?
I think first we need to detach the device that will set the S1 and S2 stage
translation to bypass/abort then we
need to remove the vIOMMU. Do you have anything that you feel if we relinquish
the vIOMMU after detach is not right.
iommu_release_dt_devices() will effectively remove the device from the
domain. One could decide to store the S1 information per device and
therefore it feels wrong that we are removing the S1 information later on.
In addition to that, a device can be removed while the domain is running.
Therefore I think it would make more sense to remove the vIOMMU every
time we deassign the device.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bd30d3798c..abbaf37a2e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -27,6 +27,7 @@
#include <asm/setup.h>
#include <asm/cpufeature.h>
#include <asm/domain_build.h>
+#include <asm/viommu.h>
#include <xen/event.h>
#include <xen/irq.h>
@@ -3858,6 +3859,7 @@ void __init create_domUs(void)
struct domain *d;
struct xen_domctl_createdomain d_cfg = {
.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
+ .arch.viommu_type = viommu_get_type(),
I don't think the vIOMMU should be enabled to dom0less domU by default.
I am not enabling the vIOMMU by default. If vIOMMU is disabled via the command
line or config option
then viommu_get_type() will return XEN_DOMCTL_CONFIG_VIOMMU_NONE and in that
case
domain_viommu_init() will return 0 without doing anything.
What I mean by default is if the command line is set then you will
enable the vIOMMU for both dom0 and dom0less domUs.
vIOMMU should be selected per-domain.
.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
/*
* The default of 1023 should be sufficient for guests because
@@ -4052,6 +4054,7 @@ void __init create_dom0(void)
printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
dom0_cfg.arch.tee_type = tee_get_type();
dom0_cfg.max_vcpus = dom0_max_vcpus();
+ dom0_cfg.arch.viommu_type = viommu_get_type();
Same here.
if ( iommu_enabled )
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
diff --git a/xen/arch/arm/include/asm/viommu.h
b/xen/arch/arm/include/asm/viommu.h
new file mode 100644
index 0000000000..7cd3818a12
--- /dev/null
+++ b/xen/arch/arm/include/asm/viommu.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
+#ifndef __ARCH_ARM_VIOMMU_H__
+#define __ARCH_ARM_VIOMMU_H__
+
+#ifdef CONFIG_VIRTUAL_IOMMU
+
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <public/xen.h>
+
+struct viommu_ops {
+ /*
+ * Called during domain construction if toolstack requests to enable
+ * vIOMMU support.
+ */
+ int (*domain_init)(struct domain *d);
+
+ /*
+ * Called during domain destruction to free resources used by vIOMMU.
+ */
+ int (*relinquish_resources)(struct domain *d);
+};
+
+struct viommu_desc {
+ /* vIOMMU domains init/free operations described above. */
+ const struct viommu_ops *ops;
+
+ /*
+ * ID of vIOMMU. Corresponds to xen_arch_domainconfig.viommu_type.
Did you mean ID rather than type?
I mean here type of physical IOMMU on the host available SMMUv3, SMMUv1 or
IPMMU.
If you think ID is the right name here I will change it.
Hmmm... I inverted the two names. I was actually asking whether ID
should be Type instead.
Cheers,
--
Julien Grall
|
