|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v7 10/10] xen/common: do not implicitly permit access to mapped I/O memory
Currently, the XEN_DOMCTL_memory_mapping hypercall implicitly grants
to a domain access permission to the I/O memory areas mapped in its
guest address space. This conflicts with the presence of a specific
hypercall (XEN_DOMCTL_iomem_permission) used to grant such a permission
to a domain.
This commit attempts to separate the functions of the two hypercalls by
having only the latter be able to permit I/O memory access to a domain,
and the former just performing the mapping after a permissions check.
This commit also attempts to change existing code to be sure to grant
access permission to PCI-related I/O memory ranges (for passthrough of
PCI devices specified in the domain's configuration) and to VGA-related
memory ranges (for VGA passthrough, if gfx_passthru = 1 in the domain
configuration). As of the latter, VGA needs some extra memory ranges to
be mapped with respect to PCI; the access to those memory ranges was
previously given implicitly when calling xc_domain_memory_mapping, while
now it must be explicitly granted.
Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>
Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>
---
v7:
- Let iomem_permission check if the calling domain is allowed to access
memory ranges to be mapped to a domain. Remove such a check from the
memory_mapping hypercall.
- Do not handle I/O ports and I/O memory differently when allowing
to a domain to access a PCI device.
- Change the construct used by libxl during PCI-related initialization
from a switch to an if to better suit the new execution flow.
---
tools/libxl/libxl_create.c | 17 ++++++++++++++++
tools/libxl/libxl_pci.c | 26 +++++++++--------------
xen/common/domctl.c | 51 +++++++++++++++++-----------------------------
3 files changed, 46 insertions(+), 48 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4de0fb2..e544bbf 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1149,6 +1149,23 @@ static void domcreate_launch_dm(libxl__egc *egc,
libxl__multidev *multidev,
libxl__spawn_stub_dm(egc, &dcs->dmss);
else
libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+
+ /*
+ * If VGA passthru is enabled by domain config, be sure that the
+ * domain can access VGA-related iomem regions.
+ */
+ if (d_config->b_info.u.hvm.gfx_passthru.val) {
+ uint64_t vga_iomem_start = 0xa0000 >> XC_PAGE_SHIFT;
+ ret = xc_domain_iomem_permission(CTX->xch, domid,
+ vga_iomem_start, 0x20, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to give dom%d access to iomem range "
+ "%"PRIx64"-%"PRIx64" for VGA passthru",
+ domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+ goto error_out;
+ }
+ }
return;
}
case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 44d0453..032e981 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -846,10 +846,13 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t
domid,
static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev,
int starting)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int rc, hvm = 0;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_INVALID)
+ return ERROR_FAIL;
+
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0) {
@@ -867,8 +870,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, i
}
if ( rc )
return ERROR_FAIL;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ }
{
char *sysfs_path = libxl__sprintf(gc,
SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev,
pcidev->func);
@@ -937,10 +939,6 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, i
return ERROR_FAIL;
}
}
- break;
- }
- case LIBXL_DOMAIN_TYPE_INVALID:
- return ERROR_FAIL;
}
out:
if (!libxl_is_stubdom(ctx, domid, NULL)) {
@@ -1166,6 +1164,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
{
libxl_ctx *ctx = libxl__gc_owner(gc);
libxl_device_pci *assigned;
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int hvm = 0, rc, num;
int stubdomid = 0;
@@ -1181,8 +1180,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
}
rc = ERROR_FAIL;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0)
@@ -1203,8 +1201,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
rc = ERROR_FAIL;
goto out_fail;
}
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ } else if (type != LIBXL_DOMAIN_TYPE_PV)
+ abort();
{
char *sysfs_path = libxl__sprintf(gc,
SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev,
pcidev->func);
@@ -1254,10 +1252,6 @@ skip1:
}
}
fclose(f);
- break;
- }
- default:
- abort();
}
out:
/* don't do multiple resets while some functions are still passed through
*/
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 866338b..8ee72eb 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -803,18 +803,22 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
{
unsigned long mfn = op->u.iomem_permission.first_mfn;
unsigned long nr_mfns = op->u.iomem_permission.nr_mfns;
+ unsigned long mfn_end = mfn + nr_mfns - 1;
int allow = op->u.iomem_permission.allow_access;
ret = -EINVAL;
- if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */
+ if ( mfn_end < mfn ) /* wrap? */
break;
- if ( xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, allow) )
- ret = -EPERM;
- else if ( allow )
- ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
+ ret = -EPERM;
+ if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
+ xsm_iomem_permission(XSM_HOOK, d, mfn, mfn_end, allow) )
+ break;
+
+ if ( allow )
+ ret = iomem_permit_access(d, mfn, mfn_end);
else
- ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+ ret = iomem_deny_access(d, mfn, mfn_end);
#ifdef CONFIG_X86
if ( !ret )
memory_type_changed(d);
@@ -838,7 +842,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
break;
ret = -EPERM;
- if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+ if ( !iomem_access_permitted(d, mfn, mfn_end) )
break;
ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
@@ -851,40 +855,23 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = iomem_permit_access(d, mfn, mfn_end);
- if ( !ret )
- {
- ret = map_mmio_regions(d, gfn, nr_mfns, _mfn(mfn));
- if ( ret )
- {
- printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
- d->domain_id, gfn, mfn, ret);
- if ( iomem_deny_access(d, mfn, mfn_end) &&
- is_hardware_domain(current->domain) )
- printk(XENLOG_ERR
- "memory_map: failed to deny dom%d access to
[%lx,%lx]\n",
- d->domain_id, mfn, mfn_end);
- }
- }
+ ret = map_mmio_regions(d, gfn, nr_mfns, _mfn(mfn));
+ if ( ret )
+ printk(XENLOG_G_WARNING
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
+ d->domain_id, gfn, mfn, ret);
}
else
{
- int rc;
-
printk(XENLOG_G_INFO
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- rc = unmap_mmio_regions(d, gfn, nr_mfns, _mfn(mfn));
- ret = iomem_deny_access(d, mfn, mfn_end);
- if ( !ret )
- ret = rc;
+ ret = unmap_mmio_regions(d, gfn, nr_mfns, _mfn(mfn));
if ( ret && is_hardware_domain(current->domain) )
printk(XENLOG_ERR
- "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, rc ? "removing" : "denying", d->domain_id,
- mfn, mfn_end);
+ "memory_map: error %ld removing dom%d access to
[%lx,%lx]\n",
+ ret, d->domain_id, mfn, mfn_end);
}
#ifdef CONFIG_X86
/* Do this unconditionally to cover errors on above failure paths. */
--
1.9.2
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |