Re: HVM support for e820_host (Was: Bug: Limitation of <=2GB RAM in domU persists with 4.3.0)

[Gordan Bobic <gordan@bobich.net>]

 On Tue, 3 Sep 2013 17:08:33 -0400, Konrad Rzeszutek Wilk 
 <konrad.wilk@oracle.com> wrote:

> You are missing the hypervisor patch to set the E820 for HVM guests.
> http://lists.xen.org/archives/html/xen-devel/2013-05/msg01603.html
>
> And that should make it possible to "stash" the E820 in the 
> hypervisor.

 Regarding Jan's comment on the thread here:
 http://lists.xen.org/archives/html/xen-devel/2013-05/msg01649.html

 Should this not instead of:
 ===
 @@ -595,7 +595,7 @@ void arch_domain_destroy(struct domain *d)
      if ( is_hvm_domain(d) )
          hvm_domain_destroy(d);
      else
 -        xfree(d->arch.pv_domain.e820);
 +        xfree(d->arch.e820);
 
      free_domain_pirqs(d);
      if ( !is_idle_domain(d) )
 ===

 be something like:

 ===
 @@ -595,7 +595,6 @@ void arch_domain_destroy(struct domain *d)
      if ( is_hvm_domain(d) )
          hvm_domain_destroy(d);
 -    else
 -        xfree(d->arch.pv_domain.e820);
 +        xfree(d->arch.e820);
 
      free_domain_pirqs(d);
      if ( !is_idle_domain(d) )
 ===

 The question I have is will d->arch.e820 always be there and set
 even with e820_host=0? Or does there need to be an extra check
 here?

> Then after that you will need to implement in the hvmloader.c the
> XENMEM_memory_map hypercall to get the E820 and do something with it.
>
>
> Oh, and something like this probably should do it - not compile 
> tested
> in any way:
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 1fcaed0..7b38890 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3146,6 +3146,7 @@ static long hvm_memory_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>      case XENMEM_machine_memory_map:
>      case XENMEM_machphys_mapping:
>          return -ENOSYS;
> +    case XENMEM_memory_map:
>      case XENMEM_decrease_reservation:
>          rc = do_memory_op(cmd, arg);
>          current->domain->arch.hvm_domain.qemu_mapcache_invalidate = 
> 1;
> @@ -3216,10 +3217,10 @@ static long hvm_memory_op_compat32(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>
>      switch ( cmd & MEMOP_CMD_MASK )
>      {
> -    case XENMEM_memory_map:
>      case XENMEM_machine_memory_map:
>      case XENMEM_machphys_mapping:
>          return -ENOSYS;
> +    case XENMEM_memory_map:
>      case XENMEM_decrease_reservation:
>          rc = compat_memory_op(cmd, arg);
>          current->domain->arch.hvm_domain.qemu_mapcache_invalidate = 
> 1;
>
> diff --git a/tools/firmware/hvmloader/e820.c
> b/tools/firmware/hvmloader/e820.c
> index 2e05e93..86fb20a 100644
> --- a/tools/firmware/hvmloader/e820.c
> +++ b/tools/firmware/hvmloader/e820.c
> @@ -68,16 +68,42 @@ void dump_e820_table(struct e820entry *e820,
> unsigned int nr)
>      }
>  }
>
> +static const char *e820_names(int type)
> +{
> +    switch (type) {
> +        case E820_RAM: return "RAM";
> +        case E820_RESERVED: return "Reserved";
> +        case E820_ACPI: return "ACPI";
> +        case E820_NVS: return "ACPI NVS";
> +        case E820_UNUSABLE: return "Unusable";
> +        default: break;
> +    }
> +    return "Unknown";
> +}
> +
> +
>  /* Create an E820 table based on memory parameters provided in 
> hvm_info. */
>  int build_e820_table(struct e820entry *e820,
>                       unsigned int lowmem_reserved_base,
>                       unsigned int bios_image_base)
>  {
>      unsigned int nr = 0;
> +    struct xen_memory_map op;
> +    struct e820entry map[E820MAX];
> +    int rc;
>
>      if ( !lowmem_reserved_base )
>              lowmem_reserved_base = 0xA0000;
>
> +    set_xen_guest_handle(op.buffer, map);
> +
> +    rc = hypercall_memory_op ( XENMEM_memory_op, &op);
> +    if ( rc != -ENOSYS) { /* It works!? */
> +        int i;
> +        for ( i = 0; i < op.nr_entries; i++ )
> +            printf("    %lx -> %lx %s\n", map[i].addr >> 12,
> +                   (map[i].addr + map[i].size) >> 12,
> e820_names(map[i].type));
> +    }
>      /* Lowmem must be at least 512K to keep Windows happy) */
>      ASSERT ( lowmem_reserved_base > 512<<10 );

 Thanks. :) Will try that when I've verified the first two
 patches (mine and Mukesh's) build cleanly in my 4.3.0 package
 build.

 Gordan