Re: [PATCH v2 11/18] xen: get memory map from hypervisor for PVH

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:10PM +0200, Juergen Gross wrote:
> Retrieve the memory map from the hypervisor and normalize it to contain
> no overlapping entries and to be sorted by address.
>
> Signed-off-by: Juergen Gross <address@hidden>
> ---
>  grub-core/kern/i386/xen/pvh.c | 98 
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index c1b1cf8db..b4933b454 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -22,7 +22,16 @@
>  #include <grub/mm.h>
>  #include <grub/i386/cpuid.h>
>  #include <grub/xen.h>
> +#include <grub/i386/linux.h>
>  #include <grub/machine/kernel.h>
> +#include <xen/memory.h>
> +
> +struct xen_machine_mmap_entry
> +{
> +  grub_uint64_t addr;
> +  grub_uint64_t len;
> +  grub_uint32_t type;
> +} GRUB_PACKED;

Could not we reuse grub_e820_mmap_entry here?

>  grub_uint64_t grub_rsdp_addr;
>
> @@ -30,6 +39,8 @@ static struct { char _entry[32]; } hypercall_page[128]
>    __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct xen_machine_mmap_entry map[128];

A constant instead of 128? If no why 128?

> +static unsigned int nr_map_entries;
>
>  static void
>  grub_xen_early_halt (void)
> @@ -93,11 +104,98 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>    return __res;
>  }
>
> +static void
> +grub_xen_sort_mmap (void)
> +{
> +  grub_uint64_t from, to;
> +  unsigned int i;
> +  struct xen_machine_mmap_entry tmp;
> +
> +  /* Align map entries to page boundaries. */
> +  for (i = 0; i < nr_map_entries; i++)
> +    {
> +      from = map[i].addr;
> +      to = from + map[i].len;
> +      if (map[i].type == GRUB_MEMORY_AVAILABLE)
> +     {
> +       from = ALIGN_UP(from, GRUB_XEN_PAGE_SIZE);

Lack of space between macro name and "(".
Here and below...

> +       to = ALIGN_DOWN(to, GRUB_XEN_PAGE_SIZE);
> +     }
> +      else
> +     {
> +       from = ALIGN_DOWN(from, GRUB_XEN_PAGE_SIZE);
> +       to = ALIGN_UP(to, GRUB_XEN_PAGE_SIZE);
> +     }
> +      map[i].addr = from;
> +      map[i].len = to - from;
> +    }

[...]

Daniel