[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC][v2][PATCH 04/14] tools/libxl: detect and avoid conflicts with RDM



On 2015/6/7 19:20, Wei Liu wrote:
On Wed, Jun 03, 2015 at 10:25:47AM +0800, Chen, Tiejun wrote:
[...]
+static struct xen_reserved_device_memory
+*xc_device_get_rdm(libxl__gc *gc,
+                   uint32_t flag,
+                   uint16_t seg,
+                   uint8_t bus,
+                   uint8_t devfn,
+                   unsigned int *nr_entries)
+{
+    struct xen_reserved_device_memory *xrdm = NULL;
+    int rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
+                                           xrdm, nr_entries);
+

Please separate declaration and function call. Also change xrdm to NULL

Are you saying this?

     struct xen_reserved_device_memory *xrdm = NULL;
     int rc;

     rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
                                        xrdm, nr_entries);

Yes, splitting "rc = " to a separate line. The other thing is:

      rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
                                         NULL, nr_entries);
                                        ^ here

It's mostly cosmetic. IMHO it is clearer than passing xrdm which is
always NULL in that function call.

Right.



in that function call.

Sorry, what do you mean by this point? Or you let me to change xrdm to NULL
inside xc_reserved_device_memory_map()?


+    assert( rc <= 0 );
+    /* "0" means we have no any rdm entry. */
+    if ( !rc )
+        goto out;

Also set *nr_entries = 0; otherwise you can't distinguish error vs 0
entries.

*nr_entries is always updated by xc_reserved_device_memory_map() above.


Actually no. If xc_hypercall_bounce_pre fails in the function,
nr_entries is untouched.

Sure.



+
+    if ( errno == ENOBUFS )
+    {
+        if ( (xrdm = malloc(*nr_entries *
+                            sizeof(xen_reserved_device_memory_t))) == NULL )
[...]
+    return -1;

Please return libxl error code.

ERROR_FAIL?


Yes, that's fine.

Thanks
Tiejun


[...]
+        args.mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
+    mmio_start = (1ull << 32) - args.mmio_size;
+
+    if (args.lowmem_size > mmio_start)
+        args.lowmem_size = mmio_start;
+
+    /*
+     * We'd like to set a memory boundary to determine if we need to check
+     * any overlap with reserved device memory.
+     */
+    rdm_mem_boundary = 0x80000000;
+    if (info->rdm_mem_boundary_memkb)


I'm going to update this chunk of codes as follows:

#1. @@ -858,6 +858,12 @@ const char *libxl_defbool_to_string(libxl_defbool
b);
  #define LIBXL_TIMER_MODE_DEFAULT -1
  #define LIBXL_MEMKB_DEFAULT ~0ULL

+/*
+ * We'd like to set a memory boundary to determine if we need to check
+ * any overlap with reserved device memory.
+ */
+#define LIBXL_RDM_MEM_BOUNDARY_MEMKB_DEFAULT (2048 * 1024)
+
  #define LIBXL_MS_VM_GENID_LEN 16
  typedef struct {
      uint8_t bytes[LIBXL_MS_VM_GENID_LEN];

I think you mean info->rdm_mem_boundary_memkb != LIBXL_MEMKB_DEFAULT?


#2.

@@ -109,6 +109,10 @@ void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info)
  {
      if (b_info->rdm.reserve == LIBXL_RDM_RESERVE_FLAG_INVALID)
          b_info->rdm.reserve = LIBXL_RDM_RESERVE_FLAG_RELAXED;
+
+    if (b_info->rdm_mem_boundary_memkb == LIBXL_MEMKB_DEFAULT)
+        b_info->rdm_mem_boundary_memkb =
+                            LIBXL_RDM_MEM_BOUNDARY_MEMKB_DEFAULT;

This looks plausible.

Wei.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.