xen: RMRR fix

From: Chen, Tiejun
Sent: Monday, December 01, 2014 5:24 PM

This should be based on a new parameter globally, 'pci_rdmforce'.

pci_rdmforce = 1 => Of course this should be 0 by default.

'1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM.

But we can override that by one specific pci device:

pci = ['AA:BB.CC,rdmforce=0/1]

But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach.

So this domctl provides an approach to control how to populate reserved device memory by tools.

Note we always post a message to user about this once we owns RMRR.

Signed-off-by Tiejun Chen <[email protected]>

---
 docs/man/xl.cfg.pod.5              |  6 +++++
 docs/misc/vtd.txt                  | 15 ++++++++++++
 tools/libxc/include/xenctrl.h      |  6 +++++
 tools/libxc/xc_domain.c            | 28 +++++++++++++++++++++++
 tools/libxl/libxl_create.c         |  3 +++
 tools/libxl/libxl_dm.c             | 47
++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h       |  4 ++++
 tools/libxl/libxl_types.idl        |  2 ++
 tools/libxl/libxlu_pci.c           |  2 ++
 tools/libxl/xl_cmdimpl.c           | 10 ++++++++
 xen/drivers/passthrough/pci.c      | 39
+++++++++++++++++++++++++++++++
 xen/drivers/passthrough/vtd/dmar.c |  8 +++++++
 xen/include/asm-x86/hvm/domain.h   |  4 ++++
 xen/include/public/domctl.h        | 21 +++++++++++++++++
 xen/xsm/flask/hooks.c              |  1 +
 15 files changed, 196 insertions(+)

docs/man/xl.cfg.pod.5 +6 -0

index 622ea53..9adc41e 100644
@@ -645,6 +645,12 @@ dom0 without confirmation.  Please use with care.
 D0-D3hot power management states for the PCI device. False (0) by
 default.
 
+=item B<rdmforce=BOOLEAN>
+
+(HVM/x86 only) Specifies that the VM would force to check and try to
+reserve all reserved device memory, like RMRR, associated to the PCI
+device. False (0) by default.
+
 =back
 
 =back

docs/misc/vtd.txt +6 -0

index 9af0e99..23544d5 100644
@@ -111,6 +111,21 @@ in the config file:
 To override for a specific device:
 	pci = [ '01:00.0,msitranslate=0', '03:00.0' ]
 
+RDM, 'reserved device memory', for PCI Device Passthrough
+---------------------------------------------------------
+
+The BIOS controls some devices in terms of some reginos of memory used for
+these devices. This kind of region should be reserved before creating a VM
+to make sure they are not occupied by RAM/MMIO to conflict, and also we

can

docs/misc/vtd.txt +9 -0

+create necessary IOMMU table successfully.
+
+To enable this globally, add "pci_rdmforce" in the config file:
+
+	pci_rdmforce = 1         (default is 0)
+
+Or just enable for a specific device:
+	pci = [ '01:00.0,rdmforce=1', '03:00.0' ]
+
 
 Caveat on Conventional PCI Device Passthrough
 ---------------------------------------------

tools/libxc/include/xenctrl.h +6 -0

index 0ad8b8d..84012fe 100644
@@ -2038,6 +2038,12 @@ int xc_assign_device(xc_interface *xch,
                      uint32_t domid,
                      uint32_t machine_bdf);
 
+int xc_domain_device_setrdm(xc_interface *xch,
+                            uint32_t domid,
+                            uint32_t num_pcidevs,
+                            uint32_t pci_rdmforce,
+                            struct xen_guest_pcidev_info *pcidevs);
+
 int xc_get_device_group(xc_interface *xch,
                      uint32_t domid,
                      uint32_t machine_bdf,

tools/libxc/xc_domain.c +28 -0

index b864872..7fd43e9 100644
@@ -1633,6 +1633,34 @@ int xc_assign_device(
     return do_domctl(xch, &domctl);
 }
 
+int xc_domain_device_setrdm(xc_interface *xch,
+                            uint32_t domid,
+                            uint32_t num_pcidevs,
+                            uint32_t pci_rdmforce,
+                            struct xen_guest_pcidev_info *pcidevs)
+{
+    int ret;
+    DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BOUNCE(pcidevs,
+
 num_pcidevs*sizeof(xen_guest_pcidev_info_t),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( xc_hypercall_bounce_pre(xch, pcidevs) )
+        return -1;
+
+    domctl.cmd = XEN_DOMCTL_set_rdm;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_rdm.flags = pci_rdmforce;
+    domctl.u.set_rdm.num_pcidevs = num_pcidevs;
+    set_xen_guest_handle(domctl.u.set_rdm.pcidevs, pcidevs);
+
+    ret = do_domctl(xch, &domctl);
+
+    xc_hypercall_bounce_post(xch, pcidevs);
+
+    return ret;
+}
+
 int xc_get_device_group(
     xc_interface *xch,

     uint32_t domid,

tools/libxl/libxl_create.c +3 -0

index 1198225..c615686 100644
@@ -862,6 +862,9 @@ static void initiate_domain_create(libxl__egc *egc,
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
     if (ret) goto error_out;
 
+    ret = libxl__domain_device_setrdm(gc, d_config, domid);
+    if (ret) goto error_out;
+
     if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
         LOG(ERROR, "Invalid scheduling parameters\n");
         ret = ERROR_INVAL;

tools/libxl/libxl_dm.c +26 -0

index 3e191c3..e50587d 100644
@@ -90,6 +90,53 @@ const char *libxl__domain_device_model(libxl__gc *gc,
     return dm;
 }
 
+int libxl__domain_device_setrdm(libxl__gc *gc,
+                                libxl_domain_config *d_config,
+                                uint32_t dm_domid)
+{
+    int i, ret;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    struct xen_guest_pcidev_info *pcidevs = NULL;
+    uint32_t rdmforce = 0;
+
+    if ( d_config->num_pcidevs )
+    {
+        pcidevs =
 malloc(d_config->num_pcidevs*sizeof(xen_guest_pcidev_info_t));
+        if ( pcidevs )
+        {
+            for (i = 0; i < d_config->num_pcidevs; i++)
+            {
+                pcidevs[i].devfn = PCI_DEVFN(d_config->pcidevs[i].dev,
+
 d_config->pcidevs[i].func);
+                pcidevs[i].bus = d_config->pcidevs[i].bus;
+                pcidevs[i].seg = d_config->pcidevs[i].domain;
+                pcidevs[i].flags = d_config->pcidevs[i].rdmforce &
+                                   PCI_DEV_RDM_CHECK;
+            }
+        }
+        else
+        {

tools/libxl/libxl_dm.c +11 -0

+
+    ret = xc_domain_device_setrdm(ctx->xch, dm_domid,
+                                  (uint32_t)d_config->num_pcidevs,
+                                  rdmforce,
+                                  pcidevs);
+    if ( d_config->num_pcidevs )
+        free(pcidevs);
+
+    return ret;
+}
+
 const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config

*guest_config)

 {
     const libxl_vnc_info *vnc = NULL;

tools/libxl/libxl_internal.h +4 -0

index a38f695..be397a6 100644
@@ -1477,6 +1477,10 @@ _hidden int libxl__need_xenpv_qemu(libxl__gc
 *gc,
         int nr_disks, libxl_device_disk *disks,
         int nr_channels, libxl_device_channel *channels);
 
+_hidden int libxl__domain_device_setrdm(libxl__gc *gc,
+                                        libxl_domain_config *info,
+                                        uint32_t domid);
+
 /*
  * This function will cause the whole libxl process to hang

* if the device model does not respond. It is deprecated.

tools/libxl/libxl_types.idl +0 -0

index f7fc695..0076a32 100644
@@ -398,6 +398,7 @@ libxl_domain_build_info =

Struct("domain_build_info",[

     ("kernel",           string),
     ("cmdline",          string),
     ("ramdisk",          string),
+    ("rdmforce",         libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",

libxl_bios_type),

tools/libxl/libxl_types.idl +1 -0

@@ -518,6 +519,7 @@ libxl_device_pci = Struct("device_pci", [
     ("power_mgmt", bool),
     ("permissive", bool),
     ("seize", bool),
+    ("rdmforce", bool),
     ])
 
 libxl_device_vtpm = Struct("device_vtpm", [

tools/libxl/libxlu_pci.c +0 -0

index 26fb143..989eac8 100644
@@ -143,6 +143,8 @@ int xlu_pci_parse_bdf(XLU_Config *cfg,

libxl_device_pci *pcidev, const char *str

                     pcidev->permissive = atoi(tok);
                 }else if ( !strcmp(optkey, "seize") ) {
                     pcidev->seize = atoi(tok);

tools/libxl/libxlu_pci.c +2 -0

+                }else if ( !strcmp(optkey, "rdmforce") ) {
+                    pcidev->rdmforce = atoi(tok);
                 }else{
                     XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s",

optkey);
                 }

tools/libxl/xl_cmdimpl.c +1 -0

index 0e754e7..9c23733 100644
@@ -919,6 +919,7 @@ static void parse_config_data(const char
 *config_source,
     int pci_msitranslate = 0;
     int pci_permissive = 0;
     int pci_seize = 0;
+    int pci_rdmforce = 0;
     int i, e;
 

     libxl_domain_create_info *c_info = &d_config->c_info;

tools/libxl/xl_cmdimpl.c +9 -0

@@ -1699,6 +1700,9 @@ skip_vfb:
     if (!xlu_cfg_get_long (config, "pci_seize", &l, 0))
         pci_seize = l;
 
+    if (!xlu_cfg_get_long (config, "pci_rdmforce", &l, 0))
+        pci_rdmforce = l;
+
     /* To be reworked (automatically enabled) once the auto ballooning
      * after guest starts is done (with PCI devices passed in). */
     if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {

@@ -1719,6 +1723,7 @@ skip_vfb:
             pcidev->power_mgmt = pci_power_mgmt;
             pcidev->permissive = pci_permissive;
             pcidev->seize = pci_seize;
+            pcidev->rdmforce = pci_rdmforce;
             if (!xlu_pci_parse_bdf(config, pcidev, buf))
                 d_config->num_pcidevs++;
         }

@@ -1726,6 +1731,11 @@ skip_vfb:
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
+    if ((c_info->type == LIBXL_DOMAIN_TYPE_HVM) && pci_rdmforce)
+        libxl_defbool_set(&b_info->rdmforce, true);
+    else
+        libxl_defbool_set(&b_info->rdmforce, false);
+
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
     case 0:
         {

xen/drivers/passthrough/pci.c +39 -0

index 78c6977..ae924ad 100644
@@ -34,6 +34,7 @@
 #include <xen/tasklet.h>
 #include <xsm/xsm.h>
 #include <asm/msi.h>
+#include <xen/stdbool.h>
 
 struct pci_seg {
     struct list_head alldevs_list;

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
         }
         break;
 
+    case XEN_DOMCTL_set_rdm:
+    {
+        struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+        struct xen_guest_pcidev_info *pcidevs = NULL;
+        struct domain *d = rcu_lock_domain_by_any_id(domctl->domain);
+
+        if ( d == NULL )
+            return -ESRCH;
+
+        d->arch.hvm_domain.pci_force =
+                            xdsr->flags & PCI_DEV_RDM_CHECK ?
 true : false;
+        d->arch.hvm_domain.num_pcidevs = xdsr->num_pcidevs;
+        d->arch.hvm_domain.pcidevs = NULL;
+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);
+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;
+            }
+
+            if ( copy_from_guest(pcidevs, xdsr->pcidevs,
+
 xdsr->num_pcidevs*sizeof(*pcidevs)) )
+            {
+                xfree(pcidevs);
+                rcu_unlock_domain(d);
+                return -EFAULT;
+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;
+        rcu_unlock_domain(d);
+    }
+        break;
+
     case XEN_DOMCTL_assign_device:

         if ( unlikely(d->is_dying) )
         {

xen/drivers/passthrough/vtd/dmar.c +8 -0

index 1152c3a..5e41e7a 100644
@@ -674,6 +674,14 @@ acpi_parse_one_rmrr(struct acpi_dmar_header
 *header)
                         "  RMRR region: base_addr %"PRIx64
                         " end_address %"PRIx64"\n",
                         rmrru->base_address, rmrru->end_address);
+            /*
+             * TODO: we may provide a precise paramter just to reserve
+             * RMRR range specific to one device.
+             */
+            dprintk(XENLOG_WARNING VTDPREFIX,
+                    "So please set pci_rdmforce to reserve these ranges"
+                    " if you need such a device in hotplug case.\n");
+
             acpi_register_rmrr_unit(rmrru);
         }

     }

xen/include/asm-x86/hvm/domain.h +4 -0

index 2757c7f..38530e5 100644
@@ -90,6 +90,10 @@ struct hvm_domain {
     /* Cached CF8 for guest PCI config cycles */
     uint32_t                pci_cf8;
 
+    bool_t                  pci_force;
+    uint32_t                num_pcidevs;
+    struct xen_guest_pcidev_info      *pcidevs;
+
     struct pl_time         pl_time;
 
     struct hvm_io_handler *io_handler;

xen/include/public/domctl.h +19 -0

index 57e2ed7..ba8970d 100644
@@ -508,6 +508,25 @@ struct xen_domctl_get_device_group {
 typedef struct xen_domctl_get_device_group
 xen_domctl_get_device_group_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_get_device_group_t);
 
+/* Currently just one bit to indicate force to check Reserved Device Memory.
 */
+#define PCI_DEV_RDM_CHECK   0x1
+struct xen_guest_pcidev_info {
+    uint16_t    seg;
+    uint8_t     bus;
+    uint8_t     devfn;
+    uint32_t    flags;
+};
+typedef struct xen_guest_pcidev_info xen_guest_pcidev_info_t;
+DEFINE_XEN_GUEST_HANDLE(xen_guest_pcidev_info_t);
+/* Control whether/how we check and reserve device memory. */
+struct xen_domctl_set_rdm {
+    uint32_t    flags;
+    uint32_t    num_pcidevs;
+    XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
+};
+typedef struct xen_domctl_set_rdm xen_domctl_set_rdm_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_rdm_t);
+
 /* Pass-through interrupts: bind real irq -> hvm devfn. */

/* XEN_DOMCTL_bind_pt_irq */
/* XEN_DOMCTL_unbind_pt_irq */

xen/include/public/domctl.h +2 -0

@@ -1070,6 +1089,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_arm_configure_domain          76
+#define XEN_DOMCTL_set_rdm                       77
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002

@@ -1135,6 +1155,7 @@ struct xen_domctl {
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
+        struct xen_domctl_set_rdm           set_rdm;
         uint8_t                             pad[128];
     } u;
 };

xen/xsm/flask/hooks.c +1 -1

index d48463f..5a760e2 100644
@@ -592,6 +592,7 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_test_assign_device:
     case XEN_DOMCTL_assign_device:
     case XEN_DOMCTL_deassign_device:
+    case XEN_DOMCTL_set_rdm:
 #endif
         return 0;
 
--

1.9.1

Quoted 16 lines

From: Chen, Tiejun
Sent: Monday, December 01, 2014 5:24 PM

This should be based on a new parameter globally, 'pci_rdmforce'.

pci_rdmforce = 1 => Of course this should be 0 by default.

'1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM.

But we can override that by one specific pci device:

pci = ['AA:BB.CC,rdmforce=0/1]

Quoted 4 lines

But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach.

since no one requires it, why bother adding it? better to just keep global option.

Quoted 10 lines

So this domctl provides an approach to control how to populate reserved device memory by tools.

Note we always post a message to user about this once we owns RMRR.

Signed-off-by Tiejun Chen <[email protected]>

---

Quoted 38 lines

tools/libxl/libxl_dm.c +30 -0

@@ -90,6 +90,53 @@ const char *libxl__domain_device_model(libxl__gc *gc,
      return dm;
  }
 
+int libxl__domain_device_setrdm(libxl__gc *gc,
+                                libxl_domain_config *d_config,
+                                uint32_t dm_domid)
+{
+    int i, ret;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    struct xen_guest_pcidev_info *pcidevs = NULL;
+    uint32_t rdmforce = 0;
+
+    if ( d_config->num_pcidevs )
+    {
+        pcidevs =
 malloc(d_config->num_pcidevs*sizeof(xen_guest_pcidev_info_t));
+        if ( pcidevs )
+        {
+            for (i = 0; i < d_config->num_pcidevs; i++)
+            {
+                pcidevs[i].devfn = PCI_DEVFN(d_config->pcidevs[i].dev,
+
 d_config->pcidevs[i].func);
+                pcidevs[i].bus = d_config->pcidevs[i].bus;
+                pcidevs[i].seg = d_config->pcidevs[i].domain;
+                pcidevs[i].flags = d_config->pcidevs[i].rdmforce &
+                                   PCI_DEV_RDM_CHECK;
+            }
+        }
+        else
+        {
+            LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                               "Can't allocate for pcidevs.");
+            return -1;
+        }

tools/libxl/libxl_dm.c const char *libxl__domain_device_model(libxl__gc *gc, · line 128

Quoted 6 lines

tools/libxl/libxl_dm.c +6 -0

+    }
+    rdmforce = libxl_defbool_val(d_config->b_info.rdmforce) ? 1 : 0;
+
+    /* Nothing to do. */
+    if ( !rdmforce && !d_config->num_pcidevs )
+        return 0;

move check before creating pcidevs.

This should be based on a new parameter globally, 'pci_rdmforce'.

pci_rdmforce = 1 => Of course this should be 0 by default.

'1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM.

But we can override that by one specific pci device:

pci = ['AA:BB.CC,rdmforce=0/1]

But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach.

So this domctl provides an approach to control how to populate reserved device memory by tools.

Note we always post a message to user about this once we owns RMRR.

Signed-off-by Tiejun Chen <[email protected]>

---
 docs/man/xl.cfg.pod.5              |  6 +++++
 docs/misc/vtd.txt                  | 15 ++++++++++++
 tools/libxc/include/xenctrl.h      |  6 +++++
 tools/libxc/xc_domain.c            | 28 +++++++++++++++++++++++
 tools/libxl/libxl_create.c         |  3 +++
 tools/libxl/libxl_dm.c             | 47 ++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h       |  4 ++++
 tools/libxl/libxl_types.idl        |  2 ++
 tools/libxl/libxlu_pci.c           |  2 ++
 tools/libxl/xl_cmdimpl.c           | 10 ++++++++

 xen/drivers/passthrough/pci.c      | 39 +++++++++++++++++++++++++++++++
 xen/drivers/passthrough/vtd/dmar.c |  8 +++++++
 xen/include/asm-x86/hvm/domain.h   |  4 ++++

 xen/include/public/domctl.h        | 21 +++++++++++++++++
 xen/xsm/flask/hooks.c              |  1 +
 15 files changed, 196 insertions(+)

docs/man/xl.cfg.pod.5 +0 -0

index 622ea53..9adc41e 100644

docs/man/xl.cfg.pod.5 +1 -0

+
 =back
 
 =back

docs/misc/vtd.txt +0 -0

index 9af0e99..23544d5 100644

docs/misc/vtd.txt +4 -0

+
+Or just enable for a specific device:
+	pci = [ '01:00.0,rdmforce=1', '03:00.0' ]
+
 
 Caveat on Conventional PCI Device Passthrough
 ---------------------------------------------

tools/libxc/include/xenctrl.h +6 -0

index 0ad8b8d..84012fe 100644
@@ -2038,6 +2038,12 @@ int xc_assign_device(xc_interface *xch,
                      uint32_t domid,
                      uint32_t machine_bdf);
 
+int xc_domain_device_setrdm(xc_interface *xch,
+                            uint32_t domid,
+                            uint32_t num_pcidevs,
+                            uint32_t pci_rdmforce,
+                            struct xen_guest_pcidev_info *pcidevs);
+
 int xc_get_device_group(xc_interface *xch,
                      uint32_t domid,
                      uint32_t machine_bdf,

tools/libxc/xc_domain.c +28 -0

index b864872..7fd43e9 100644
@@ -1633,6 +1633,34 @@ int xc_assign_device(
     return do_domctl(xch, &domctl);
 }
 
+int xc_domain_device_setrdm(xc_interface *xch,
+                            uint32_t domid,
+                            uint32_t num_pcidevs,
+                            uint32_t pci_rdmforce,
+                            struct xen_guest_pcidev_info *pcidevs)
+{
+    int ret;
+    DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BOUNCE(pcidevs,
+                             num_pcidevs*sizeof(xen_guest_pcidev_info_t),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( xc_hypercall_bounce_pre(xch, pcidevs) )
+        return -1;
+
+    domctl.cmd = XEN_DOMCTL_set_rdm;
+    domctl.domain = (domid_t)domid;
+    domctl.u.set_rdm.flags = pci_rdmforce;
+    domctl.u.set_rdm.num_pcidevs = num_pcidevs;
+    set_xen_guest_handle(domctl.u.set_rdm.pcidevs, pcidevs);
+
+    ret = do_domctl(xch, &domctl);
+
+    xc_hypercall_bounce_post(xch, pcidevs);
+
+    return ret;
+}
+
 int xc_get_device_group(
     xc_interface *xch,
     uint32_t domid,

tools/libxl/libxl_create.c +3 -0

index 1198225..c615686 100644
@@ -862,6 +862,9 @@ static void initiate_domain_create(libxl__egc *egc,
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
     if (ret) goto error_out;
 
+    ret = libxl__domain_device_setrdm(gc, d_config, domid);
+    if (ret) goto error_out;
+
     if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
         LOG(ERROR, "Invalid scheduling parameters\n");
         ret = ERROR_INVAL;

tools/libxl/libxl_dm.c +47 -0

index 3e191c3..e50587d 100644
@@ -90,6 +90,53 @@ const char *libxl__domain_device_model(libxl__gc *gc,
     return dm;
 }
 
+int libxl__domain_device_setrdm(libxl__gc *gc,
+                                libxl_domain_config *d_config,
+                                uint32_t dm_domid)
+{
+    int i, ret;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    struct xen_guest_pcidev_info *pcidevs = NULL;
+    uint32_t rdmforce = 0;
+
+    if ( d_config->num_pcidevs )
+    {
+        pcidevs = malloc(d_config->num_pcidevs*sizeof(xen_guest_pcidev_info_t));
+        if ( pcidevs )
+        {
+            for (i = 0; i < d_config->num_pcidevs; i++)
+            {
+                pcidevs[i].devfn = PCI_DEVFN(d_config->pcidevs[i].dev,
+                                             d_config->pcidevs[i].func);
+                pcidevs[i].bus = d_config->pcidevs[i].bus;
+                pcidevs[i].seg = d_config->pcidevs[i].domain;
+                pcidevs[i].flags = d_config->pcidevs[i].rdmforce &
+                                   PCI_DEV_RDM_CHECK;
+            }
+        }
+        else
+        {
+            LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                               "Can't allocate for pcidevs.");
+            return -1;
+        }
+    }
+    rdmforce = libxl_defbool_val(d_config->b_info.rdmforce) ? 1 : 0;
+
+    /* Nothing to do. */
+    if ( !rdmforce && !d_config->num_pcidevs )
+        return 0;
+
+    ret = xc_domain_device_setrdm(ctx->xch, dm_domid,
+                                  (uint32_t)d_config->num_pcidevs,
+                                  rdmforce,
+                                  pcidevs);
+    if ( d_config->num_pcidevs )
+        free(pcidevs);
+
+    return ret;
+}
+
 const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config *guest_config)
 {
     const libxl_vnc_info *vnc = NULL;

tools/libxl/libxl_internal.h +4 -0

index a38f695..be397a6 100644
@@ -1477,6 +1477,10 @@ _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_disks, libxl_device_disk *disks,
         int nr_channels, libxl_device_channel *channels);
 
+_hidden int libxl__domain_device_setrdm(libxl__gc *gc,
+                                        libxl_domain_config *info,
+                                        uint32_t domid);
+
 /*
  * This function will cause the whole libxl process to hang
  * if the device model does not respond.  It is deprecated.

tools/libxl/libxl_types.idl +2 -0

index f7fc695..0076a32 100644
@@ -398,6 +398,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("kernel",           string),
     ("cmdline",          string),
     ("ramdisk",          string),
+    ("rdmforce",         libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),

@@ -518,6 +519,7 @@ libxl_device_pci = Struct("device_pci", [
     ("power_mgmt", bool),
     ("permissive", bool),
     ("seize", bool),
+    ("rdmforce", bool),
     ])
 
 libxl_device_vtpm = Struct("device_vtpm", [

tools/libxl/libxlu_pci.c +2 -0

index 26fb143..989eac8 100644
@@ -143,6 +143,8 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str
                     pcidev->permissive = atoi(tok);
                 }else if ( !strcmp(optkey, "seize") ) {
                     pcidev->seize = atoi(tok);
+                }else if ( !strcmp(optkey, "rdmforce") ) {
+                    pcidev->rdmforce = atoi(tok);
                 }else{
                     XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey);
                 }

tools/libxl/xl_cmdimpl.c +10 -0

index 0e754e7..9c23733 100644
@@ -919,6 +919,7 @@ static void parse_config_data(const char *config_source,
     int pci_msitranslate = 0;
     int pci_permissive = 0;
     int pci_seize = 0;
+    int pci_rdmforce = 0;
     int i, e;
 
     libxl_domain_create_info *c_info = &d_config->c_info;

@@ -1699,6 +1700,9 @@ skip_vfb:
     if (!xlu_cfg_get_long (config, "pci_seize", &l, 0))
         pci_seize = l;
 
+    if (!xlu_cfg_get_long (config, "pci_rdmforce", &l, 0))
+        pci_rdmforce = l;
+
     /* To be reworked (automatically enabled) once the auto ballooning
      * after guest starts is done (with PCI devices passed in). */
     if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {

@@ -1719,6 +1723,7 @@ skip_vfb:
             pcidev->power_mgmt = pci_power_mgmt;
             pcidev->permissive = pci_permissive;
             pcidev->seize = pci_seize;
+            pcidev->rdmforce = pci_rdmforce;
             if (!xlu_pci_parse_bdf(config, pcidev, buf))
                 d_config->num_pcidevs++;
         }

@@ -1726,6 +1731,11 @@ skip_vfb:
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
+    if ((c_info->type == LIBXL_DOMAIN_TYPE_HVM) && pci_rdmforce)
+        libxl_defbool_set(&b_info->rdmforce, true);
+    else
+        libxl_defbool_set(&b_info->rdmforce, false);
+
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
     case 0:
         {

xen/drivers/passthrough/pci.c +1 -0

index 78c6977..ae924ad 100644
@@ -34,6 +34,7 @@
 #include <xen/tasklet.h>
 #include <xsm/xsm.h>
 #include <asm/msi.h>
+#include <xen/stdbool.h>
 
 struct pci_seg {
     struct list_head alldevs_list;

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
         }
         break;

xen/drivers/passthrough/pci.c +9 -0

+                return -EFAULT;
+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;
+        rcu_unlock_domain(d);
+    }
+        break;
+
     case XEN_DOMCTL_assign_device:
         if ( unlikely(d->is_dying) )
         {

xen/drivers/passthrough/vtd/dmar.c +0 -0

index 1152c3a..5e41e7a 100644

xen/include/public/domctl.h +8 -0

+    uint32_t    num_pcidevs;
+    XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
+};
+typedef struct xen_domctl_set_rdm xen_domctl_set_rdm_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_rdm_t);
+
 /* Pass-through interrupts: bind real irq -> hvm devfn. */
 /* XEN_DOMCTL_bind_pt_irq */
 /* XEN_DOMCTL_unbind_pt_irq */

@@ -1070,6 +1089,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_arm_configure_domain          76
+#define XEN_DOMCTL_set_rdm                       77
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002

@@ -1135,6 +1155,7 @@ struct xen_domctl {
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
+        struct xen_domctl_set_rdm           set_rdm;
         uint8_t                             pad[128];
     } u;
 };

xen/xsm/flask/hooks.c +0 -0

index d48463f..5a760e2 100644

Signature · 2 lines

-- 
1.9.1

Quoted 25 lines · Tiejun Chen

This should be based on a new parameter globally, 'pci_rdmforce'.

pci_rdmforce = 1 => Of course this should be 0 by default.

'1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM.

But we can override that by one specific pci device:

pci = ['AA:BB.CC,rdmforce=0/1]

But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach.

So this domctl provides an approach to control how to populate reserved device memory by tools.

Note we always post a message to user about this once we owns RMRR.

Signed-off-by Tiejun Chen <[email protected]>

Quoted 11 lines

---
  docs/man/xl.cfg.pod.5              |  6 +++++
  docs/misc/vtd.txt                  | 15 ++++++++++++
  tools/libxc/include/xenctrl.h      |  6 +++++
  tools/libxc/xc_domain.c            | 28 +++++++++++++++++++++++
  tools/libxl/libxl_create.c         |  3 +++
  tools/libxl/libxl_dm.c             | 47 ++++++++++++++++++++++++++++++++++++++
  tools/libxl/libxl_internal.h       |  4 ++++
  tools/libxl/libxl_types.idl        |  2 ++
  tools/libxl/libxlu_pci.c           |  2 ++
  tools/libxl/xl_cmdimpl.c           | 10 ++++++++

In the past we had split the hypervisor and the
toolstack patches in two. So that one could focus
on the hypervisor ones first, and then in another
patch on the toolstack.

But perhaps this was intended to be in one patch?

Quoted 4 lines

  xen/drivers/passthrough/pci.c      | 39 +++++++++++++++++++++++++++++++
  xen/drivers/passthrough/vtd/dmar.c |  8 +++++++
  xen/include/asm-x86/hvm/domain.h   |  4 ++++

I don't see ARM here? Should there be an ARM variant of this? If not

should the toolstack ones only run under x86?

Quoted 13 lines

  xen/include/public/domctl.h        | 21 +++++++++++++++++
  xen/xsm/flask/hooks.c              |  1 +
  15 files changed, 196 insertions(+)

docs/man/xl.cfg.pod.5 +0 -0

index 622ea53..9adc41e 100644
@@ -645,6 +645,12 @@ dom0 without confirmation.  Please use with care.
  D0-D3hot power management states for the PCI device. False (0) by
  default.
 

docs/man/xl.cfg.pod.5 dom0 without confirmation. Please use with care. · line 650

Quoted 3 lines

docs/man/xl.cfg.pod.5 +3 -0

+=item B<rdmforce=BOOLEAN>
+
+(HVM/x86 only) Specifies that the VM would force to check and try to

s/force/forced/

docs/man/xl.cfg.pod.5 dom0 without confirmation. Please use with care. · line 652

Quoted 2 lines

docs/man/xl.cfg.pod.5 +2 -0

+reserve all reserved device memory, like RMRR, associated to the PCI
+device. False (0) by default.

Not sure I understand. How would the VM be forced to do this? Or is it that the hvmloader would force to do this? And if it fails (as you

say 'try') ? What then?

Quoted 13 lines

docs/man/xl.cfg.pod.5 +1 -0

+
  =back
 
  =back

docs/misc/vtd.txt +0 -0

index 9af0e99..23544d5 100644
@@ -111,6 +111,21 @@ in the config file:
  To override for a specific device:
  	pci = [ '01:00.0,msitranslate=0', '03:00.0' ]
 

docs/misc/vtd.txt in the config file: · line 117

Quoted 4 lines

docs/misc/vtd.txt +4 -0

+RDM, 'reserved device memory', for PCI Device Passthrough
+---------------------------------------------------------
+
+The BIOS controls some devices in terms of some reginos of memory used for

Could you elaborate what 'some devices' are? Network cards? GPUs? What are the most commons ones.

s/reginos/regions/

And by regions you mean BAR regions?

docs/misc/vtd.txt in the config file: · line 119

Quoted 3 lines

docs/misc/vtd.txt +2 -0

+these devices. This kind of region should be reserved before creating a VM
+to make sure they are not occupied by RAM/MMIO to conflict, and also we can

You said 'This' but here you are using the plural ' are'. IF you want it plural it needs to be 'These regions'

docs/misc/vtd.txt in the config file: · line 124

Quoted 5 lines

docs/misc/vtd.txt +5 -0

+create necessary IOMMU table successfully.
+
+To enable this globally, add "pci_rdmforce" in the config file:
+
+	pci_rdmforce = 1         (default is 0)

The guest config file? Or /etc/xen/xl.conf ?

Quoted 9 lines

docs/misc/vtd.txt +4 -0

+
+Or just enable for a specific device:
+	pci = [ '01:00.0,rdmforce=1', '03:00.0' ]
+
 
  Caveat on Conventional PCI Device Passthrough
  ---------------------------------------------

tools/libxc/include/xenctrl.h +0 -0

Quoted 14 lines

xen/drivers/passthrough/pci.c +1 -0

@@ -34,6 +34,7 @@
  #include <xen/tasklet.h>
  #include <xsm/xsm.h>
  #include <asm/msi.h>
+#include <xen/stdbool.h>
 
  struct pci_seg {
      struct list_head alldevs_list;

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
          }
          break;
 

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1565

Quoted 9 lines

xen/drivers/passthrough/pci.c +9 -0

+    case XEN_DOMCTL_set_rdm:
+    {
+        struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+        struct xen_guest_pcidev_info *pcidevs = NULL;
+        struct domain *d = rcu_lock_domain_by_any_id(domctl->domain);
+
+        if ( d == NULL )
+            return -ESRCH;
+

What if this is called on an PV domain?

You are also missing the XSM checks.

What if this is called multiple times. Is it OK to over-ride the 'pci_force' or should it stick once?

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1567

Quoted 4 lines

xen/drivers/passthrough/pci.c +2 -0

+        d->arch.hvm_domain.pci_force =
+                            xdsr->flags & PCI_DEV_RDM_CHECK ? true : false;

Won't we crash here if this is called for PV guests?

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1568

Quoted 2 lines

xen/drivers/passthrough/pci.c +1 -0

+        d->arch.hvm_domain.num_pcidevs = xdsr->num_pcidevs;

What if the 'num_pcidevs' has some bogus value. You need to check for that.

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1569

Quoted 3 lines

xen/drivers/passthrough/pci.c +1 -0

+        d->arch.hvm_domain.pcidevs = NULL;

Please first free it. It might be that the toolstack is doing this a couple of times. You don't want to leak memory.

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1578

Quoted 10 lines

xen/drivers/passthrough/pci.c +9 -0

+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);
+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;

But you already have set 'num_pcidevs' to some value. This copying/check should be done before you modify 'd->arch.hvm_domain'...

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1585

Quoted 7 lines

xen/drivers/passthrough/pci.c +7 -0

+            }
+
+            if ( copy_from_guest(pcidevs, xdsr->pcidevs,
+                                 xdsr->num_pcidevs*sizeof(*pcidevs)) )
+            {
+                xfree(pcidevs);
+                rcu_unlock_domain(d);

Ditto. You need to do these checks before you modify 'd->arch.hvm_domain'.

Quoted 16 lines

xen/drivers/passthrough/pci.c +9 -0

+                return -EFAULT;
+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;
+        rcu_unlock_domain(d);
+    }
+        break;
+
      case XEN_DOMCTL_assign_device:
          if ( unlikely(d->is_dying) )
          {

xen/drivers/passthrough/vtd/dmar.c +0 -0

index 1152c3a..5e41e7a 100644

xen/drivers/passthrough/vtd/dmar.c acpi_parse_one_rmrr(struct acpi_dmar_header *header) · line 678

Quoted 6 lines

xen/drivers/passthrough/vtd/dmar.c +2 -0

@@ -674,6 +674,14 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
                          "  RMRR region: base_addr %"PRIx64
                          " end_address %"PRIx64"\n",
                          rmrru->base_address, rmrru->end_address);
+            /*
+             * TODO: we may provide a precise paramter just to reserve

s/paramter/parameter/

xen/drivers/passthrough/vtd/dmar.c acpi_parse_one_rmrr(struct acpi_dmar_header *header) · line 683

Quoted 5 lines

xen/drivers/passthrough/vtd/dmar.c +5 -0

+             * RMRR range specific to one device.
+             */
+            dprintk(XENLOG_WARNING VTDPREFIX,
+                    "So please set pci_rdmforce to reserve these ranges"
+                    " if you need such a device in hotplug case.\n");

'Please set rdmforce to reserve ranges %lx->%lx if you plan to hotplug this device.'

But then this is going to be a bit verbose, so perhaps:

'Ranges %lx-%lx need rdmforce to properly work.' ?

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 92

Quoted 13 lines

xen/drivers/passthrough/vtd/dmar.c +1 -0

+
              acpi_register_rmrr_unit(rmrru);
          }
      }

xen/include/asm-x86/hvm/domain.h +0 -0

index 2757c7f..38530e5 100644
@@ -90,6 +90,10 @@ struct hvm_domain {
      /* Cached CF8 for guest PCI config cycles */
      uint32_t                pci_cf8;
 

Maybe a comment explaining its purpose?

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 96

Quoted 5 lines

xen/include/asm-x86/hvm/domain.h +4 -0

+    bool_t                  pci_force;
+    uint32_t                num_pcidevs;
+    struct xen_guest_pcidev_info      *pcidevs;
+

You are also missing freeing of this in the hypervisor when the guest is destroyed. Please fix that.

Quoted 12 lines

xen/include/asm-x86/hvm/domain.h +0 -0

      struct pl_time         pl_time;
 
      struct hvm_io_handler *io_handler;

xen/include/public/domctl.h +0 -0

index 57e2ed7..ba8970d 100644
@@ -508,6 +508,25 @@ struct xen_domctl_get_device_group {
  typedef struct xen_domctl_get_device_group xen_domctl_get_device_group_t;
  DEFINE_XEN_GUEST_HANDLE(xen_domctl_get_device_group_t);
 

xen/include/public/domctl.h struct xen_domctl_get_device_group { · line 511

Quoted 1 line

xen/include/public/domctl.h +1 -0

+/* Currently just one bit to indicate force to check Reserved Device Memory. */

Not sure I understand. Did you mean:

'Check Reserved Device Memory'.

What happens if you do not have this flag? What are the semantics of this hypercall - as in what will it mean.

xen/include/public/domctl.h struct xen_domctl_get_device_group { · line 523

Quoted 13 lines

xen/include/public/domctl.h +12 -0

+#define PCI_DEV_RDM_CHECK   0x1
+struct xen_guest_pcidev_info {
+    uint16_t    seg;
+    uint8_t     bus;
+    uint8_t     devfn;
+    uint32_t    flags;
+};
+typedef struct xen_guest_pcidev_info xen_guest_pcidev_info_t;
+DEFINE_XEN_GUEST_HANDLE(xen_guest_pcidev_info_t);
+/* Control whether/how we check and reserve device memory. */
+struct xen_domctl_set_rdm {
+    uint32_t    flags;

What is this 'flags' purpose compared to the 'pcidevs.flags'? Please explain.

Quoted 29 lines

xen/include/public/domctl.h +8 -0

+    uint32_t    num_pcidevs;
+    XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
+};
+typedef struct xen_domctl_set_rdm xen_domctl_set_rdm_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_rdm_t);
+
  /* Pass-through interrupts: bind real irq -> hvm devfn. */
  /* XEN_DOMCTL_bind_pt_irq */
  /* XEN_DOMCTL_unbind_pt_irq */

@@ -1070,6 +1089,7 @@ struct xen_domctl {
  #define XEN_DOMCTL_setvnumainfo                  74
  #define XEN_DOMCTL_psr_cmt_op                    75
  #define XEN_DOMCTL_arm_configure_domain          76
+#define XEN_DOMCTL_set_rdm                       77
  #define XEN_DOMCTL_gdbsx_guestmemio            1000
  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002

@@ -1135,6 +1155,7 @@ struct xen_domctl {
          struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
          struct xen_domctl_vnuma             vnuma;
          struct xen_domctl_psr_cmt_op        psr_cmt_op;
+        struct xen_domctl_set_rdm           set_rdm;
          uint8_t                             pad[128];
      } u;
  };

xen/xsm/flask/hooks.c +0 -1

index d48463f..5a760e2 100644
--- a/xen/xsm/flask/hooks.c

xen/xsm/flask/hooks.c static int flask_domctl(struct domain *d, int cmd) · line 595

Quoted 6 lines

+++ b/xen/xsm/flask/hooks.c

xen/xsm/flask/hooks.c +1 -0

@@ -592,6 +592,7 @@ static int flask_domctl(struct domain *d, int cmd)
      case XEN_DOMCTL_test_assign_device:
      case XEN_DOMCTL_assign_device:
      case XEN_DOMCTL_deassign_device:
+    case XEN_DOMCTL_set_rdm:

There is more to XSM than just this file..

Please compile with XSM enabled.

xen/xsm/flask/hooks.c static int flask_domctl(struct domain *d, int cmd) · line 597

Quoted 2 lines

xen/xsm/flask/hooks.c +0 -0

  #endif
          return 0;

Also how does this work with 32-bit dom0s? Is there a need to use the compat layer?

Quoted 38 lines · Konrad Rzeszutek Wilk

Quoted 25 lines · Tiejun Chen

This should be based on a new parameter globally, 'pci_rdmforce'.

pci_rdmforce = 1 => Of course this should be 0 by default.

'1' means we should force check to reserve all ranges. If failed VM wouldn't be created successfully. This also can give user a chance to work well with later hotplug, even if not a device assignment while creating VM.

But we can override that by one specific pci device:

pci = ['AA:BB.CC,rdmforce=0/1]

But this 'rdmforce' should be 1 by default since obviously any passthrough device always need to do this. Actually no one really want to set as '0' so it may be unnecessary but I'd like to leave this as a potential approach.

So this domctl provides an approach to control how to populate reserved device memory by tools.

Note we always post a message to user about this once we owns RMRR.

Signed-off-by Tiejun Chen <[email protected]>

Quoted 11 lines

---
 docs/man/xl.cfg.pod.5              |  6 +++++
 docs/misc/vtd.txt                  | 15 ++++++++++++
 tools/libxc/include/xenctrl.h      |  6 +++++
 tools/libxc/xc_domain.c            | 28 +++++++++++++++++++++++
 tools/libxl/libxl_create.c         |  3 +++
 tools/libxl/libxl_dm.c             | 47 ++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h       |  4 ++++
 tools/libxl/libxl_types.idl        |  2 ++
 tools/libxl/libxlu_pci.c           |  2 ++
 tools/libxl/xl_cmdimpl.c           | 10 ++++++++

Quoted 5 lines

In the past we had split the hypervisor and the
toolstack patches in two. So that one could focus
on the hypervisor ones first, and then in another
patch on the toolstack.

Yes.

Quoted 1 line

But perhaps this was intended to be in one patch?

This change also involve docs so its little bit harder to understand the whole page if we split this.

Quoted 6 lines

Quoted 4 lines

 xen/drivers/passthrough/pci.c      | 39 +++++++++++++++++++++++++++++++
 xen/drivers/passthrough/vtd/dmar.c |  8 +++++++
 xen/include/asm-x86/hvm/domain.h   |  4 ++++

I don't see ARM here? Should there be an ARM variant of this? If not

ARM doesn't need this feature.

Quoted 1 line

should the toolstack ones only run under x86?

And I think this shouldn't broken current ARM path as well. I mean this would return simply since ARM hasn't such a hypercall handler.

Quoted 13 lines

Quoted 13 lines

 xen/include/public/domctl.h        | 21 +++++++++++++++++
 xen/xsm/flask/hooks.c              |  1 +
 15 files changed, 196 insertions(+)

docs/man/xl.cfg.pod.5 +0 -0

index 622ea53..9adc41e 100644
@@ -645,6 +645,12 @@ dom0 without confirmation.  Please use with care.
 D0-D3hot power management states for the PCI device. False (0) by
 default.
 

Quoted 5 lines

docs/man/xl.cfg.pod.5 dom0 without confirmation. Please use with care. · line 650

Quoted 3 lines

docs/man/xl.cfg.pod.5 +3 -0

+=item B<rdmforce=BOOLEAN>
+
+(HVM/x86 only) Specifies that the VM would force to check and try to

s/force/forced/

I guess you're saying 'be forced'.

Quoted 5 lines

docs/man/xl.cfg.pod.5 dom0 without confirmation. Please use with care. · line 652

Quoted 2 lines

docs/man/xl.cfg.pod.5 +2 -0

+reserve all reserved device memory, like RMRR, associated to the PCI
+device. False (0) by default.

Not sure I understand. How would the VM be forced to do this? Or is it that the hvmloader would force to do this? And if it fails (as you

Yes.

Quoted 1 line

say 'try') ? What then?

In most cases we can reserve these regions but if these RMRR regions overlap with some fixed range, like guest BIOS, we can't succeed in this case.

Quoted 13 lines

Quoted 13 lines

docs/man/xl.cfg.pod.5 +1 -0

+
 =back
 
 =back

docs/misc/vtd.txt +0 -0

index 9af0e99..23544d5 100644
@@ -111,6 +111,21 @@ in the config file:
 To override for a specific device:
 	pci = [ '01:00.0,msitranslate=0', '03:00.0' ]
 

Quoted 7 lines

docs/misc/vtd.txt in the config file: · line 117

Quoted 4 lines

docs/misc/vtd.txt +4 -0

+RDM, 'reserved device memory', for PCI Device Passthrough
+---------------------------------------------------------
+
+The BIOS controls some devices in terms of some reginos of memory used for

Could you elaborate what 'some devices' are? Network cards? GPUs? What are the most commons ones.

Some legacy USB device to perform PS2 emulation, and GPU has a stolen memory as I remember.

Quoted 2 lines

s/reginos/regions/

Fixed.

Quoted 2 lines

And by regions you mean BAR regions?

No. I guess you want to know some background about RMRR :)

There's a good brief description in Linux:

What is RMRR?
-------------

There are some devices the BIOS controls, for e.g USB devices to perform PS2 emulation. The regions of memory used for these devices are marked reserved in the e820 map. When we turn on DMA translation, DMA to those regions will fail. Hence BIOS uses RMRR to specify these regions along with devices that need to access these regions. OS is expected to setup unity mappings for these regions for these devices to access these regions.

Quoted 6 lines

docs/misc/vtd.txt in the config file: · line 119

Quoted 3 lines

docs/misc/vtd.txt +2 -0

+these devices. This kind of region should be reserved before creating a VM
+to make sure they are not occupied by RAM/MMIO to conflict, and also we can

You said 'This' but here you are using the plural ' are'. IF you want it plural it needs to be 'These regions'

Thanks for your correction.

Quoted 7 lines

docs/misc/vtd.txt in the config file: · line 124

Quoted 5 lines

docs/misc/vtd.txt +5 -0

+create necessary IOMMU table successfully.
+
+To enable this globally, add "pci_rdmforce" in the config file:
+
+	pci_rdmforce = 1         (default is 0)

The guest config file? Or /etc/xen/xl.conf ?

The guest config file. Here I just follow something about 'pci_msitranslate' since they have that usage in common.

Quoted 9 lines

Quoted 9 lines

docs/misc/vtd.txt +4 -0

+
+Or just enable for a specific device:
+	pci = [ '01:00.0,rdmforce=1', '03:00.0' ]
+
 
 Caveat on Conventional PCI Device Passthrough
 ---------------------------------------------

tools/libxc/include/xenctrl.h +0 -0

[snip]

Quoted 14 lines

Quoted 14 lines

xen/drivers/passthrough/pci.c +1 -0

@@ -34,6 +34,7 @@
 #include <xen/tasklet.h>
 #include <xsm/xsm.h>
 #include <asm/msi.h>
+#include <xen/stdbool.h>
 
 struct pci_seg {
     struct list_head alldevs_list;

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
         }
         break;
 

Quoted 11 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1565

Quoted 9 lines

xen/drivers/passthrough/pci.c +9 -0

+    case XEN_DOMCTL_set_rdm:
+    {
+        struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+        struct xen_guest_pcidev_info *pcidevs = NULL;
+        struct domain *d = rcu_lock_domain_by_any_id(domctl->domain);
+
+        if ( d == NULL )
+            return -ESRCH;
+

What if this is called on an PV domain?

Currently we just support this in HVM, so I'd like to add this,

         if ( d == NULL )
             return -ESRCH;

Quoted 3 lines

You are also missing the XSM checks.

Just see this below.

Quoted 3 lines

What if this is called multiple times. Is it OK to over-ride the 'pci_force' or should it stick once?

It should be fine since just xc/hvmloader need such an information while creating a VM.

And especially, currently we just call this one time to set. So why we need to call this again and again? I think if anyone want to extend such a case you're worrying, he should know any effect before he take a action, right?

Quoted 7 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1567

Quoted 5 lines

xen/drivers/passthrough/pci.c +2 -0

+        d->arch.hvm_domain.pci_force =
+                            xdsr->flags & PCI_DEV_RDM_CHECK ? true : false;

Won't we crash here if this is called for PV guests?

After the line, 'ASSERT( is_hvm_domain(d) );', is added, this problem should be gone.

Quoted 5 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1568

Quoted 3 lines

xen/drivers/passthrough/pci.c +1 -0

+        d->arch.hvm_domain.num_pcidevs = xdsr->num_pcidevs;

What if the 'num_pcidevs' has some bogus value. You need to check for that.

This value is grabbed from that existing interface, assign_device, so I mean this is already checked.

Quoted 7 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1569

Quoted 3 lines

xen/drivers/passthrough/pci.c +1 -0

+        d->arch.hvm_domain.pcidevs = NULL;

Please first free it. It might be that the toolstack is doing this a couple of times. You don't want to leak memory.

Okay,

(quoted diff) +2 -0

+        if ( d->arch.hvm_domain.pcidevs )
+            xfree(d->arch.hvm_domain.pcidevs);

Quoted 12 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1578

Quoted 11 lines

xen/drivers/passthrough/pci.c +9 -0

+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);
+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;

Quoted 2 lines

But you already have set 'num_pcidevs' to some value. This copying/check should be done before you modify 'd->arch.hvm_domain'...

This makes sense so I'll move down this fragment.

Quoted 26 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1585

Quoted 7 lines

xen/drivers/passthrough/pci.c +7 -0

+            }
+
+            if ( copy_from_guest(pcidevs, xdsr->pcidevs,
+                                 xdsr->num_pcidevs*sizeof(*pcidevs)) )
+            {
+                xfree(pcidevs);
+                rcu_unlock_domain(d);

Ditto. You need to do these checks before you modify 'd->arch.hvm_domain'.

Quoted 16 lines

xen/drivers/passthrough/pci.c +9 -0

+                return -EFAULT;
+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;
+        rcu_unlock_domain(d);
+    }
+        break;
+
     case XEN_DOMCTL_assign_device:
         if ( unlikely(d->is_dying) )
         {

xen/drivers/passthrough/vtd/dmar.c +0 -0

index 1152c3a..5e41e7a 100644

Quoted 8 lines

xen/drivers/passthrough/vtd/dmar.c acpi_parse_one_rmrr(struct acpi_dmar_header *header) · line 678

Quoted 6 lines

xen/drivers/passthrough/vtd/dmar.c +2 -0

@@ -674,6 +674,14 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
                         "  RMRR region: base_addr %"PRIx64
                         " end_address %"PRIx64"\n",
                         rmrru->base_address, rmrru->end_address);
+            /*
+             * TODO: we may provide a precise paramter just to reserve

s/paramter/parameter/

Fixed.

Quoted 5 lines

xen/drivers/passthrough/vtd/dmar.c acpi_parse_one_rmrr(struct acpi_dmar_header *header) · line 683

Quoted 5 lines

xen/drivers/passthrough/vtd/dmar.c +5 -0

+             * RMRR range specific to one device.
+             */
+            dprintk(XENLOG_WARNING VTDPREFIX,
+                    "So please set pci_rdmforce to reserve these ranges"
+                    " if you need such a device in hotplug case.\n");

s/hotplug/passthrough

Quoted 6 lines

'Please set rdmforce to reserve ranges %lx->%lx if you plan to hotplug this device.'

But then this is going to be a bit verbose, so perhaps:

'Ranges %lx-%lx need rdmforce to properly work.' ?

Its unnecessary to output range again since we already have such a print message here.

Quoted 15 lines

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 92

Quoted 13 lines

xen/drivers/passthrough/vtd/dmar.c +1 -0

+
             acpi_register_rmrr_unit(rmrru);
         }
     }

xen/include/asm-x86/hvm/domain.h +0 -0

index 2757c7f..38530e5 100644
@@ -90,6 +90,10 @@ struct hvm_domain {
     /* Cached CF8 for guest PCI config cycles */
     uint32_t                pci_cf8;
 

Maybe a comment explaining its purpose?

Okay.

/* Force to check/reserve Reserved Device Memory. */

    bool_t                  pci_force;

Quoted 8 lines

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 96

Quoted 5 lines

xen/include/asm-x86/hvm/domain.h +4 -0

+    bool_t                  pci_force;
+    uint32_t                num_pcidevs;
+    struct xen_guest_pcidev_info      *pcidevs;
+

You are also missing freeing of this in the hypervisor when the guest is destroyed. Please fix that.

You're right. I will go there next revision.

Quoted 12 lines

Quoted 12 lines

xen/include/asm-x86/hvm/domain.h +0 -0

     struct pl_time         pl_time;
 
     struct hvm_io_handler *io_handler;

xen/include/public/domctl.h +0 -0

index 57e2ed7..ba8970d 100644
@@ -508,6 +508,25 @@ struct xen_domctl_get_device_group {
 typedef struct xen_domctl_get_device_group xen_domctl_get_device_group_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_get_device_group_t);
 

Quoted 5 lines

xen/include/public/domctl.h struct xen_domctl_get_device_group { · line 511

Quoted 1 line

xen/include/public/domctl.h +1 -0

+/* Currently just one bit to indicate force to check Reserved Device Memory. */

Not sure I understand. Did you mean:

'Check Reserved Device Memory'.

I can change this as '...force checking Reserved Device Memory.'

Quoted 3 lines

What happens if you do not have this flag? What are the semantics of this hypercall - as in what will it mean.

If we have no this flag, these devices owned RMRR can't work in passthrough case.

Quoted 16 lines

xen/include/public/domctl.h struct xen_domctl_get_device_group { · line 523

Quoted 13 lines

xen/include/public/domctl.h +12 -0

+#define PCI_DEV_RDM_CHECK   0x1
+struct xen_guest_pcidev_info {
+    uint16_t    seg;
+    uint8_t     bus;
+    uint8_t     devfn;
+    uint32_t    flags;
+};
+typedef struct xen_guest_pcidev_info xen_guest_pcidev_info_t;
+DEFINE_XEN_GUEST_HANDLE(xen_guest_pcidev_info_t);
+/* Control whether/how we check and reserve device memory. */
+struct xen_domctl_set_rdm {
+    uint32_t    flags;

What is this 'flags' purpose compared to the 'pcidevs.flags'? Please explain.

I replied something to Kevin, and we just need a global flag so we can remove pcidevs.flags.

Quoted 29 lines

Quoted 29 lines

xen/include/public/domctl.h +8 -0

+    uint32_t    num_pcidevs;
+    XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
+};
+typedef struct xen_domctl_set_rdm xen_domctl_set_rdm_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_rdm_t);
+
 /* Pass-through interrupts: bind real irq -> hvm devfn. */
 /* XEN_DOMCTL_bind_pt_irq */
 /* XEN_DOMCTL_unbind_pt_irq */

@@ -1070,6 +1089,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_arm_configure_domain          76
+#define XEN_DOMCTL_set_rdm                       77
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002

@@ -1135,6 +1155,7 @@ struct xen_domctl {
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
+        struct xen_domctl_set_rdm           set_rdm;
         uint8_t                             pad[128];
     } u;
 };

xen/xsm/flask/hooks.c +0 -1

index d48463f..5a760e2 100644
--- a/xen/xsm/flask/hooks.c

Quoted 8 lines

xen/xsm/flask/hooks.c static int flask_domctl(struct domain *d, int cmd) · line 595

Quoted 6 lines

+++ b/xen/xsm/flask/hooks.c

xen/xsm/flask/hooks.c +1 -0

@@ -592,6 +592,7 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_test_assign_device:
     case XEN_DOMCTL_assign_device:
     case XEN_DOMCTL_deassign_device:
+    case XEN_DOMCTL_set_rdm:

There is more to XSM than just this file..

But I don't see more other stuff, like XEN_DOMCTL_assign_device.

Quoted 1 line

Quoted 1 line

Please compile with XSM enabled.

Anyway, I add XSM_ENABLE = y and FLASK_ENABLE = y in Config.mk then recompile, but looks good.

Anything I'm missing?

Quoted 5 lines

Quoted 3 lines

xen/xsm/flask/hooks.c +0 -0

 #endif
         return 0;

Quoted 2 lines

Also how does this work with 32-bit dom0s? Is there a need to use the compat layer?

Are you saying in xsm case? Others?

Actually this new DOMCTL is similar with XEN_DOMCTL_assign_device in some senses but I don't see such an issue you're pointing.

Thanks
Tiejun

Quoted 28 lines

Quoted 14 lines

Quoted 14 lines

xen/drivers/passthrough/pci.c +1 -0

@@ -34,6 +34,7 @@
  #include <xen/tasklet.h>
  #include <xsm/xsm.h>
  #include <asm/msi.h>
+#include <xen/stdbool.h>
 
  struct pci_seg {
      struct list_head alldevs_list;

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
          }
          break;
 

Quoted 11 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1565

Quoted 9 lines

xen/drivers/passthrough/pci.c +9 -0

+    case XEN_DOMCTL_set_rdm:
+    {
+        struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+        struct xen_guest_pcidev_info *pcidevs = NULL;
+        struct domain *d = rcu_lock_domain_by_any_id(domctl->domain);
+
+        if ( d == NULL )
+            return -ESRCH;
+

What if this is called on an PV domain?

Currently we just support this in HVM, so I'd like to add this,

Review

Quoted 5 lines

          if ( d == NULL )
              return -ESRCH;

(quoted diff) +2 -0

+        ASSERT( is_hvm_domain(d) );
+

No. Please don't crash the hypervisor.

Just return -ENOSYS or such when done for PV guests.

Quoted 11 lines

Quoted 4 lines

You are also missing the XSM checks.

Just see this below.

Quoted 3 lines

What if this is called multiple times. Is it OK to over-ride the 'pci_force' or should it stick once?

Quoted 6 lines

It should be fine since just xc/hvmloader need such an information while creating a VM.

And especially, currently we just call this one time to set. So why we need to call this again and again? I think if anyone want to extend such a case you're worrying, he should know any effect before he take a action, right?

Program defensively and also think about preemption. If this call end up

being preempted you might need to call it again. Or if the third-party toolstack use this operation and call this with wacky values?

Quoted 8 lines

Quoted 7 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1567

Quoted 5 lines

xen/drivers/passthrough/pci.c +2 -0

+        d->arch.hvm_domain.pci_force =
+                            xdsr->flags & PCI_DEV_RDM_CHECK ? true : false;

Won't we crash here if this is called for PV guests?

Quoted 2 lines

After the line, 'ASSERT( is_hvm_domain(d) );', is added, this problem should be gone.

No it won't be. You will just crash the hypervisor.

Please please put yourself in the mind that the toolstack can (and will) have bugs.

Quoted 6 lines

Quoted 5 lines

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1568

Quoted 3 lines

xen/drivers/passthrough/pci.c +1 -0

+        d->arch.hvm_domain.num_pcidevs = xdsr->num_pcidevs;

What if the 'num_pcidevs' has some bogus value. You need to check for that.

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

          return 0;

Also how does this work with 32-bit dom0s? Is there a need to use the compat layer?

Quoted 4 lines

Are you saying in xsm case? Others?

Actually this new DOMCTL is similar with XEN_DOMCTL_assign_device in some senses but I don't see such an issue you're pointing.

I was thinking about the compat layer and making sure it works properly.

Quoted 10 lines

Quoted 8 lines · On 09.12.14 at 02:06, <[email protected]> wrote:

Quoted 2 lines

Also how does this work with 32-bit dom0s? Is there a need to use the compat layer?

Are you saying in xsm case? Others?

Actually this new DOMCTL is similar with XEN_DOMCTL_assign_device in some senses but I don't see such an issue you're pointing.

I was thinking about the compat layer and making sure it works properly.

Do we really need this consideration? I mean I referred to that existing XEN_DOMCTL_assign_device to implement this new DOMCTL, but looks there's nothing related to this point.

Or could you make your thought clear to me with an exiting example? Then I can take a look at what exactly should be taken in my new DOMCTL since I'm a fresh man to work out this properly inside xen.

Quoted 15 lines

Quoted 10 lines

Quoted 8 lines · On 09.12.14 at 02:06, <[email protected]> wrote:

Quoted 2 lines

Also how does this work with 32-bit dom0s? Is there a need to use the compat layer?

Are you saying in xsm case? Others?

Actually this new DOMCTL is similar with XEN_DOMCTL_assign_device in some senses but I don't see such an issue you're pointing.

I was thinking about the compat layer and making sure it works properly.

Do we really need this consideration? I mean I referred to that existing XEN_DOMCTL_assign_device to implement this new DOMCTL, but looks there's nothing related to this point.

Quoted 3 lines

Or could you make your thought clear to me with an exiting example? Then I can take a look at what exactly should be taken in my new DOMCTL since I'm a fresh man to work out this properly inside xen.

I think Konrad got a little confused here - domctl-s intentionally are structured so that they don't need a compat translation layer.

Jan

xen/drivers/passthrough/pci.c +6 -0

+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;
+            }
+

From: Jan Beulich [mailto:[email protected]]
Sent: Thursday, December 04, 2014 11:33 PM

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1589

Quoted 8 lines

xen/drivers/passthrough/pci.c +8 -0

+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;
+            }
+
+            if ( copy_from_guest(pcidevs, xdsr->pcidevs,
+

xdsr->num_pcidevs*sizeof(*pcidevs)) )

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1590

Quoted 8 lines

xen/drivers/passthrough/pci.c +8 -0

+            {
+                xfree(pcidevs);
+                rcu_unlock_domain(d);
+                return -EFAULT;
+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;

If the operation gets issued more than once for a given domain, you're leaking the old pointer here. Overall should think a bit more about this multiple use case (or outright disallow it).

xen/drivers/passthrough/pci.c line 37

Quoted 8 lines

Quoted 1 line

Quoted 1 line · On 01.12.14 at 10:24, <[email protected]> wrote:

xen/drivers/passthrough/pci.c +1 -0

@@ -34,6 +34,7 @@
  #include <xen/tasklet.h>
  #include <xsm/xsm.h>
  #include <asm/msi.h>
+#include <xen/stdbool.h>

Please don't - we use bool_t in the hypervisor, not bool. The header

only exists for source code shared with the tools.

Quoted 5 lines

xen/drivers/passthrough/pci.c +0 -0

@@ -1553,6 +1554,44 @@ int iommu_do_pci_domctl(
          }
          break;
 

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1561

Quoted 5 lines

xen/drivers/passthrough/pci.c +5 -0

+    case XEN_DOMCTL_set_rdm:
+    {
+        struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+        struct xen_guest_pcidev_info *pcidevs = NULL;
+        struct domain *d = rcu_lock_domain_by_any_id(domctl->domain);

"d" gets passed into this function - no need to shadow the variable

and (wrongly) re-obtain the pointer.

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1568

Quoted 7 lines

xen/drivers/passthrough/pci.c +7 -0

+
+        if ( d == NULL )
+            return -ESRCH;
+
+        d->arch.hvm_domain.pci_force =
+                            xdsr->flags & PCI_DEV_RDM_CHECK ? true : false;
+        d->arch.hvm_domain.num_pcidevs = xdsr->num_pcidevs;

You shouldn't set the count before setting the pointer.

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1574

Quoted 7 lines

xen/drivers/passthrough/pci.c +6 -0

+        d->arch.hvm_domain.pcidevs = NULL;
+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);

New domctl-s must not represent security risks: xdsr->num_pcidevs can be (almost) arbitrarily large - do you really want to allow such huge allocations? A reasonable upper bound could for example be

the total number of PCI devices the hypervisor knows about.

Quoted 13 lines

xen/drivers/passthrough/pci.c +12 -0

+            if ( pcidevs == NULL )
+            {
+                rcu_unlock_domain(d);
+                return -ENOMEM;
+            }
+
+            if ( copy_from_guest(pcidevs, xdsr->pcidevs,
+                                 xdsr->num_pcidevs*sizeof(*pcidevs)) )
+            {
+                xfree(pcidevs);
+                rcu_unlock_domain(d);
+                return -EFAULT;

xen/drivers/passthrough/pci.c int iommu_do_pci_domctl( · line 1590

Quoted 4 lines

xen/drivers/passthrough/pci.c +4 -0

+            }
+        }
+
+        d->arch.hvm_domain.pcidevs = pcidevs;

If the operation gets issued more than once for a given domain, you're leaking the old pointer here. Overall should think a bit more about this multiple use case (or outright disallow it).

Quoted 4 lines

xen/drivers/passthrough/vtd/dmar.c acpi_parse_one_rmrr(struct acpi_dmar_header *header) · line 683

Quoted 10 lines

xen/drivers/passthrough/vtd/dmar.c +7 -0

@@ -674,6 +674,14 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
                          "  RMRR region: base_addr %"PRIx64
                          " end_address %"PRIx64"\n",
                          rmrru->base_address, rmrru->end_address);
+            /*
+             * TODO: we may provide a precise paramter just to reserve
+             * RMRR range specific to one device.
+             */
+            dprintk(XENLOG_WARNING VTDPREFIX,
+                    "So please set pci_rdmforce to reserve these ranges"
+                    " if you need such a device in hotplug case.\n");

It makes no sense to use dprintk() here. I also don't see how this message relates to whatever may have been logged immediately before, so the wording ("So please set ...") is questionable. Nor is the reference to "hotplug case" meaningful here - in this context, only physical (host) device hotplug can be meant without further qualification. In the end I think trying to log something here is just wrong - simply drop the message and make sure whatever you want

to say can be found easily by looking elsewhere.

Quoted 7 lines

xen/include/asm-x86/hvm/domain.h +0 -0

@@ -90,6 +90,10 @@ struct hvm_domain {
      /* Cached CF8 for guest PCI config cycles */
      uint32_t                pci_cf8;
 

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 95

Quoted 3 lines

xen/include/asm-x86/hvm/domain.h +3 -0

+    bool_t                  pci_force;
+    uint32_t                num_pcidevs;
+    struct xen_guest_pcidev_info      *pcidevs;

Without a comment all these field names are pretty questionable.

Quoted 1 line

Quoted 1 line · On 08.12.14 at 07:06, <[email protected]> wrote:

Quoted 1 line · Jan Beulich

Quoted 1 line

Quoted 1 line

Quoted 1 line · On 01.12.14 at 10:24, <[email protected]> wrote:

Looks this could be fine,

d->arch.hvm_domain.pci_force = xdsr->flags & PCI_DEV_RDM_CHECK;

Quoted 7 lines

Quoted 6 lines

xen/drivers/passthrough/pci.c +6 -0

+        d->arch.hvm_domain.pcidevs = NULL;
+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);

Quoted 3 lines

New domctl-s must not represent security risks: xdsr->num_pcidevs can be (almost) arbitrarily large - do you really want to allow such huge allocations? A reasonable upper bound could for example be

Sorry, as you know this num_pcidevs is from tools, and actually it share that result from that existing hypercall, assign_device, while parsing 'pci=[]', so I couldn't understand why this can be (almost" arbitrarily large.

Quoted 1 line

the total number of PCI devices the hypervisor knows about.

I take a quick look at this but looks we have no this exact value that we can get directly.

Quoted 11 lines

xen/include/asm-x86/hvm/domain.h struct hvm_domain { · line 95

Quoted 9 lines

xen/include/asm-x86/hvm/domain.h +3 -0

@@ -90,6 +90,10 @@ struct hvm_domain {
      /* Cached CF8 for guest PCI config cycles */
      uint32_t                pci_cf8;
 
+    bool_t                  pci_force;
+    uint32_t                num_pcidevs;
+    struct xen_guest_pcidev_info      *pcidevs;

Without a comment all these field names are pretty questionable.

Yeah, I try to add some comments,

     /* A global flag, we need to check/reserve all Reserved Device 
Memory. */
     bool_t                  pci_force;
     /*
      * If pci_force is 0, this represents how many pci devices we need
      * to check/reserve Reserved Device Memory.
      * If pci_force is 1, this is always 0.
      */
     uint32_t                num_pcidevs;
     /* This represents those pci devices instances when num_pcidevs != 
0. */
     struct xen_guest_pcidev_info      *pcidevs;

Quoted 6 lines

Quoted 1 line

Quoted 1 line · On 08.12.14 at 07:06, <[email protected]> wrote:

Quoted 1 line · Jan Beulich

Quoted 1 line

Quoted 1 line

Quoted 1 line · On 01.12.14 at 10:24, <[email protected]> wrote:

Looks this could be fine,

d->arch.hvm_domain.pci_force = xdsr->flags & PCI_DEV_RDM_CHECK;

Which is correct only because PCI_DEV_RDM_CHECK happens to be
1. Such hidden dependencies shouldn't be introduced though, in particular to avoid others then cloning the code for a flag that's not
1. The canonical form (found in many places throughout the treei

     d->arch.hvm_domain.pci_force = !!(xdsr->flags & PCI_DEV_RDM_CHECK);

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

xen/drivers/passthrough/pci.c +6 -0

+        d->arch.hvm_domain.pcidevs = NULL;
+
+        if ( xdsr->num_pcidevs )
+        {
+            pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+                                    xdsr->num_pcidevs);

Quoted 8 lines

Quoted 3 lines

New domctl-s must not represent security risks: xdsr->num_pcidevs can be (almost) arbitrarily large - do you really want to allow such huge allocations? A reasonable upper bound could for example be

Sorry, as you know this num_pcidevs is from tools, and actually it share that result from that existing hypercall, assign_device, while parsing 'pci=[]', so I couldn't understand why this can be (almost" arbitrarily large.

You imply well behaved tools, which you shouldn't when viewing things from a security perspective.

Quoted 4 lines

Quoted 1 line

the total number of PCI devices the hypervisor knows about.

I take a quick look at this but looks we have no this exact value that we can get directly.

You need some upper bound. Whether you introduce a properly

maintained count or a suitable estimate thereof doesn't matter.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

xen/include/asm-x86/hvm/domain.h +0 -0

@@ -90,6 +90,10 @@ struct hvm_domain {
       /* Cached CF8 for guest PCI config cycles */

I really didn't necessarily mean individual comments - one for the whole group would suffice.

Also I don't think pci_force is really the right name - all_pcidevs or some such would seem more suitable following the entire series. And finally, I'm generally advocating for avoiding redundant data items - I'm sure this "all" notion can be encoded reasonable with just the other two field (and a suitable checking macro).

Quoted 1 line

Quoted 1 line · On 09.12.14 at 03:38, <[email protected]> wrote:

Quoted 4 lines · Jan Beulich

Quoted 3 lines

Quoted 1 line

Quoted 1 line · On 08.12.14 at 07:06, <[email protected]> wrote:

I take a quick look at this but looks we have no this exact value that we can get directly.

Quoted 1 line

You need some upper bound. Whether you introduce a properly

In theory, we may have at most the number, domain(16bit) x bus(8bit) x device(5bit) x function(3bit), 2^16 x 2^8 x 2^5 x 2^3 = 0x1000000, so could we define a macro like this,

#define PCI_DEVICES_NUM_UP 0x1000000

From: Jan Beulich <[email protected]>

This is a prerequisite for punching holes into HVM and PVH guests' P2M to allow passing through devices that are associated with (on VT-d) RMRRs.

Signed-off-by Jan Beulich <[email protected]>

Acked-by Kevin Tian <[email protected]>

---
 xen/common/compat/memory.c           | 54 ++++++++++++++++++++++++++++++++++++
 xen/common/memory.c                  | 51 ++++++++++++++++++++++++++++++++++
 xen/drivers/passthrough/iommu.c      | 10 +++++++
 xen/drivers/passthrough/vtd/dmar.c   | 17 ++++++++++++
 xen/drivers/passthrough/vtd/extern.h |  1 +
 xen/drivers/passthrough/vtd/iommu.c  |  1 +
 xen/include/public/memory.h          | 24 +++++++++++++++-
 xen/include/xen/iommu.h              |  4 +++
 xen/include/xlat.lst                 |  3 +-
 9 files changed, 163 insertions(+), 2 deletions(-)

xen/common/compat/memory.c +54 -0

index 06c90be..60512fa 100644
@@ -16,6 +16,37 @@ CHECK_TYPE(domid);
 
 CHECK_mem_access_op;
 
+#ifdef HAS_PASSTHROUGH
+struct get_reserved_device_memory {
+    struct compat_reserved_device_memory_map map;
+    unsigned int used_entries;
+};
+
+static int get_reserved_device_memory(xen_pfn_t start,
+                                      xen_ulong_t nr, void *ctxt)
+{
+    struct get_reserved_device_memory *grdm = ctxt;
+
+    if ( grdm->used_entries < grdm->map.nr_entries )
+    {
+        struct compat_reserved_device_memory rdm = {
+            .start_pfn = start, .nr_pages = nr
+        };
+
+        if ( rdm.start_pfn != start || rdm.nr_pages != nr )
+            return -ERANGE;
+
+        if ( __copy_to_compat_offset(grdm->map.buffer, grdm->used_entries,
+                                     &rdm, 1) )
+            return -EFAULT;
+    }
+
+    ++grdm->used_entries;
+
+    return 0;
+}
+#endif
+
 int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
 {
     int split, op = cmd & MEMOP_CMD_MASK;

@@ -273,6 +304,29 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
             break;
         }
 
+#ifdef HAS_PASSTHROUGH
+        case XENMEM_reserved_device_memory_map:
+        {
+            struct get_reserved_device_memory grdm;
+
+            if ( copy_from_guest(&grdm.map, compat, 1) ||
+                 !compat_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
+                return -EFAULT;
+
+            grdm.used_entries = 0;
+            rc = iommu_get_reserved_device_memory(get_reserved_device_memory,
+                                                  &grdm);
+
+            if ( !rc && grdm.map.nr_entries < grdm.used_entries )
+                rc = -ENOBUFS;
+            grdm.map.nr_entries = grdm.used_entries;
+            if ( __copy_to_guest(compat, &grdm.map, 1) )
+                rc = -EFAULT;
+
+            return rc;
+        }
+#endif
+
         default:
             return compat_arch_memory_op(cmd, compat);
         }

xen/common/memory.c +28 -0

index 9f21bd3..4788acc 100644
@@ -692,6 +692,34 @@ out:
     return rc;
 }
 
+#ifdef HAS_PASSTHROUGH
+struct get_reserved_device_memory {
+    struct xen_reserved_device_memory_map map;
+    unsigned int used_entries;
+};
+
+static int get_reserved_device_memory(xen_pfn_t start,
+                                      xen_ulong_t nr, void *ctxt)
+{
+    struct get_reserved_device_memory *grdm = ctxt;
+
+    if ( grdm->used_entries < grdm->map.nr_entries )
+    {
+        struct xen_reserved_device_memory rdm = {
+            .start_pfn = start, .nr_pages = nr
+        };
+
+        if ( __copy_to_guest_offset(grdm->map.buffer, grdm->used_entries,
+                                    &rdm, 1) )
+            return -EFAULT;
+    }
+
+    ++grdm->used_entries;
+
+    return 0;
+}
+#endif
+
 long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d;

@@ -1101,6 +1129,29 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }

xen/common/memory.c +10 -0

+        if ( !rc && grdm.map.nr_entries < grdm.used_entries )
+            rc = -ENOBUFS;
+        grdm.map.nr_entries = grdm.used_entries;
+        if ( __copy_to_guest(arg, &grdm.map, 1) )
+            rc = -EFAULT;
+
+        break;
+    }
+#endif
+
     default:
         rc = arch_memory_op(cmd, arg);
         break;

xen/drivers/passthrough/iommu.c +10 -0

index cc12735..7c17e8d 100644
@@ -344,6 +344,16 @@ void iommu_crash_shutdown(void)
     iommu_enabled = iommu_intremap = 0;
 }
 
+int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
+{
+    const struct iommu_ops *ops = iommu_get_ops();
+
+    if ( !iommu_enabled || !ops->get_reserved_device_memory )
+        return 0;
+
+    return ops->get_reserved_device_memory(func, ctxt);
+}
+
 bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature)
 {
     const struct hvm_iommu *hd = domain_hvm_iommu(d);

xen/drivers/passthrough/vtd/dmar.c +17 -0

index 5e41e7a..86cfad3 100644
@@ -901,3 +901,20 @@ int platform_supports_x2apic(void)
     unsigned int mask = ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT;
     return cpu_has_x2apic && ((dmar_flags & mask) == ACPI_DMAR_INTR_REMAP);
 }
+
+int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
+{
+    struct acpi_rmrr_unit *rmrr;
+    int rc = 0;
+
+    list_for_each_entry(rmrr, &acpi_rmrr_units, list)
+    {
+        rc = func(PFN_DOWN(rmrr->base_address),
+                  PFN_UP(rmrr->end_address) - PFN_DOWN(rmrr->base_address),
+                  ctxt);
+        if ( rc )
+            break;
+    }
+
+    return rc;
+}

xen/drivers/passthrough/vtd/extern.h +1 -0

index 5524dba..f9ee9b0 100644
@@ -75,6 +75,7 @@ int domain_context_mapping_one(struct domain *domain, struct iommu *iommu,
                                u8 bus, u8 devfn, const struct pci_dev *);
 int domain_context_unmap_one(struct domain *domain, struct iommu *iommu,
                              u8 bus, u8 devfn);
+int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt);
 
 unsigned int io_apic_read_remap_rte(unsigned int apic, unsigned int reg);
 void io_apic_write_remap_rte(unsigned int apic,

xen/drivers/passthrough/vtd/iommu.c +1 -0

index 19d8165..a38f201 100644
@@ -2491,6 +2491,7 @@ const struct iommu_ops intel_iommu_ops = {
     .crash_shutdown = vtd_crash_shutdown,
     .iotlb_flush = intel_iommu_iotlb_flush,
     .iotlb_flush_all = intel_iommu_iotlb_flush_all,
+    .get_reserved_device_memory = intel_iommu_get_reserved_device_memory,
     .dump_p2m_table = vtd_dump_p2m_table,
 };
 

xen/include/public/memory.h +23 -1

index 595f953..cee4535 100644
@@ -572,7 +572,29 @@ struct xen_vnuma_topology_info {
 typedef struct xen_vnuma_topology_info xen_vnuma_topology_info_t;
 DEFINE_XEN_GUEST_HANDLE(xen_vnuma_topology_info_t);
 
-/* Next available subop number is 27 */
+/*
+ * For legacy reasons, some devices must be configured with special memory
+ * regions to function correctly.  The guest must avoid using any of these
+ * regions.
+ */
+#define XENMEM_reserved_device_memory_map   27
+struct xen_reserved_device_memory {
+    xen_pfn_t start_pfn;
+    xen_ulong_t nr_pages;
+};
+typedef struct xen_reserved_device_memory xen_reserved_device_memory_t;
+DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_t);
+
+struct xen_reserved_device_memory_map {
+    /* IN/OUT */
+    unsigned int nr_entries;
+    /* OUT */
+    XEN_GUEST_HANDLE(xen_reserved_device_memory_t) buffer;
+};
+typedef struct xen_reserved_device_memory_map xen_reserved_device_memory_map_t;
+DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
+
+/* Next available subop number is 28 */
 
 #endif /* __XEN_PUBLIC_MEMORY_H__ */
 

xen/include/xen/iommu.h +4 -0

index 8eb764a..409f6f8 100644
@@ -120,6 +120,8 @@ void iommu_dt_domain_destroy(struct domain *d);
 
 struct page_info;
 
+typedef int iommu_grdm_t(xen_pfn_t start, xen_ulong_t nr, void *ctxt);
+
 struct iommu_ops {
     int (*init)(struct domain *d);
     void (*hwdom_init)(struct domain *d);

@@ -156,12 +158,14 @@ struct iommu_ops {
     void (*crash_shutdown)(void);
     void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int page_count);
     void (*iotlb_flush_all)(struct domain *d);
+    int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
     void (*dump_p2m_table)(struct domain *d);
 };
 
 void iommu_suspend(void);
 void iommu_resume(void);
 void iommu_crash_shutdown(void);
+int iommu_get_reserved_device_memory(iommu_grdm_t *, void *);
 
 void iommu_share_p2m_table(struct domain *d);
 

xen/include/xlat.lst +2 -1

index 41b3e35..42229fd 100644
@@ -61,9 +61,10 @@
 !	memory_exchange			memory.h
 !	memory_map			memory.h
 !	memory_reservation		memory.h
-?	mem_access_op		memory.h
+?	mem_access_op			memory.h
 !	pod_target			memory.h
 !	remove_from_physmap		memory.h
+!	reserved_device_memory_map	memory.h
 ?	physdev_eoi			physdev.h
 ?	physdev_get_free_pirq		physdev.h
 ?	physdev_irq			physdev.h

Signature · 1 line

1.9.1

Quoted 9 lines · Tiejun Chen

From: Jan Beulich <[email protected]>

This is a prerequisite for punching holes into HVM and PVH guests' P2M to allow passing through devices that are associated with (on VT-d) RMRRs.

Signed-off-by Jan Beulich <[email protected]>

Acked-by Kevin Tian <[email protected]>

---

Quoted 4 lines

(quoted diff) +0 -0

@@ -1101,6 +1129,29 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
          break;
      }
 

xen/common/memory.c long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) · line 1140

Quoted 9 lines

xen/common/memory.c +9 -0

+#ifdef HAS_PASSTHROUGH
+    case XENMEM_reserved_device_memory_map:
+    {
+        struct get_reserved_device_memory grdm;
+
+        if ( copy_from_guest(&grdm.map, arg, 1) ||
+             !guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
+            return -EFAULT;
+

Shouldn't there be an XSM check here?

xen/common/compat/memory.c int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) · line 319

Quoted 5 lines

xen/common/compat/memory.c +4 -0

+        grdm.used_entries = 0;
+        rc = iommu_get_reserved_device_memory(get_reserved_device_memory,
+                                              &grdm);
+

Also since we doing an iteration over possible many nr_entries should we think about returning -EAGAIN to user-space so that it can retry?

(As in, have preemption baked in this hypercall)

Quoted 2 lines

(quoted diff) +2 -0

+        if ( !rc && grdm.map.nr_entries < grdm.used_entries )
+            rc = -ENOBUFS;

Quoted 163 lines

xen/common/memory.c +8 -0

+        grdm.map.nr_entries = grdm.used_entries;
+        if ( __copy_to_guest(arg, &grdm.map, 1) )
+            rc = -EFAULT;
+
+        break;
+    }
+#endif
+
      default:
          rc = arch_memory_op(cmd, arg);
          break;

xen/drivers/passthrough/iommu.c +10 -0

index cc12735..7c17e8d 100644
@@ -344,6 +344,16 @@ void iommu_crash_shutdown(void)
      iommu_enabled = iommu_intremap = 0;
  }
 
+int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
+{
+    const struct iommu_ops *ops = iommu_get_ops();
+
+    if ( !iommu_enabled || !ops->get_reserved_device_memory )
+        return 0;
+
+    return ops->get_reserved_device_memory(func, ctxt);
+}
+
  bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature)
  {
      const struct hvm_iommu *hd = domain_hvm_iommu(d);

xen/drivers/passthrough/vtd/dmar.c +17 -0

index 5e41e7a..86cfad3 100644
@@ -901,3 +901,20 @@ int platform_supports_x2apic(void)
      unsigned int mask = ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT;
      return cpu_has_x2apic && ((dmar_flags & mask) == ACPI_DMAR_INTR_REMAP);
  }
+
+int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
+{
+    struct acpi_rmrr_unit *rmrr;
+    int rc = 0;
+
+    list_for_each_entry(rmrr, &acpi_rmrr_units, list)
+    {
+        rc = func(PFN_DOWN(rmrr->base_address),
+                  PFN_UP(rmrr->end_address) - PFN_DOWN(rmrr->base_address),
+                  ctxt);
+        if ( rc )
+            break;
+    }
+
+    return rc;
+}

xen/drivers/passthrough/vtd/extern.h +1 -0

index 5524dba..f9ee9b0 100644
@@ -75,6 +75,7 @@ int domain_context_mapping_one(struct domain *domain, struct iommu *iommu,
                                 u8 bus, u8 devfn, const struct pci_dev *);
  int domain_context_unmap_one(struct domain *domain, struct iommu *iommu,
                               u8 bus, u8 devfn);
+int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt);
 
  unsigned int io_apic_read_remap_rte(unsigned int apic, unsigned int reg);
  void io_apic_write_remap_rte(unsigned int apic,

xen/drivers/passthrough/vtd/iommu.c +1 -0

index 19d8165..a38f201 100644
@@ -2491,6 +2491,7 @@ const struct iommu_ops intel_iommu_ops = {
      .crash_shutdown = vtd_crash_shutdown,
      .iotlb_flush = intel_iommu_iotlb_flush,
      .iotlb_flush_all = intel_iommu_iotlb_flush_all,
+    .get_reserved_device_memory = intel_iommu_get_reserved_device_memory,
      .dump_p2m_table = vtd_dump_p2m_table,
  };
 

xen/include/public/memory.h +23 -1

index 595f953..cee4535 100644
@@ -572,7 +572,29 @@ struct xen_vnuma_topology_info {
  typedef struct xen_vnuma_topology_info xen_vnuma_topology_info_t;
  DEFINE_XEN_GUEST_HANDLE(xen_vnuma_topology_info_t);
 
-/* Next available subop number is 27 */
+/*
+ * For legacy reasons, some devices must be configured with special memory
+ * regions to function correctly.  The guest must avoid using any of these
+ * regions.
+ */
+#define XENMEM_reserved_device_memory_map   27
+struct xen_reserved_device_memory {
+    xen_pfn_t start_pfn;
+    xen_ulong_t nr_pages;
+};
+typedef struct xen_reserved_device_memory xen_reserved_device_memory_t;
+DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_t);
+
+struct xen_reserved_device_memory_map {
+    /* IN/OUT */
+    unsigned int nr_entries;
+    /* OUT */
+    XEN_GUEST_HANDLE(xen_reserved_device_memory_t) buffer;
+};
+typedef struct xen_reserved_device_memory_map xen_reserved_device_memory_map_t;
+DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
+
+/* Next available subop number is 28 */
 
  #endif /* __XEN_PUBLIC_MEMORY_H__ */
 

xen/include/xen/iommu.h +4 -0

index 8eb764a..409f6f8 100644
@@ -120,6 +120,8 @@ void iommu_dt_domain_destroy(struct domain *d);
 
  struct page_info;
 
+typedef int iommu_grdm_t(xen_pfn_t start, xen_ulong_t nr, void *ctxt);
+
  struct iommu_ops {
      int (*init)(struct domain *d);
      void (*hwdom_init)(struct domain *d);

@@ -156,12 +158,14 @@ struct iommu_ops {
      void (*crash_shutdown)(void);
      void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int page_count);
      void (*iotlb_flush_all)(struct domain *d);
+    int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
      void (*dump_p2m_table)(struct domain *d);
  };
 
  void iommu_suspend(void);
  void iommu_resume(void);
  void iommu_crash_shutdown(void);
+int iommu_get_reserved_device_memory(iommu_grdm_t *, void *);
 
  void iommu_share_p2m_table(struct domain *d);
 

xen/include/xlat.lst +2 -2

index 41b3e35..42229fd 100644
@@ -61,9 +61,10 @@
  !	memory_exchange			memory.h
  !	memory_map			memory.h
  !	memory_reservation		memory.h
-?	mem_access_op		memory.h
+?	mem_access_op			memory.h
  !	pod_target			memory.h
  !	remove_from_physmap		memory.h
+!	reserved_device_memory_map	memory.h
  ?	physdev_eoi			physdev.h
  ?	physdev_get_free_pirq		physdev.h
  ?	physdev_irq			physdev.h
--

1.9.1