[Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Julien Grall writes ("[for-4.10] Re: [Xen-devel] [PATCH v2] tools/libxl: mark 
special pages as reserved in e820 map for PVH"):
> I would recommend to tag your patch is 4.10 to help reviewers prioritize 
> review on your patch. I have done it now.

Thanks.  Looking at the thread, I would have liked to see an answer to
this comment by Roger:

| Albeit I would also prefer this to not be PVH specific. Ideally I
| would like both PVH and HVM to share the logic to mark the reserved
| regions in the memory map. I guess this can be fixed afterwards by
| moving away this logic from hvmloader and handling the creation of
| the memory map for both HVM and PVH in libxl.

But it seems to be a bugfix and has had review from the x86
perspective, so:

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>


If anyone is feeling up to doing some improvement, I would like to see
a rework of the algorithm to avoid this error-prone duplicated-
information construction:

  +    /* Add mmio entry for PVH. */
  +    if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH)
  +        e820_entries++;

  @@ -564,6 +567,14 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,

  +    /* mmio area */
  +    if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH) {
  +        e820[nr].addr = dom->mmio_start;
  +        e820[nr].size = dom->mmio_size;
  +        e820[nr].type = E820_RESERVED;
  +        nr++;
  +    }
  +

That is, there should be no separate pre-calculation of the number of
entries.  There would have to be an expanding array instead.

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel