Re: [Qemu-devel] [PATCH v2] xenfv: set has_acpi_build to false

Re: [Qemu-devel] [PATCH v2] xenfv: set has_acpi_build to false

[Igor Mammedov]

On Thu, 27 Oct 2016 14:51:09 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> Xen's toolstack is in charge of building ACPI tables. Skip ACPI table
> building and loading in QEMU by setting has_acpi_build to false for
> xenfv machine.
> 
> This issue is discovered due to direct kernel boot on Xen doesn't boot
> anymore, because the new ACPI tables cause the guest to exceed its
> memory allocation limit.
> 
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> ---
>  hw/i386/pc_piix.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a54a468..08596a1 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -1094,10 +1094,14 @@ DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>  #ifdef CONFIG_XEN
>  static void xenfv_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      m->desc = "Xen Fully-virtualized PC";
>      m->max_cpus = HVM_MAX_VCPUS;
>      m->default_machine_opts = "accel=xen";
>      m->hot_add_cpu = pc_hot_add_cpu;
> +
> +    pcmc->has_acpi_build = false;
>  }
>  
>  DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] xenfv: set has_acpi_build to false

[Sander Eikelenboom]

Thursday, October 27, 2016, 3:51:09 PM, you wrote:

> Xen's toolstack is in charge of building ACPI tables. Skip ACPI table
> building and loading in QEMU by setting has_acpi_build to false for
> xenfv machine.

> This issue is discovered due to direct kernel boot on Xen doesn't boot
> anymore, because the new ACPI tables cause the guest to exceed its
> memory allocation limit.

> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Just given this patch a spin and you may add a:
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>

Thanks again!
--
Sander

> ---
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> ---
>  hw/i386/pc_piix.c | 4 ++++
>  1 file changed, 4 insertions(+)

> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a54a468..08596a1 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -1094,10 +1094,14 @@ DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>  #ifdef CONFIG_XEN
>  static void xenfv_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      m->desc = "Xen Fully-virtualized PC";
>      m->max_cpus = HVM_MAX_VCPUS;
>      m->default_machine_opts = "accel=xen";
>      m->hot_add_cpu = pc_hot_add_cpu;
> +
+    pcmc->>has_acpi_build = false;
>  }
>  
>  DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] xenfv: set has_acpi_build to false

[Stefano Stabellini]

On Thu, 27 Oct 2016, Sander Eikelenboom wrote:
> Thursday, October 27, 2016, 3:51:09 PM, you wrote:
> 
> > Xen's toolstack is in charge of building ACPI tables. Skip ACPI table
> > building and loading in QEMU by setting has_acpi_build to false for
> > xenfv machine.
> 
> > This issue is discovered due to direct kernel boot on Xen doesn't boot
> > anymore, because the new ACPI tables cause the guest to exceed its
> > memory allocation limit.
> 
> > Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Just given this patch a spin and you may add a:
> Tested-by: Sander Eikelenboom <linux@eikelenboom.it>

The problem with this patch is that it only covers the xenfv machine
case, which is default, but QEMU can also be invoked with -M
pc,accel=xen. That case wouldn't be fixed by this patch. Wei, you can
test it by adding "xen_platform_pci=0" to the VM config file.
That's why we probably need a new option, similar to has_acpi_build, but
that can be changed at accelerator init time.


> > ---
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Sander Eikelenboom <linux@eikelenboom.it>
> > ---
> >  hw/i386/pc_piix.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index a54a468..08596a1 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -1094,10 +1094,14 @@ DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
> >  #ifdef CONFIG_XEN
> >  static void xenfv_machine_options(MachineClass *m)
> >  {
> > +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > +
> >      m->desc = "Xen Fully-virtualized PC";
> >      m->max_cpus = HVM_MAX_VCPUS;
> >      m->default_machine_opts = "accel=xen";
> >      m->hot_add_cpu = pc_hot_add_cpu;
> > +
> +    pcmc->>has_acpi_build = false;
> >  }
> >  
> >  DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] xenfv: set has_acpi_build to false

[Wei Liu]

On Thu, Oct 27, 2016 at 11:58:29AM -0700, Stefano Stabellini wrote:
> On Thu, 27 Oct 2016, Sander Eikelenboom wrote:
> > Thursday, October 27, 2016, 3:51:09 PM, you wrote:
> > 
> > > Xen's toolstack is in charge of building ACPI tables. Skip ACPI table
> > > building and loading in QEMU by setting has_acpi_build to false for
> > > xenfv machine.
> > 
> > > This issue is discovered due to direct kernel boot on Xen doesn't boot
> > > anymore, because the new ACPI tables cause the guest to exceed its
> > > memory allocation limit.
> > 
> > > Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Just given this patch a spin and you may add a:
> > Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> 
> The problem with this patch is that it only covers the xenfv machine
> case, which is default, but QEMU can also be invoked with -M
> pc,accel=xen. That case wouldn't be fixed by this patch. Wei, you can
> test it by adding "xen_platform_pci=0" to the VM config file.
> That's why we probably need a new option, similar to has_acpi_build, but
> that can be changed at accelerator init time.
> 

Do you mean we should add a new field to AccelClass?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] xenfv: set has_acpi_build to false

[Stefano Stabellini]

On Fri, 28 Oct 2016, Wei Liu wrote:
> On Thu, Oct 27, 2016 at 11:58:29AM -0700, Stefano Stabellini wrote:
> > On Thu, 27 Oct 2016, Sander Eikelenboom wrote:
> > > Thursday, October 27, 2016, 3:51:09 PM, you wrote:
> > > 
> > > > Xen's toolstack is in charge of building ACPI tables. Skip ACPI table
> > > > building and loading in QEMU by setting has_acpi_build to false for
> > > > xenfv machine.
> > > 
> > > > This issue is discovered due to direct kernel boot on Xen doesn't boot
> > > > anymore, because the new ACPI tables cause the guest to exceed its
> > > > memory allocation limit.
> > > 
> > > > Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > 
> > > Just given this patch a spin and you may add a:
> > > Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> > 
> > The problem with this patch is that it only covers the xenfv machine
> > case, which is default, but QEMU can also be invoked with -M
> > pc,accel=xen. That case wouldn't be fixed by this patch. Wei, you can
> > test it by adding "xen_platform_pci=0" to the VM config file.
> > That's why we probably need a new option, similar to has_acpi_build, but
> > that can be changed at accelerator init time.
> > 
> 
> Do you mean we should add a new field to AccelClass?

I mean that this patch is insufficient unfortunately. I am not sure
about what is the best way to solve this problem, but Eduardo suggested
to add a new PCMachineState field:
http://marc.info/?l=qemu-devel&m=147749203112422&w=2

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel