[PATCH v6 11/11] xen, common: do not implicitly permit access to mapped I/O memory

[Arianna Avanzini <avanzini.arianna@gmail.com>]

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).

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
---
 tools/libxl/libxl_create.c | 17 +++++++++++++++++
 tools/libxl/libxl_pci.c    | 24 ++++++++++++++++++------
 xen/common/domctl.c        | 37 ++++++++++---------------------------
 3 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index cdf03cd..924c2e9 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1151,6 +1151,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..33dc3ce 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -867,7 +867,10 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
         }
         if ( rc )
             return ERROR_FAIL;
-        break;
+        /*
+         * fall through and let I/O memory permissions for PCI devices be given
+         * also to HVM domains
+         */
     case LIBXL_DOMAIN_TYPE_PV:
     {
         char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
@@ -886,14 +889,14 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
                 continue;
             size = end - start + 1;
             if (start) {
-                if (flags & PCI_BAR_IO) {
+                if ((flags & PCI_BAR_IO) && !hvm) {
                     rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
                     if (rc < 0) {
                         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
                         fclose(f);
                         return ERROR_FAIL;
                     }
-                } else {
+                } else if (!(flags & PCI_BAR_IO)) {
                     rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
                                                     (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
                     if (rc < 0) {
@@ -905,6 +908,9 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
             }
         }
         fclose(f);
+        if (hvm)
+            break;
+
         sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
                                    pcidev->bus, pcidev->dev, pcidev->func);
         f = fopen(sysfs_path, "r");
@@ -1203,7 +1209,10 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
             rc = ERROR_FAIL;
             goto out_fail;
         }
-        break;
+        /*
+         * fall through and let I/O memory permissions for PCI devices be given
+         * also to HVM domains
+         */
     case LIBXL_DOMAIN_TYPE_PV:
     {
         char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
@@ -1222,11 +1231,11 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
                 continue;
             size = end - start + 1;
             if (start) {
-                if (flags & PCI_BAR_IO) {
+                if ((flags & PCI_BAR_IO) && !hvm) {
                     rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 0);
                     if (rc < 0)
                         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_domain_ioport_permission error 0x%x/0x%x", start, size);
-                } else {
+                } else if (!(flags & PCI_BAR_IO)) {
                     rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
                                                     (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 0);
                     if (rc < 0)
@@ -1236,6 +1245,9 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
         }
         fclose(f);
 skip1:
+        if (hvm)
+            break;
+
         sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
                                    pcidev->bus, pcidev->dev, pcidev->func);
         f = fopen(sysfs_path, "r");
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index c550c9f..18b975d 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -838,7 +838,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             return ret;
 
         ret = -EPERM;
-        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+        if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
+             !iomem_access_permitted(d, mfn, mfn_end) )
             break;
 
         ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
@@ -851,41 +852,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, gfn_end, 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, gfn_end, 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 tmp_rc = 0;
-
             printk(XENLOG_G_INFO
                    "memory_map: remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
                    d->domain_id, gfn, mfn, nr_mfns);
 
-            tmp_rc = unmap_mmio_regions(d, gfn, gfn_end, mfn);
-            ret = iomem_deny_access(d, mfn, mfn_end);
-            if ( !ret )
-                ret = tmp_rc;
+            ret = unmap_mmio_regions(d, gfn, gfn_end, mfn);
             if ( ret && is_hardware_domain(current->domain) )
                 printk(XENLOG_ERR
-                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
-                       ret, tmp_rc ? "removing" : "denying", d->domain_id,
-                       mfn, mfn_end);
+                       "memory_map: error %ld removing dom%d mapping "
+                       "to [%lx,%lx]\n", ret, d->domain_id, mfn, mfn_end);
         }
         /* Do this unconditionally to cover errors on above failure paths. */
         memory_type_changed(d);
-- 
1.9.1