| Message ID | 1478018591-26425-1-git-send-email-wei.liu2@citrix.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 01, 2016 at 04:43:11PM +0000, Wei Liu wrote: > Introduce this field to control whether ACPI build is enabled by a > particular machine or accelerator. > > It defaults to true so that PC machine has ACPI build by default. Xen > accelerator will disable it because Xen is in charge of building ACPI > tables for the guest. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Sander Eikelenboom <linux@eikelenboom.it> > > Tested a backport version which only involves trivial code movement. It > worked with both -m xenfv and -m pc,accel=xen. > > Sander, if you want the backported patch please let me know. > --- > hw/i386/acpi-build.c | 2 +- > hw/i386/pc.c | 19 +++++++++++++++++++ > include/hw/i386/pc.h | 3 +++ > xen-common.c | 6 ++++++ > 4 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 93be96f..a5cd2fd 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2954,7 +2954,7 @@ void acpi_setup(void) > return; > } > > - if (!pcmc->has_acpi_build) { > + if (!pcmc->has_acpi_build || !pcms->acpi_build_enabled) { > ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n"); > return; > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f56ea0f..3e7982f 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2143,6 +2143,20 @@ static bool pc_machine_get_nvdimm(Object *obj, Error **errp) > return pcms->acpi_nvdimm_state.is_enabled; > } > > +static bool pc_machine_get_acpi_build(Object *obj, Error **errp) > +{ > + PCMachineState *pcms = PC_MACHINE(obj); > + > + return pcms->acpi_build_enabled; > +} > + > +static void pc_machine_set_acpi_build(Object *obj, bool value, Error **errp) > +{ > + PCMachineState *pcms = PC_MACHINE(obj); > + > + pcms->acpi_build_enabled = value; > +} > + > static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp) > { > PCMachineState *pcms = PC_MACHINE(obj); > @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj) > pcms->vmport = ON_OFF_AUTO_AUTO; > /* nvdimm is disabled on default. */ > pcms->acpi_nvdimm_state.is_enabled = false; > + /* acpi build is enabled by default. */ > + pcms->acpi_build_enabled = true; If you set: pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build; the default value will be more consistent with the actual results (where pc-1.6 and older don't have ACPI build). Then you would probably be able to remove the pcmc->has_acpi_build check from acpi_setup() and only check pcms->acpi_build_enabled. > } > > static void pc_machine_reset(void) > @@ -2319,6 +2335,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > > object_class_property_add_bool(oc, PC_MACHINE_NVDIMM, > pc_machine_get_nvdimm, pc_machine_set_nvdimm, &error_abort); > + > + object_class_property_add_bool(oc, PC_MACHINE_ACPI_BUILD, > + pc_machine_get_acpi_build, pc_machine_set_acpi_build, &error_abort); > } > > static const TypeInfo pc_machine_info = { > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 17fff80..ec8cd0c 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -63,6 +63,8 @@ struct PCMachineState { > > AcpiNVDIMMState acpi_nvdimm_state; > > + bool acpi_build_enabled; > + > /* RAM information (sizes, addresses, configuration): */ > ram_addr_t below_4g_mem_size, above_4g_mem_size; > > @@ -87,6 +89,7 @@ struct PCMachineState { > #define PC_MACHINE_VMPORT "vmport" > #define PC_MACHINE_SMM "smm" > #define PC_MACHINE_NVDIMM "nvdimm" > +#define PC_MACHINE_ACPI_BUILD "acpi-build" > > /** > * PCMachineClass: > diff --git a/xen-common.c b/xen-common.c > index e641ad1..b1858d7 100644 > --- a/xen-common.c > +++ b/xen-common.c > @@ -9,6 +9,7 @@ > */ > > #include "qemu/osdep.h" > +#include "hw/i386/pc.h" > #include "hw/xen/xen_backend.h" > #include "qmp-commands.h" > #include "sysemu/char.h" > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int running, > > static int xen_init(MachineState *ms) > { > + PCMachineState *pcms = PC_MACHINE(ms); > + > + /* Disable ACPI build because Xen handles it */ > + pcms->acpi_build_enabled = false; > + > xen_xc = xc_interface_open(0, 0, 0); > if (xen_xc == NULL) { > xen_be_printf(NULL, 0, "can't open xen interface\n"); > -- > 2.1.4 >
On Tue, Nov 01, 2016 at 04:43:11PM +0000, Wei Liu wrote: [...] > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int running, > > static int xen_init(MachineState *ms) > { > + PCMachineState *pcms = PC_MACHINE(ms); > + > + /* Disable ACPI build because Xen handles it */ > + pcms->acpi_build_enabled = false; I just noticed that I don't see any code to disable ACPI build in the case of "-machine pc,accel=xen". I suggest a xen_enabled() check in pc_init1() and pc_q35_init().
On Tue, Nov 01, 2016 at 03:02:31PM -0200, Eduardo Habkost wrote: > On Tue, Nov 01, 2016 at 04:43:11PM +0000, Wei Liu wrote: > [...] > > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int running, > > > > static int xen_init(MachineState *ms) > > { > > + PCMachineState *pcms = PC_MACHINE(ms); > > + > > + /* Disable ACPI build because Xen handles it */ > > + pcms->acpi_build_enabled = false; > > I just noticed that I don't see any code to disable ACPI build in > the case of "-machine pc,accel=xen". I suggest a xen_enabled() Hmm... I think this code snippet does exactly that -- xen_init is the initialization function for Xen accelerator. So this covers -m xenfv (because it sets accelerator to xen) and -m whatever,accel=xen. Did I miss anything? Wei.
On Tue, Nov 01, 2016 at 05:12:37PM +0000, Wei Liu wrote: > On Tue, Nov 01, 2016 at 03:02:31PM -0200, Eduardo Habkost wrote: > > On Tue, Nov 01, 2016 at 04:43:11PM +0000, Wei Liu wrote: > > [...] > > > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int running, > > > > > > static int xen_init(MachineState *ms) > > > { > > > + PCMachineState *pcms = PC_MACHINE(ms); > > > + > > > + /* Disable ACPI build because Xen handles it */ > > > + pcms->acpi_build_enabled = false; > > > > I just noticed that I don't see any code to disable ACPI build in > > the case of "-machine pc,accel=xen". I suggest a xen_enabled() > > Hmm... I think this code snippet does exactly that -- xen_init is the > initialization function for Xen accelerator. So this covers -m xenfv > (because it sets accelerator to xen) and -m whatever,accel=xen. > > Did I miss anything? Nevermind, I confused it with pc_xen_hvm_init() (which is specific for xenfv). The patch looks correct.
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 93be96f..a5cd2fd 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2954,7 +2954,7 @@ void acpi_setup(void) return; } - if (!pcmc->has_acpi_build) { + if (!pcmc->has_acpi_build || !pcms->acpi_build_enabled) { ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n"); return; } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f56ea0f..3e7982f 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2143,6 +2143,20 @@ static bool pc_machine_get_nvdimm(Object *obj, Error **errp) return pcms->acpi_nvdimm_state.is_enabled; } +static bool pc_machine_get_acpi_build(Object *obj, Error **errp) +{ + PCMachineState *pcms = PC_MACHINE(obj); + + return pcms->acpi_build_enabled; +} + +static void pc_machine_set_acpi_build(Object *obj, bool value, Error **errp) +{ + PCMachineState *pcms = PC_MACHINE(obj); + + pcms->acpi_build_enabled = value; +} + static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj) pcms->vmport = ON_OFF_AUTO_AUTO; /* nvdimm is disabled on default. */ pcms->acpi_nvdimm_state.is_enabled = false; + /* acpi build is enabled by default. */ + pcms->acpi_build_enabled = true; } static void pc_machine_reset(void) @@ -2319,6 +2335,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) object_class_property_add_bool(oc, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm, pc_machine_set_nvdimm, &error_abort); + + object_class_property_add_bool(oc, PC_MACHINE_ACPI_BUILD, + pc_machine_get_acpi_build, pc_machine_set_acpi_build, &error_abort); } static const TypeInfo pc_machine_info = { diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 17fff80..ec8cd0c 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -63,6 +63,8 @@ struct PCMachineState { AcpiNVDIMMState acpi_nvdimm_state; + bool acpi_build_enabled; + /* RAM information (sizes, addresses, configuration): */ ram_addr_t below_4g_mem_size, above_4g_mem_size; @@ -87,6 +89,7 @@ struct PCMachineState { #define PC_MACHINE_VMPORT "vmport" #define PC_MACHINE_SMM "smm" #define PC_MACHINE_NVDIMM "nvdimm" +#define PC_MACHINE_ACPI_BUILD "acpi-build" /** * PCMachineClass: diff --git a/xen-common.c b/xen-common.c index e641ad1..b1858d7 100644 --- a/xen-common.c +++ b/xen-common.c @@ -9,6 +9,7 @@ */ #include "qemu/osdep.h" +#include "hw/i386/pc.h" #include "hw/xen/xen_backend.h" #include "qmp-commands.h" #include "sysemu/char.h" @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int running, static int xen_init(MachineState *ms) { + PCMachineState *pcms = PC_MACHINE(ms); + + /* Disable ACPI build because Xen handles it */ + pcms->acpi_build_enabled = false; + xen_xc = xc_interface_open(0, 0, 0); if (xen_xc == NULL) { xen_be_printf(NULL, 0, "can't open xen interface\n");
Introduce this field to control whether ACPI build is enabled by a particular machine or accelerator. It defaults to true so that PC machine has ACPI build by default. Xen accelerator will disable it because Xen is in charge of building ACPI tables for the guest. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Igor Mammedov <imammedo@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Anthony PERARD <anthony.perard@citrix.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Sander Eikelenboom <linux@eikelenboom.it> Tested a backport version which only involves trivial code movement. It worked with both -m xenfv and -m pc,accel=xen. Sander, if you want the backported patch please let me know. --- hw/i386/acpi-build.c | 2 +- hw/i386/pc.c | 19 +++++++++++++++++++ include/hw/i386/pc.h | 3 +++ xen-common.c | 6 ++++++ 4 files changed, 29 insertions(+), 1 deletion(-)