Re: [RFC PATCH 4/9] hvm: Introduce "fixed memory layout" feature

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.08.2025 17:25, Teddy Astie wrote:
> @@ -686,10 +691,31 @@ static int domain_construct_memmap(libxl__gc *gc,
>      /* We always own at least one lowmem entry. */
>      unsigned int e820_entries = 1;
>      struct e820entry *e820 = NULL;
> +    uint64_t highmem_start = ((uint64_t)1 << 32);
>      uint64_t highmem_size =
>                      dom->highmem_end ? dom->highmem_end - (1ull << 32) : 0;
>      uint32_t lowmem_start = dom->device_model ? GUEST_LOW_MEM_START_DEFAULT 
> : 0;
>      unsigned page_size = XC_DOM_PAGE_SIZE(dom);
> +    /* Special region starts at the first 1G boundary after the highmem */
> +    uint64_t special_region_start =
> +        (highmem_start + highmem_size + GB(1) - 1) & ~(GB(1) - 1);

That is, inaccessible before entering PAE mode?

The open-coding of ROUNDUP() also isn't nice, but sadly unavoidable as long
the the macro works in terms of unsigned long.

> @@ -769,6 +805,40 @@ static int domain_construct_memmap(libxl__gc *gc,
>          e820[nr].type = E820_RAM;
>      }
>  
> +    /* Special regions */
> +    if (libxl_defbool_val(d_config->b_info.arch_x86.fixed_mem_layout))
> +    {
> +        e820[nr].type = XEN_HVM_MEMMAP_TYPE_SHARED_INFO;
> +        e820[nr].addr = special_region_offset;
> +        e820[nr].size = page_size;
> +        special_region_offset += e820[nr].size;
> +        nr++;
> +
> +        if ( gnttab_frame_count )
> +        {
> +            e820[nr].type = XEN_HVM_MEMMAP_TYPE_GRANT_TABLE;
> +            e820[nr].addr = special_region_offset;
> +            e820[nr].size = gnttab_frame_count * page_size;
> +            special_region_offset += e820[nr].size;
> +            nr++;
> +        }
> +
> +        if (d_config->b_info.max_grant_version >= 2 && 
> gnttab_status_frame_count)
> +        {
> +            e820[nr].type = XEN_HVM_MEMMAP_TYPE_GNTTAB_STATUS;
> +            e820[nr].addr = special_region_offset;
> +            e820[nr].size = gnttab_status_frame_count * page_size;
> +            special_region_offset += e820[nr].size;
> +            nr++;
> +        }
> +
> +        e820[nr].type = XEN_HVM_MEMMAP_TYPE_FOREIGN_REG;
> +        e820[nr].addr = special_region_offset;
> +        e820[nr].size = MB(512);

Can we really know this is going to be enough for all use cases?

> --- a/xen/include/public/arch-x86/hvm/start_info.h
> +++ b/xen/include/public/arch-x86/hvm/start_info.h
> @@ -99,6 +99,13 @@
>  #define XEN_HVM_MEMMAP_TYPE_DISABLED  6
>  #define XEN_HVM_MEMMAP_TYPE_PMEM      7
>  
> +/* Xen-specific types (OEM-specific range of the ACPI spec) */
> +#define XEN_HVM_MEMMAP_TYPE_SHARED_INFO   0xF0000001 /* Shared info page */
> +#define XEN_HVM_MEMMAP_TYPE_GRANT_TABLE   0xF0000002 /* Grant table pages */
> +#define XEN_HVM_MEMMAP_TYPE_GNTTAB_STATUS 0xF0000003 /* Grant table status 
> page (v2) */
> +#define XEN_HVM_MEMMAP_TYPE_FOREIGN_REG   0xF0000004 /* Suitable region for 
> grant mappings */
> +                                                     /* and foreign mappings 
> */

I question it being legitimate for us to introduce new E820 types.

Jan