Re: [PATCH for-5.0 v2] xen: fixup RAM memory region initialization

[Paolo Bonzini <pbonzini@redhat.com>]

On 02/04/20 16:54, Igor Mammedov wrote:
> Since bd457782b3b0 ("x86/pc: use memdev for RAM") Xen
> machine fails to start with:
>    qemu-system-i386: xen: failed to populate ram at 0
> 
> The reason is that xen_ram_alloc() which is called by
> memory_region_init_ram(), compares memory region with
> statically allocated 'global' ram_memory memory region
> that it uses for RAM, and does nothing in case it matches.
> 
> While it's possible feed machine->ram to xen_ram_alloc()
> in the same manner to keep that hack working, I'd prefer
> not to keep that circular dependency and try to untangle that.
> 
> However it doesn't look trivial to fix, so as temporary
> fixup opt out Xen machine from memdev based RAM allocation,
> and let xen_ram_alloc() do its trick for now.
> 
> Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   * make it work with -M pc,accel=xen
> ---
>  hw/xen/xen-common.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> index 15650d7f6a..a15070f7f6 100644
> --- a/hw/xen/xen-common.c
> +++ b/hw/xen/xen-common.c
> @@ -19,6 +19,7 @@
>  #include "sysemu/runstate.h"
>  #include "migration/misc.h"
>  #include "migration/global_state.h"
> +#include "hw/boards.h"
>  
>  //#define DEBUG_XEN
>  
> @@ -151,6 +152,8 @@ static void xen_setup_post(MachineState *ms, AccelState *accel)
>  
>  static int xen_init(MachineState *ms)
>  {
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
>      xen_xc = xc_interface_open(0, 0, 0);
>      if (xen_xc == NULL) {
>          xen_pv_printf(NULL, 0, "can't open xen interface\n");
> @@ -170,6 +173,10 @@ static int xen_init(MachineState *ms)
>          return -1;
>      }
>      qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
> +    /*
> +     * opt out of system RAM being allocated by generic code
> +     */
> +    mc->default_ram_id = NULL;
>      return 0;
>  }
>  
> 

Monkey patching the class is slightly disgusting. :)  It would be better
to use a new ms->default_ram_id, and assign it from the class in
instance_init, but I've queued it anyway.

Paolo