This patch defines a new bit reported in the hw_cap field of struct
xen_sysctl_physinfo to indicate whether the platform supports sharing of
HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack
whether the domain needs extra memory to store discrete IOMMU page tables
or not.
NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not
supported or the IOMMU is disabled, and defines it to false if
!CONFIG_HVM.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Christian Lindig <christian.lindig@citrix.com>
Cc: David Scott <dave@recoil.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
v11:
- Fix abi-check breakage
v10:
- Set flag in common code (which means clearing iommu_hap_pt_share if
HAP cannot be enabled or is configured out).
v9:
- New in v9
---
tools/libxl/libxl.c | 2 ++
tools/libxl/libxl.h | 7 +++++++
tools/libxl/libxl_types.idl | 1 +
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 3 ++-
tools/xl/xl_info.c | 5 +++--
xen/arch/x86/hvm/hvm.c | 25 +++++++++++++++++-------
xen/common/sysctl.c | 2 ++
xen/drivers/passthrough/amd/iommu_init.c | 5 ++++-
xen/drivers/passthrough/iommu.c | 18 ++++++++++++++++-
xen/drivers/passthrough/vtd/iommu.c | 6 +++++-
xen/include/public/sysctl.h | 6 +++++-
xen/include/xen/iommu.h | 8 +++++++-
13 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 57073c06d5..a0d84281d0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -402,6 +402,8 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
physinfo->cap_shadow =
!!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_shadow);
+ physinfo->cap_iommu_hap_pt_share =
+ !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share);
GC_FREE;
return 0;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 466df2cdf5..8169d44bda 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -401,6 +401,13 @@
*/
#define LIBXL_HAVE_PHYSINFO_CAP_HAP_SHADOW 1
+/*
+ * LIBXL_HAVE_PHYSINFO_CAP_IOMMU_HAP_PT_SHARE indicates that libxl_physinfo
+ * has a cap_iommu_hap_pt_share field that indicates whether the hardware
+ * supports sharing the IOMMU and HAP page tables.
+ */
+#define LIBXL_HAVE_PHYSINFO_CAP_IOMMU_HAP_PT_SHARE 1
+
/*
* libxl ABI compatibility
*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6f431baec2..7253d6e0fb 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1027,6 +1027,7 @@ libxl_physinfo = Struct("physinfo", [
("cap_hvm_directio", bool), # No longer HVM specific
("cap_hap", bool),
("cap_shadow", bool),
+ ("cap_iommu_hap_pt_share", bool),
], dir=DIR_OUT)
libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 35dddbbd9c..de4bae6012 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -110,6 +110,7 @@ type physinfo_cap_flag =
| CAP_DirectIO
| CAP_HAP
| CAP_Shadow
+ | CAP_IOMMU_HAP_PT_SHARE
type physinfo =
{
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 0dd55e9d8b..c885e75895 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -57,7 +57,6 @@ type domain_create_flag =
| CDF_OOS_OFF
| CDF_XS_DOMAIN
| CDF_IOMMU
-
type domctl_create_config = {
ssidref: int32;
handle: string;
@@ -95,6 +94,8 @@ type physinfo_cap_flag =
| CAP_DirectIO
| CAP_HAP
| CAP_Shadow
+ | CAP_IOMMU_HAP_PT_SHARE
+
type physinfo = {
threads_per_core : int;
cores_per_socket : int;
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 148c4740ae..bfbca93997 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -210,13 +210,14 @@ static void output_physinfo(void)
info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
);
- maybe_printf("virt_caps :%s%s%s%s%s%s\n",
+ maybe_printf("virt_caps :%s%s%s%s%s%s%s\n",
info.cap_pv ? " pv" : "",
info.cap_hvm ? " hvm" : "",
info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
info.cap_hap ? " hap" : "",
- info.cap_shadow ? " shadow" : ""
+ info.cap_shadow ? " shadow" : "",
+ info.cap_iommu_hap_pt_share ? " iommu_hap_pt_share" : ""
);
vinfo = libxl_get_version_info(ctx);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3831c6d4c1..ed8272cf93 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -142,6 +142,22 @@ static struct notifier_block cpu_nfb = {
.notifier_call = cpu_callback
};
+static bool __init hap_supported(const struct hvm_function_table *fns)
+{
+ if ( !fns->hap_supported )
+ {
+ printk("HVM: Hardware Assisted Paging (HAP) not detected\n");
+ return false;
+ }
+ else if ( !opt_hap_enabled )
+ {
+ printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
+ return false;
+ }
+
+ return true;
+}
+
static int __init hvm_enable(void)
{
const struct hvm_function_table *fns = NULL;
@@ -158,13 +174,8 @@ static int __init hvm_enable(void)
hvm_enabled = 1;
printk("HVM: %s enabled\n", fns->name);
- if ( !fns->hap_supported )
- printk("HVM: Hardware Assisted Paging (HAP) not detected\n");
- else if ( !opt_hap_enabled )
- {
- hvm_funcs.hap_supported = 0;
- printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
- }
+ if ( !hap_supported(fns) )
+ iommu_hap_pt_share = false;
else
{
printk("HVM: Hardware Assisted Paging (HAP) detected\n");
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 92b4ea0d21..ef840fcb76 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -269,6 +269,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
arch_do_physinfo(pi);
if ( iommu_enabled )
pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
+ if ( iommu_hap_pt_share )
+ pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
if ( copy_to_guest(u_sysctl, op, 1) )
ret = -EFAULT;
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index bb5a3e57c9..2e011d4e43 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1401,12 +1401,15 @@ int __init amd_iommu_init(bool xt)
if ( rc )
goto error_out;
+#ifndef iommu_hap_pt_share
/*
* Disable sharing HAP page tables with AMD IOMMU,
* since it only supports p2m_ram_rw, and this would
* prevent doing IO to/from mapped grant frames.
*/
- iommu_hap_pt_share = 0;
+ iommu_hap_pt_share = false;
+#endif
+
printk(XENLOG_DEBUG "AMD-Vi: Disabled HAP memory map sharing with IOMMU\n");
/* per iommu initialization */
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 09ce9d9294..bdf12f85e6 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -49,7 +49,11 @@ int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
* default until we find a good solution to resolve it.
*/
bool_t __read_mostly iommu_intpost;
-bool_t __read_mostly iommu_hap_pt_share = 1;
+
+#ifndef iommu_hap_pt_share
+bool __read_mostly iommu_hap_pt_share = true;
+#endif
+
bool_t __read_mostly iommu_debug;
bool_t __read_mostly amd_iommu_perdev_intremap = 1;
@@ -103,7 +107,14 @@ static int __init parse_iommu_param(const char *s)
else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
iommu_hwdom_strict = val;
else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
+ {
+#ifndef iommu_hap_pt_share
iommu_hap_pt_share = val;
+#else
+ if (val != iommu_hap_pt_share)
+ rc = -EINVAL;
+#endif
+ }
else
rc = -EINVAL;
@@ -511,7 +522,12 @@ int __init iommu_setup(void)
iommu_enabled = (rc == 0);
}
if ( !iommu_enabled )
+ {
iommu_intremap = 0;
+#ifndef iommu_hap_pt_share
+ iommu_hap_pt_share = false;
+#endif
+ }
if ( (force_iommu && !iommu_enabled) ||
(force_intremap && !iommu_intremap) )
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7ffafdc065..a7381a7449 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1898,6 +1898,7 @@ int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
return rc;
}
+#ifndef iommu_hap_pt_share
static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
{
u64 ept_cap, vtd_cap = iommu->cap;
@@ -1910,6 +1911,7 @@ static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
return (ept_has_2mb(ept_cap) && opt_hap_2mb) == cap_sps_2mb(vtd_cap) &&
(ept_has_1gb(ept_cap) && opt_hap_1gb) == cap_sps_1gb(vtd_cap);
}
+#endif
/*
* set VT-d page table directory to EPT table if allowed
@@ -2309,8 +2311,10 @@ static int __init vtd_setup(void)
if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 )
iommu_intpost = 0;
+#ifndef iommu_hap_pt_share
if ( !vtd_ept_page_compatible(iommu) )
- iommu_hap_pt_share = 0;
+ iommu_hap_pt_share = false;
+#endif
ret = iommu_set_interrupt(drhd);
if ( ret )
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index e324442f92..19457a4e30 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -96,9 +96,13 @@ struct xen_sysctl_tbuf_op {
/* The platform supports software paging. */
#define _XEN_SYSCTL_PHYSCAP_shadow 4
#define XEN_SYSCTL_PHYSCAP_shadow (1u<<_XEN_SYSCTL_PHYSCAP_shadow)
+/* The platform supports sharing of HAP page tables with the IOMMU. */
+#define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
+#define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share \
+ (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
/* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */
-#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_shadow
+#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share
struct xen_sysctl_physinfo {
uint32_t threads_per_core;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index ab258b848b..a519f4d87b 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -55,7 +55,13 @@ static inline bool_t dfn_eq(dfn_t x, dfn_t y)
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_verbose, iommu_igfx;
extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
-extern bool_t iommu_hap_pt_share;
+
+#ifdef CONFIG_HVM
+extern bool iommu_hap_pt_share;
+#else
+#define iommu_hap_pt_share false
+#endif
+
extern bool_t iommu_debug;
extern bool_t amd_iommu_perdev_intremap;
--
2.20.1.2.gb21ebb671
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
This patch defines a new bit reported in the hw_cap field of struct
xen_sysctl_physinfo to indicate whether the platform supports sharing of
HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack
whether the domain needs extra memory to store discrete IOMMU page tables
or not.
NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not
supported or the IOMMU is disabled, and defines it to false if
!CONFIG_HVM.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Christian Lindig <christian.lindig@citrix.com>
Cc: David Scott <dave@recoil.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
v11.1:
- introduce clear_iommu_hap_pt_share()
- drop unnecessary "else"
v11:
- Fix abi-check breakage
v10:
- Set flag in common code (which means clearing iommu_hap_pt_share if
HAP cannot be enabled or is configured out).
v9:
- New in v9
---
tools/libxl/libxl.c | 2 ++
tools/libxl/libxl.h | 7 +++++++
tools/libxl/libxl_types.idl | 1 +
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 3 ++-
tools/xl/xl_info.c | 5 +++--
xen/arch/x86/hvm/hvm.c | 26 +++++++++++++++++++-------
xen/common/sysctl.c | 2 ++
xen/drivers/passthrough/amd/iommu_init.c | 3 ++-
xen/drivers/passthrough/iommu.c | 11 ++++++++++-
xen/drivers/passthrough/vtd/iommu.c | 2 +-
xen/include/public/sysctl.h | 6 +++++-
xen/include/xen/iommu.h | 17 ++++++++++++++++-
13 files changed, 71 insertions(+), 15 deletions(-)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -402,6 +402,8 @@ int libxl_get_physinfo(libxl_ctx *ctx, l
physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
physinfo->cap_shadow =
!!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_shadow);
+ physinfo->cap_iommu_hap_pt_share =
+ !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share);
GC_FREE;
return 0;
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -402,6 +402,13 @@
#define LIBXL_HAVE_PHYSINFO_CAP_HAP_SHADOW 1
/*
+ * LIBXL_HAVE_PHYSINFO_CAP_IOMMU_HAP_PT_SHARE indicates that libxl_physinfo
+ * has a cap_iommu_hap_pt_share field that indicates whether the hardware
+ * supports sharing the IOMMU and HAP page tables.
+ */
+#define LIBXL_HAVE_PHYSINFO_CAP_IOMMU_HAP_PT_SHARE 1
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1027,6 +1027,7 @@ libxl_physinfo = Struct("physinfo", [
("cap_hvm_directio", bool), # No longer HVM specific
("cap_hap", bool),
("cap_shadow", bool),
+ ("cap_iommu_hap_pt_share", bool),
], dir=DIR_OUT)
libxl_connectorinfo = Struct("connectorinfo", [
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -110,6 +110,7 @@ type physinfo_cap_flag =
| CAP_DirectIO
| CAP_HAP
| CAP_Shadow
+ | CAP_IOMMU_HAP_PT_SHARE
type physinfo =
{
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -57,7 +57,6 @@ type domain_create_flag =
| CDF_OOS_OFF
| CDF_XS_DOMAIN
| CDF_IOMMU
-
type domctl_create_config = {
ssidref: int32;
handle: string;
@@ -95,6 +94,8 @@ type physinfo_cap_flag =
| CAP_DirectIO
| CAP_HAP
| CAP_Shadow
+ | CAP_IOMMU_HAP_PT_SHARE
+
type physinfo = {
threads_per_core : int;
cores_per_socket : int;
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -210,13 +210,14 @@ static void output_physinfo(void)
info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
);
- maybe_printf("virt_caps :%s%s%s%s%s%s\n",
+ maybe_printf("virt_caps :%s%s%s%s%s%s%s\n",
info.cap_pv ? " pv" : "",
info.cap_hvm ? " hvm" : "",
info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
info.cap_hap ? " hap" : "",
- info.cap_shadow ? " shadow" : ""
+ info.cap_shadow ? " shadow" : "",
+ info.cap_iommu_hap_pt_share ? " iommu_hap_pt_share" : ""
);
vinfo = libxl_get_version_info(ctx);
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -142,6 +142,23 @@ static struct notifier_block cpu_nfb = {
.notifier_call = cpu_callback
};
+static bool __init hap_supported(const struct hvm_function_table *fns)
+{
+ if ( !fns->hap_supported )
+ {
+ printk("HVM: Hardware Assisted Paging (HAP) not detected\n");
+ return false;
+ }
+
+ if ( !opt_hap_enabled )
+ {
+ printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
+ return false;
+ }
+
+ return true;
+}
+
static int __init hvm_enable(void)
{
const struct hvm_function_table *fns = NULL;
@@ -158,13 +175,8 @@ static int __init hvm_enable(void)
hvm_enabled = 1;
printk("HVM: %s enabled\n", fns->name);
- if ( !fns->hap_supported )
- printk("HVM: Hardware Assisted Paging (HAP) not detected\n");
- else if ( !opt_hap_enabled )
- {
- hvm_funcs.hap_supported = 0;
- printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
- }
+ if ( !hap_supported(fns) )
+ clear_iommu_hap_pt_share();
else
{
printk("HVM: Hardware Assisted Paging (HAP) detected\n");
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -269,6 +269,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
arch_do_physinfo(pi);
if ( iommu_enabled )
pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
+ if ( iommu_hap_pt_share )
+ pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
if ( copy_to_guest(u_sysctl, op, 1) )
ret = -EFAULT;
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1406,7 +1406,8 @@ int __init amd_iommu_init(bool xt)
* since it only supports p2m_ram_rw, and this would
* prevent doing IO to/from mapped grant frames.
*/
- iommu_hap_pt_share = 0;
+ clear_iommu_hap_pt_share();
+
printk(XENLOG_DEBUG "AMD-Vi: Disabled HAP memory map sharing with IOMMU\n");
/* per iommu initialization */
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -49,7 +49,11 @@ int8_t __hwdom_initdata iommu_hwdom_rese
* default until we find a good solution to resolve it.
*/
bool_t __read_mostly iommu_intpost;
-bool_t __read_mostly iommu_hap_pt_share = 1;
+
+#ifndef iommu_hap_pt_share
+bool __read_mostly iommu_hap_pt_share = true;
+#endif
+
bool_t __read_mostly iommu_debug;
bool_t __read_mostly amd_iommu_perdev_intremap = 1;
@@ -102,8 +106,10 @@ static int __init parse_iommu_param(cons
iommu_hwdom_passthrough = val;
else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
iommu_hwdom_strict = val;
+#ifndef iommu_hap_pt_share
else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
iommu_hap_pt_share = val;
+#endif
else
rc = -EINVAL;
@@ -511,7 +517,10 @@ int __init iommu_setup(void)
iommu_enabled = (rc == 0);
}
if ( !iommu_enabled )
+ {
iommu_intremap = 0;
+ clear_iommu_hap_pt_share();
+ }
if ( (force_iommu && !iommu_enabled) ||
(force_intremap && !iommu_intremap) )
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2310,7 +2310,7 @@ static int __init vtd_setup(void)
iommu_intpost = 0;
if ( !vtd_ept_page_compatible(iommu) )
- iommu_hap_pt_share = 0;
+ clear_iommu_hap_pt_share();
ret = iommu_set_interrupt(drhd);
if ( ret )
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -96,9 +96,13 @@ struct xen_sysctl_tbuf_op {
/* The platform supports software paging. */
#define _XEN_SYSCTL_PHYSCAP_shadow 4
#define XEN_SYSCTL_PHYSCAP_shadow (1u<<_XEN_SYSCTL_PHYSCAP_shadow)
+/* The platform supports sharing of HAP page tables with the IOMMU. */
+#define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
+#define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share \
+ (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
/* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */
-#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_shadow
+#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share
struct xen_sysctl_physinfo {
uint32_t threads_per_core;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -55,7 +55,22 @@ static inline bool_t dfn_eq(dfn_t x, dfn
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_verbose, iommu_igfx;
extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
-extern bool_t iommu_hap_pt_share;
+
+#ifdef CONFIG_HVM
+extern bool iommu_hap_pt_share;
+#else
+#define iommu_hap_pt_share false
+#endif
+
+static inline void clear_iommu_hap_pt_share(void)
+{
+#ifndef iommu_hap_pt_share
+ iommu_hap_pt_share = false;
+#elif iommu_hap_pt_share
+ ASSERT_UNREACHABLE();
+#endif
+}
+
extern bool_t iommu_debug;
extern bool_t amd_iommu_perdev_intremap;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 13 September 2019 12:10
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Julien Grall <julien.grall@arm.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; Christian Lindig
> <christian.lindig@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Kevin Tian <kevin.tian@intel.com>;
> Stefano Stabellini <sstabellini@kernel.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; David
> Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; WeiLiu <wl@xen.org>
> Subject: [PATCH v11.1 3/6] sysctl / libxl: report whether IOMMU/HAP page table sharing is supported
>
> This patch defines a new bit reported in the hw_cap field of struct
> xen_sysctl_physinfo to indicate whether the platform supports sharing of
> HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack
> whether the domain needs extra memory to store discrete IOMMU page tables
> or not.
>
> NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not
> supported or the IOMMU is disabled, and defines it to false if
> !CONFIG_HVM.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
...with one observation...
[snip]
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -49,7 +49,11 @@ int8_t __hwdom_initdata iommu_hwdom_rese
> * default until we find a good solution to resolve it.
> */
> bool_t __read_mostly iommu_intpost;
> -bool_t __read_mostly iommu_hap_pt_share = 1;
> +
> +#ifndef iommu_hap_pt_share
> +bool __read_mostly iommu_hap_pt_share = true;
> +#endif
> +
> bool_t __read_mostly iommu_debug;
> bool_t __read_mostly amd_iommu_perdev_intremap = 1;
>
> @@ -102,8 +106,10 @@ static int __init parse_iommu_param(cons
> iommu_hwdom_passthrough = val;
> else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
> iommu_hwdom_strict = val;
> +#ifndef iommu_hap_pt_share
> else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
> iommu_hap_pt_share = val;
> +#endif
> else
> rc = -EINVAL;
>
With this change there will be a command line parse error if 'no-sharept' is passed on the command line to a hypervisor built without CONFIG_HVM. I don't know whether you really want that behaviour, which is why my patch did:
@@ -103,7 +107,14 @@ static int __init parse_iommu_param(const char *s)
else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
iommu_hwdom_strict = val;
else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
+ {
+#ifndef iommu_hap_pt_share
iommu_hap_pt_share = val;
+#else
+ if (val != iommu_hap_pt_share)
+ rc = -EINVAL;
+#endif
+ }
else
rc = -EINVAL;
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 13.09.2019 13:47, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 13 September 2019 12:10
>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
>> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Julien Grall <julien.grall@arm.com>; Andrew
>> Cooper <Andrew.Cooper3@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; Christian Lindig
>> <christian.lindig@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George Dunlap
>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Kevin Tian <kevin.tian@intel.com>;
>> Stefano Stabellini <sstabellini@kernel.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; David
>> Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; WeiLiu <wl@xen.org>
>> Subject: [PATCH v11.1 3/6] sysctl / libxl: report whether IOMMU/HAP page table sharing is supported
>>
>> This patch defines a new bit reported in the hw_cap field of struct
>> xen_sysctl_physinfo to indicate whether the platform supports sharing of
>> HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack
>> whether the domain needs extra memory to store discrete IOMMU page tables
>> or not.
>>
>> NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not
>> supported or the IOMMU is disabled, and defines it to false if
>> !CONFIG_HVM.
>>
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Christian Lindig <christian.lindig@citrix.com>
>
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Thanks.
> ...with one observation...
>
> [snip]
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -49,7 +49,11 @@ int8_t __hwdom_initdata iommu_hwdom_rese
>> * default until we find a good solution to resolve it.
>> */
>> bool_t __read_mostly iommu_intpost;
>> -bool_t __read_mostly iommu_hap_pt_share = 1;
>> +
>> +#ifndef iommu_hap_pt_share
>> +bool __read_mostly iommu_hap_pt_share = true;
>> +#endif
>> +
>> bool_t __read_mostly iommu_debug;
>> bool_t __read_mostly amd_iommu_perdev_intremap = 1;
>>
>> @@ -102,8 +106,10 @@ static int __init parse_iommu_param(cons
>> iommu_hwdom_passthrough = val;
>> else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
>> iommu_hwdom_strict = val;
>> +#ifndef iommu_hap_pt_share
>> else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
>> iommu_hap_pt_share = val;
>> +#endif
>> else
>> rc = -EINVAL;
>>
>
> With this change there will be a command line parse error if 'no-sharept' is passed on the command line to a hypervisor built without CONFIG_HVM. I don't know whether you really want that behaviour, which is why my patch did:
>
> @@ -103,7 +107,14 @@ static int __init parse_iommu_param(const char *s)
> else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
> iommu_hwdom_strict = val;
> else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
> + {
> +#ifndef iommu_hap_pt_share
> iommu_hap_pt_share = val;
> +#else
> + if (val != iommu_hap_pt_share)
> + rc = -EINVAL;
> +#endif
> + }
> else
> rc = -EINVAL;
Yes, I did change this intentionally, as I think the "sharept" option
(in its positive or negative form) should not be specified to Xen
built with this tied to a fixed value.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, Sep 13, 2019 at 01:10:18PM +0200, Jan Beulich wrote:
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -57,7 +57,6 @@ type domain_create_flag =
> | CDF_OOS_OFF
> | CDF_XS_DOMAIN
> | CDF_IOMMU
> -
Stray deletion?
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -96,9 +96,13 @@ struct xen_sysctl_tbuf_op {
> /* The platform supports software paging. */
> #define _XEN_SYSCTL_PHYSCAP_shadow 4
> #define XEN_SYSCTL_PHYSCAP_shadow (1u<<_XEN_SYSCTL_PHYSCAP_shadow)
> +/* The platform supports sharing of HAP page tables with the IOMMU. */
> +#define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
> +#define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share \
> + (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
I would drop the _hap part of this, since I don't think it adds much,
it's not like the iommu page tables can be shared with anything else?
I don't have a strong opinion, and given that the code already makes
extensive use of iommu_hap_pt_share I would be fine with that.
With the removed newline fixed (if applicable):
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 13 September 2019 14:54
> To: Jan Beulich <jbeulich@suse.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; Suravee Suthikulpanit
> <suravee.suthikulpanit@amd.com>; Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; Christian Lindig
> <christian.lindig@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; David Scott
> <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; WeiLiu <wl@xen.org>
> Subject: Re: [PATCH v11.1 3/6] sysctl / libxl: report whether IOMMU/HAP page table sharing is
> supported
>
> On Fri, Sep 13, 2019 at 01:10:18PM +0200, Jan Beulich wrote:
> > --- a/tools/ocaml/libs/xc/xenctrl.mli
> > +++ b/tools/ocaml/libs/xc/xenctrl.mli
> > @@ -57,7 +57,6 @@ type domain_create_flag =
> > | CDF_OOS_OFF
> > | CDF_XS_DOMAIN
> > | CDF_IOMMU
> > -
>
> Stray deletion?
Yes, it is. It gets fixed up by a later patch though.
>
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -96,9 +96,13 @@ struct xen_sysctl_tbuf_op {
> > /* The platform supports software paging. */
> > #define _XEN_SYSCTL_PHYSCAP_shadow 4
> > #define XEN_SYSCTL_PHYSCAP_shadow (1u<<_XEN_SYSCTL_PHYSCAP_shadow)
> > +/* The platform supports sharing of HAP page tables with the IOMMU. */
> > +#define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
> > +#define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share \
> > + (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
>
> I would drop the _hap part of this, since I don't think it adds much,
> it's not like the iommu page tables can be shared with anything else?
>
> I don't have a strong opinion, and given that the code already makes
> extensive use of iommu_hap_pt_share I would be fine with that.
>
> With the removed newline fixed (if applicable):
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks,
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Jan, On 9/13/19 12:10 PM, Jan Beulich wrote: > This patch defines a new bit reported in the hw_cap field of struct > xen_sysctl_physinfo to indicate whether the platform supports sharing of > HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack > whether the domain needs extra memory to store discrete IOMMU page tables > or not. > > NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not > supported or the IOMMU is disabled, and defines it to false if > !CONFIG_HVM. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Acked-by: Christian Lindig <christian.lindig@citrix.com> For the common code: Acked-by: Julien Grall <julien.grall@arm.com> Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich [mailto:jbeulich@suse.com] > Sent: Friday, September 13, 2019 7:10 PM > > This patch defines a new bit reported in the hw_cap field of struct > xen_sysctl_physinfo to indicate whether the platform supports sharing of > HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack > whether the domain needs extra memory to store discrete IOMMU page > tables > or not. > > NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not > supported or the IOMMU is disabled, and defines it to false if > !CONFIG_HVM. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Acked-by: Christian Lindig <christian.lindig@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, 13 Sep 2019 at 11:58, Paul Durrant <paul.durrant@citrix.com> wrote: > > This patch defines a new bit reported in the hw_cap field of struct > xen_sysctl_physinfo to indicate whether the platform supports sharing of > HAP page tables (i.e. the P2M) with the IOMMU. This informs the toolstack > whether the domain needs extra memory to store discrete IOMMU page tables > or not. > > NOTE: This patch makes sure iommu_hap_pt_shared is clear if HAP is not > supported or the IOMMU is disabled, and defines it to false if > !CONFIG_HVM. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Acked-by: Christian Lindig <christian.lindig@citrix.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Christian Lindig <christian.lindig@citrix.com> > Cc: David Scott <dave@recoil.org> > Cc: "Roger Pau Monné" <roger.pau@citrix.com> > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Cc: Kevin Tian <kevin.tian@intel.com> > > v11: > - Fix abi-check breakage > > v10: > - Set flag in common code (which means clearing iommu_hap_pt_share if > HAP cannot be enabled or is configured out). > > v9: > - New in v9 > --- > tools/libxl/libxl.c | 2 ++ > tools/libxl/libxl.h | 7 +++++++ > tools/libxl/libxl_types.idl | 1 + Hmm... I thought I acked this before, but that could be an old version that I acked. Acked-by: Wei Liu <wl@xen.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2026 Red Hat, Inc.