Re: [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

Re: [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Stefano Stabellini]

CC'ing maintainers

On Tue, 25 Oct 2016, Wei Liu wrote:
> Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> building if running on Xen.
> 
> 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>

Hi Wei,
thanks for the patch. I think the right fix is to set

pcmc->has_acpi_build = false

for the xenfv machine and for the PC machine when accel=xen.

Thoughts?


> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> 
> RFC because I'm not sure this is the best way to fix it.
> ---
>  hw/i386/acpi-build.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a26a4bb..2cdff12 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -45,6 +45,7 @@
>  #include "sysemu/tpm_backend.h"
>  #include "hw/timer/mc146818rtc_regs.h"
>  #include "sysemu/numa.h"
> +#include "hw/xen/xen.h"
>  
>  /* Supported chipsets: */
>  #include "hw/acpi/piix4.h"
> @@ -2865,6 +2866,11 @@ void acpi_setup(void)
>          return;
>      }
>  
> +    if (xen_enabled()) {
> +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> +        return;
> +    }
> +
>      build_state = g_malloc0(sizeof *build_state);
>  
>      acpi_set_pci_info();
> -- 
> 2.1.4
> 

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

Re: [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Wei Liu]

On Tue, Oct 25, 2016 at 04:33:03PM -0700, Stefano Stabellini wrote:
> CC'ing maintainers
> 
> On Tue, 25 Oct 2016, Wei Liu wrote:
> > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > building if running on Xen.
> > 
> > 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>
> 
> Hi Wei,
> thanks for the patch. I think the right fix is to set
> 
> pcmc->has_acpi_build = false
> 
> for the xenfv machine and for the PC machine when accel=xen.
> 
> Thoughts?
> 

This sounds like a better idea to me.

Wei.

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

Re: [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Eduardo Habkost]

On Wed, Oct 26, 2016 at 11:32:56AM +0100, Wei Liu wrote:
> On Tue, Oct 25, 2016 at 04:33:03PM -0700, Stefano Stabellini wrote:
> > CC'ing maintainers
> > 
> > On Tue, 25 Oct 2016, Wei Liu wrote:
> > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > building if running on Xen.
> > > 
> > > 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>
> > 
> > Hi Wei,
> > thanks for the patch. I think the right fix is to set
> > 
> > pcmc->has_acpi_build = false
> > 
> > for the xenfv machine and for the PC machine when accel=xen.
> > 
> > Thoughts?
> > 
> 
> This sounds like a better idea to me.

As we shouldn't change PCMachineClass::has_acpi_build outside
class_init, to make accel=xen disable it you'll probably need a
new PCMachineState field.

-- 
Eduardo

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Igor Mammedov]

On Tue, 25 Oct 2016 18:28:04 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> building if running on Xen.
> 
> 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>
Question is:
Why does xen guest get ACPI tables from QEMU instead of using
Xen provided ones.
Maybe it's firmware issue i.e. firmware side shouldn't load
ACPI tables from QEMU provided fwcfg file and load Xen provided instead.

> ---
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> 
> RFC because I'm not sure this is the best way to fix it.
> ---
>  hw/i386/acpi-build.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a26a4bb..2cdff12 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -45,6 +45,7 @@
>  #include "sysemu/tpm_backend.h"
>  #include "hw/timer/mc146818rtc_regs.h"
>  #include "sysemu/numa.h"
> +#include "hw/xen/xen.h"
>  
>  /* Supported chipsets: */
>  #include "hw/acpi/piix4.h"
> @@ -2865,6 +2866,11 @@ void acpi_setup(void)
>          return;
>      }
>  
> +    if (xen_enabled()) {
> +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> +        return;
> +    }
> +
>      build_state = g_malloc0(sizeof *build_state);
>  
>      acpi_set_pci_info();


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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Wei Liu]

On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> On Tue, 25 Oct 2016 18:28:04 +0100
> Wei Liu <wei.liu2@citrix.com> wrote:
> 
> > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > building if running on Xen.
> > 
> > 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>
> Question is:
> Why does xen guest get ACPI tables from QEMU instead of using
> Xen provided ones.
> Maybe it's firmware issue i.e. firmware side shouldn't load
> ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> 

It hasn't come to the point that the guest is booted. QEMU exits when
trying to populate some pages for the guest, at which point the guest
has not yet been started.  In a sense, Xen guest doesn't get ACPI from
QEMU because it never gets to that point.

Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
defaults to true and acpi_enabled is also true. These make all checks in
acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
never going to be used by Xen guests), causing the guest to exceeds its
limit.

Wei.

> > ---
> > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > 
> > RFC because I'm not sure this is the best way to fix it.
> > ---
> >  hw/i386/acpi-build.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index a26a4bb..2cdff12 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -45,6 +45,7 @@
> >  #include "sysemu/tpm_backend.h"
> >  #include "hw/timer/mc146818rtc_regs.h"
> >  #include "sysemu/numa.h"
> > +#include "hw/xen/xen.h"
> >  
> >  /* Supported chipsets: */
> >  #include "hw/acpi/piix4.h"
> > @@ -2865,6 +2866,11 @@ void acpi_setup(void)
> >          return;
> >      }
> >  
> > +    if (xen_enabled()) {
> > +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> > +        return;
> > +    }
> > +
> >      build_state = g_malloc0(sizeof *build_state);
> >  
> >      acpi_set_pci_info();
> 

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Stefano Stabellini]

On Wed, 26 Oct 2016, Wei Liu wrote:
> On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > On Tue, 25 Oct 2016 18:28:04 +0100
> > Wei Liu <wei.liu2@citrix.com> wrote:
> > 
> > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > building if running on Xen.
> > > 
> > > 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>
> > Question is:
> > Why does xen guest get ACPI tables from QEMU instead of using
> > Xen provided ones.
> > Maybe it's firmware issue i.e. firmware side shouldn't load
> > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > 
> 
> It hasn't come to the point that the guest is booted. QEMU exits when
> trying to populate some pages for the guest, at which point the guest
> has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> QEMU because it never gets to that point.
> 
> Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> defaults to true and acpi_enabled is also true. These make all checks in
> acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> never going to be used by Xen guests), causing the guest to exceeds its
> limit.

What if libxl passes -no-acpi to QEMU?


> > > ---
> > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > 
> > > RFC because I'm not sure this is the best way to fix it.
> > > ---
> > >  hw/i386/acpi-build.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a26a4bb..2cdff12 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -45,6 +45,7 @@
> > >  #include "sysemu/tpm_backend.h"
> > >  #include "hw/timer/mc146818rtc_regs.h"
> > >  #include "sysemu/numa.h"
> > > +#include "hw/xen/xen.h"
> > >  
> > >  /* Supported chipsets: */
> > >  #include "hw/acpi/piix4.h"
> > > @@ -2865,6 +2866,11 @@ void acpi_setup(void)
> > >          return;
> > >      }
> > >  
> > > +    if (xen_enabled()) {
> > > +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> > > +        return;
> > > +    }
> > > +
> > >      build_state = g_malloc0(sizeof *build_state);
> > >  
> > >      acpi_set_pci_info();
> > 
> 

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Stefano Stabellini]

On Wed, 26 Oct 2016, Stefano Stabellini wrote:
> On Wed, 26 Oct 2016, Wei Liu wrote:
> > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > 
> > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > building if running on Xen.
> > > > 
> > > > 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>
> > > Question is:
> > > Why does xen guest get ACPI tables from QEMU instead of using
> > > Xen provided ones.
> > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > > 
> > 
> > It hasn't come to the point that the guest is booted. QEMU exits when
> > trying to populate some pages for the guest, at which point the guest
> > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > QEMU because it never gets to that point.
> > 
> > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > defaults to true and acpi_enabled is also true. These make all checks in
> > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > never going to be used by Xen guests), causing the guest to exceeds its
> > limit.
> 
> What if libxl passes -no-acpi to QEMU?

Of course even if that works, I think we should still fix this properly
in QEMU


> > > > ---
> > > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > 
> > > > RFC because I'm not sure this is the best way to fix it.
> > > > ---
> > > >  hw/i386/acpi-build.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index a26a4bb..2cdff12 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -45,6 +45,7 @@
> > > >  #include "sysemu/tpm_backend.h"
> > > >  #include "hw/timer/mc146818rtc_regs.h"
> > > >  #include "sysemu/numa.h"
> > > > +#include "hw/xen/xen.h"
> > > >  
> > > >  /* Supported chipsets: */
> > > >  #include "hw/acpi/piix4.h"
> > > > @@ -2865,6 +2866,11 @@ void acpi_setup(void)
> > > >          return;
> > > >      }
> > > >  
> > > > +    if (xen_enabled()) {
> > > > +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> > > > +        return;
> > > > +    }
> > > > +
> > > >      build_state = g_malloc0(sizeof *build_state);
> > > >  
> > > >      acpi_set_pci_info();
> > > 
> > 
> 

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Igor Mammedov]

On Wed, 26 Oct 2016 16:22:34 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > On Tue, 25 Oct 2016 18:28:04 +0100
> > Wei Liu <wei.liu2@citrix.com> wrote:
> >   
> > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > building if running on Xen.
> > > 
> > > 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>  
> > Question is:
> > Why does xen guest get ACPI tables from QEMU instead of using
> > Xen provided ones.
> > Maybe it's firmware issue i.e. firmware side shouldn't load
> > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> >   
> 
> It hasn't come to the point that the guest is booted. QEMU exits when
> trying to populate some pages for the guest, at which point the guest
> has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> QEMU because it never gets to that point.
> 
> Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> defaults to true and acpi_enabled is also true. These make all checks in
> acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> never going to be used by Xen guests), causing the guest to exceeds its
> limit.
> 
> Wei.
Would something like this fix issue for you?

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a54a468..61b6026 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -1094,10 +1094,13 @@ 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,


> 
> > > ---
> > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > 
> > > RFC because I'm not sure this is the best way to fix it.
> > > ---
> > >  hw/i386/acpi-build.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a26a4bb..2cdff12 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -45,6 +45,7 @@
> > >  #include "sysemu/tpm_backend.h"
> > >  #include "hw/timer/mc146818rtc_regs.h"
> > >  #include "sysemu/numa.h"
> > > +#include "hw/xen/xen.h"
> > >  
> > >  /* Supported chipsets: */
> > >  #include "hw/acpi/piix4.h"
> > > @@ -2865,6 +2866,11 @@ void acpi_setup(void)
> > >          return;
> > >      }
> > >  
> > > +    if (xen_enabled()) {
> > > +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> > > +        return;
> > > +    }
> > > +
> > >      build_state = g_malloc0(sizeof *build_state);
> > >  
> > >      acpi_set_pci_info();  
> >   
> 


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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Wei Liu]

Cc Sander

On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote:
> On Wed, 26 Oct 2016 16:22:34 +0100
> Wei Liu <wei.liu2@citrix.com> wrote:
> 
> > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > Wei Liu <wei.liu2@citrix.com> wrote:
> > >   
> > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > building if running on Xen.
> > > > 
> > > > 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>  
> > > Question is:
> > > Why does xen guest get ACPI tables from QEMU instead of using
> > > Xen provided ones.
> > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > >   
> > 
> > It hasn't come to the point that the guest is booted. QEMU exits when
> > trying to populate some pages for the guest, at which point the guest
> > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > QEMU because it never gets to that point.
> > 
> > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > defaults to true and acpi_enabled is also true. These make all checks in
> > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > never going to be used by Xen guests), causing the guest to exceeds its
> > limit.
> > 
> > Wei.
> Would something like this fix issue for you?
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a54a468..61b6026 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -1094,10 +1094,13 @@ 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,
> 

Yes, it does.

(I have a similar patch in my queue already)

Wei.

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Wei Liu]

On Thu, Oct 27, 2016 at 12:03:42PM +0100, Wei Liu wrote:
> Cc Sander
> 
> On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote:
> > On Wed, 26 Oct 2016 16:22:34 +0100
> > Wei Liu <wei.liu2@citrix.com> wrote:
> > 
> > > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > >   
> > > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > > building if running on Xen.
> > > > > 
> > > > > 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>  
> > > > Question is:
> > > > Why does xen guest get ACPI tables from QEMU instead of using
> > > > Xen provided ones.
> > > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > > >   
> > > 
> > > It hasn't come to the point that the guest is booted. QEMU exits when
> > > trying to populate some pages for the guest, at which point the guest
> > > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > > QEMU because it never gets to that point.
> > > 
> > > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > > defaults to true and acpi_enabled is also true. These make all checks in
> > > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > > never going to be used by Xen guests), causing the guest to exceeds its
> > > limit.
> > > 
> > > Wei.
> > Would something like this fix issue for you?
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index a54a468..61b6026 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -1094,10 +1094,13 @@ 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,
> > 
> 
> Yes, it does.
> 
> (I have a similar patch in my queue already)
> 

Oh, the reason I didn't send it out is because Eduardo suggested we
should use a new field instead of setting has_acpi_build outside of
PCMachineClass init function.

I have actually gotten around to investigate this option and what is
needed to be done.

Wei.

> Wei.

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Igor Mammedov]

On Thu, 27 Oct 2016 12:10:58 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> On Thu, Oct 27, 2016 at 12:03:42PM +0100, Wei Liu wrote:
> > Cc Sander
> > 
> > On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote:  
> > > On Wed, 26 Oct 2016 16:22:34 +0100
> > > Wei Liu <wei.liu2@citrix.com> wrote:
> > >   
> > > > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:  
> > > > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > > >     
> > > > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > > > building if running on Xen.
> > > > > > 
> > > > > > 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>    
> > > > > Question is:
> > > > > Why does xen guest get ACPI tables from QEMU instead of using
> > > > > Xen provided ones.
> > > > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > > > >     
> > > > 
> > > > It hasn't come to the point that the guest is booted. QEMU exits when
> > > > trying to populate some pages for the guest, at which point the guest
> > > > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > > > QEMU because it never gets to that point.
> > > > 
> > > > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > > > defaults to true and acpi_enabled is also true. These make all checks in
> > > > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > > > never going to be used by Xen guests), causing the guest to exceeds its
> > > > limit.
> > > > 
> > > > Wei.  
> > > Would something like this fix issue for you?
> > > 
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index a54a468..61b6026 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -1094,10 +1094,13 @@ 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,
> > >   
> > 
> > Yes, it does.
> > 
> > (I have a similar patch in my queue already)
> >   
> 
> Oh, the reason I didn't send it out is because Eduardo suggested we
> should use a new field instead of setting has_acpi_build outside of
> PCMachineClass init function.
> 
> I have actually gotten around to investigate this option and what is
> needed to be done.
static void xenfv_machine_options(MachineClass *m)                               
{                                                                                
...                                              
}                                                                                
                                                                                 
DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,                               
                  xenfv_machine_options);      

#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \                     
    static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \  
    { \                                                                          
        MachineClass *mc = MACHINE_CLASS(oc); \                                  
        optsfn(mc); \                                                            
        mc->init = initfn; \                                                     
    } \  
...

So xenfv_machine_options() is a part of pc_machine_xenfv_class_init()

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Wei Liu]

On Thu, Oct 27, 2016 at 01:26:49PM +0200, Igor Mammedov wrote:
> On Thu, 27 Oct 2016 12:10:58 +0100
> Wei Liu <wei.liu2@citrix.com> wrote:
> 
> > On Thu, Oct 27, 2016 at 12:03:42PM +0100, Wei Liu wrote:
> > > Cc Sander
> > > 
> > > On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote:  
> > > > On Wed, 26 Oct 2016 16:22:34 +0100
> > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > >   
> > > > > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:  
> > > > > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > > > >     
> > > > > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > > > > building if running on Xen.
> > > > > > > 
> > > > > > > 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>    
> > > > > > Question is:
> > > > > > Why does xen guest get ACPI tables from QEMU instead of using
> > > > > > Xen provided ones.
> > > > > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > > > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > > > > >     
> > > > > 
> > > > > It hasn't come to the point that the guest is booted. QEMU exits when
> > > > > trying to populate some pages for the guest, at which point the guest
> > > > > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > > > > QEMU because it never gets to that point.
> > > > > 
> > > > > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > > > > defaults to true and acpi_enabled is also true. These make all checks in
> > > > > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > > > > never going to be used by Xen guests), causing the guest to exceeds its
> > > > > limit.
> > > > > 
> > > > > Wei.  
> > > > Would something like this fix issue for you?
> > > > 
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index a54a468..61b6026 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -1094,10 +1094,13 @@ 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,
> > > >   
> > > 
> > > Yes, it does.
> > > 
> > > (I have a similar patch in my queue already)
> > >   
> > 
> > Oh, the reason I didn't send it out is because Eduardo suggested we
> > should use a new field instead of setting has_acpi_build outside of
> > PCMachineClass init function.
> > 
> > I have actually gotten around to investigate this option and what is
> > needed to be done.
> static void xenfv_machine_options(MachineClass *m)                               
> {                                                                                
> ...                                              
> }                                                                                
>                                                                                  
> DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,                               
>                   xenfv_machine_options);      
> 
> #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \                     
>     static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \  
>     { \                                                                          
>         MachineClass *mc = MACHINE_CLASS(oc); \                                  
>         optsfn(mc); \                                                            
>         mc->init = initfn; \                                                     
>     } \  
> ...
> 
> So xenfv_machine_options() is a part of pc_machine_xenfv_class_init()

Ah, so your (and mine) patch already fits the bill. Thanks for looking
into this.

Are you going to submit a proper patch or do you want me to?

Wei.

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

Re: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Igor Mammedov]

On Thu, 27 Oct 2016 12:29:58 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> On Thu, Oct 27, 2016 at 01:26:49PM +0200, Igor Mammedov wrote:
> > On Thu, 27 Oct 2016 12:10:58 +0100
> > Wei Liu <wei.liu2@citrix.com> wrote:
> >   
> > > On Thu, Oct 27, 2016 at 12:03:42PM +0100, Wei Liu wrote:  
> > > > Cc Sander
> > > > 
> > > > On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote:    
> > > > > On Wed, 26 Oct 2016 16:22:34 +0100
> > > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > > >     
> > > > > > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:    
> > > > > > > On Tue, 25 Oct 2016 18:28:04 +0100
> > > > > > > Wei Liu <wei.liu2@citrix.com> wrote:
> > > > > > >       
> > > > > > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > > > > > > building if running on Xen.
> > > > > > > > 
> > > > > > > > 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>      
> > > > > > > Question is:
> > > > > > > Why does xen guest get ACPI tables from QEMU instead of using
> > > > > > > Xen provided ones.
> > > > > > > Maybe it's firmware issue i.e. firmware side shouldn't load
> > > > > > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> > > > > > >       
> > > > > > 
> > > > > > It hasn't come to the point that the guest is booted. QEMU exits when
> > > > > > trying to populate some pages for the guest, at which point the guest
> > > > > > has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> > > > > > QEMU because it never gets to that point.
> > > > > > 
> > > > > > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> > > > > > defaults to true and acpi_enabled is also true. These make all checks in
> > > > > > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> > > > > > never going to be used by Xen guests), causing the guest to exceeds its
> > > > > > limit.
> > > > > > 
> > > > > > Wei.    
> > > > > Would something like this fix issue for you?
> > > > > 
> > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > > index a54a468..61b6026 100644
> > > > > --- a/hw/i386/pc_piix.c
> > > > > +++ b/hw/i386/pc_piix.c
> > > > > @@ -1094,10 +1094,13 @@ 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,
> > > > >     
> > > > 
> > > > Yes, it does.
> > > > 
> > > > (I have a similar patch in my queue already)
> > > >     
> > > 
> > > Oh, the reason I didn't send it out is because Eduardo suggested we
> > > should use a new field instead of setting has_acpi_build outside of
> > > PCMachineClass init function.
> > > 
> > > I have actually gotten around to investigate this option and what is
> > > needed to be done.  
> > static void xenfv_machine_options(MachineClass *m)                               
> > {                                                                                
> > ...                                              
> > }                                                                                
> >                                                                                  
> > DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,                               
> >                   xenfv_machine_options);      
> > 
> > #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \                     
> >     static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \  
> >     { \                                                                          
> >         MachineClass *mc = MACHINE_CLASS(oc); \                                  
> >         optsfn(mc); \                                                            
> >         mc->init = initfn; \                                                     
> >     } \  
> > ...
> > 
> > So xenfv_machine_options() is a part of pc_machine_xenfv_class_init()  
> 
> Ah, so your (and mine) patch already fits the bill. Thanks for looking
> into this.
> 
> Are you going to submit a proper patch or do you want me to?
Please submit your version of patch.

> 
> Wei.
> 
> _______________________________________________
> 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: [Qemu-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests

[Igor Mammedov]

On Wed, 26 Oct 2016 16:22:34 +0100
Wei Liu <wei.liu2@citrix.com> wrote:

> On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote:
> > On Tue, 25 Oct 2016 18:28:04 +0100
> > Wei Liu <wei.liu2@citrix.com> wrote:
> >   
> > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table
> > > building if running on Xen.
> > > 
> > > 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>  
> > Question is:
> > Why does xen guest get ACPI tables from QEMU instead of using
> > Xen provided ones.
> > Maybe it's firmware issue i.e. firmware side shouldn't load
> > ACPI tables from QEMU provided fwcfg file and load Xen provided instead.
> >   
> 
> It hasn't come to the point that the guest is booted. QEMU exits when
> trying to populate some pages for the guest, at which point the guest
> has not yet been started.  In a sense, Xen guest doesn't get ACPI from
> QEMU because it never gets to that point.
> 
> Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build
> defaults to true and acpi_enabled is also true. These make all checks in
> acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are
> never going to be used by Xen guests), causing the guest to exceeds its
> limit.
> 
> Wei.
would something like this work for you?

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a54a468..61b6026 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -1094,10 +1094,13 @@ 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,

> 
> > > ---
> > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > 
> > > RFC because I'm not sure this is the best way to fix it.
> > > ---
> > >  hw/i386/acpi-build.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a26a4bb..2cdff12 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -45,6 +45,7 @@
> > >  #include "sysemu/tpm_backend.h"
> > >  #include "hw/timer/mc146818rtc_regs.h"
> > >  #include "sysemu/numa.h"
> > > +#include "hw/xen/xen.h"
> > >  
> > >  /* Supported chipsets: */
> > >  #include "hw/acpi/piix4.h"
> > > @@ -2865,6 +2866,11 @@ void acpi_setup(void)
> > >          return;
> > >      }
> > >  
> > > +    if (xen_enabled()) {
> > > +        ACPI_BUILD_DPRINTF("Xen enabled. Bailing out.\n");
> > > +        return;
> > > +    }
> > > +
> > >      build_state = g_malloc0(sizeof *build_state);
> > >  
> > >      acpi_set_pci_info();  
> >   
> 


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