Re: [Xen-devel] [PATCH v2 04/17] libxl/arm: prepare for constructing ACPI tables
Hi Shannon, On 23/06/16 04:16, Shannon Zhao wrote: [...] diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index cc5a717..f5db74b 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -1,6 +1,7 @@ #include "libxl_internal.h" #include "libxl_arch.h" #include "libxl_libfdt_compat.h" +#include "libxl_arm_acpi.h" #include #include @@ -888,8 +889,24 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, libxl__domain_build_state *state, struct xc_dom_image *dom) { +int rc; + assert(info->type == LIBXL_DOMAIN_TYPE_PV); -return libxl__prepare_dtb(gc, info, state, dom); +rc = libxl__prepare_dtb(gc, info, state, dom); +if (rc) +return rc; + +if (!state->config.acpi) { +LOG(DEBUG, "Generating ACPI tables is disabled by user."); +return 0; +} + +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) { +LOG(DEBUG, "Do not generate ACPI tables for %s", dom->guest_type); +state->config.acpi = false; Silently ignores the user configuration is usually not a good thing. So should not we return an error here? +} + +return libxl__prepare_acpi(gc, info, state, dom); } static void finalise_one_memory_node(libxl__gc *gc, void *fdt, diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c new file mode 100644 index 000..60c31f9 --- /dev/null +++ b/tools/libxl/libxl_arm_acpi.c @@ -0,0 +1,85 @@ +/* + * ARM DmoU ACPI generation s/DmoU/DomU/ [...] +#include "libxl_arm_acpi.h" + +#include Please add a newline here for clarity. +typedef uint8_t u8; +typedef uint16_t u16; +typedef uint32_t u32; +typedef uint64_t u64; Ditto. +#include +#include + +enum { +RSDP, +XSDT, +GTDT, +MADT, +FADT, +DSDT, +NUMS, +}; + +struct acpitable { +void *table; +size_t size; +}; + +static struct acpitable acpitables[NUMS]; libxl is a library which can be used to implement daemon. So anything domain specific should not be static nor global. + +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, +libxl__domain_build_state *state, +struct xc_dom_image *dom) +{ +const libxl_version_info *vers; +int rc; + +/* convenience aliases */ +xc_domain_configuration_t *xc_config = &state->config; + +vers = libxl_get_version_info(CTX); +if (vers == NULL) +return ERROR_FAIL; + +LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d guest", +vers->xen_version_major, vers->xen_version_minor); + +dom->acpitable_blob = NULL; +dom->acpitable_size = 0; + +return 0; +} + +/* + * Local variables: + * mode: C + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/tools/libxl/libxl_arm_acpi.h b/tools/libxl/libxl_arm_acpi.h new file mode 100644 index 000..5899210 --- /dev/null +++ b/tools/libxl/libxl_arm_acpi.h @@ -0,0 +1,32 @@ [...] +#include "libxl_internal.h" +#include "libxl_arch.h" + +#include + +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, The function should be prefixed with _hidden. +libxl__domain_build_state *state, +struct xc_dom_image *dom); + +/* + * Local variables: + * mode: C + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/17] libxl/arm: prepare for constructing ACPI tables
On Thu, 23 Jun 2016, Shannon Zhao wrote: > On 2016年06月23日 21:37, Stefano Stabellini wrote: > > On Thu, 23 Jun 2016, Shannon Zhao wrote: > >> + > >> +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) { > >> +LOG(DEBUG, "Do not generate ACPI tables for %s", dom->guest_type); > >> +state->config.acpi = false; > > > > Shouldn't we return here? > > > Ah, right, thanks! > > > >> +} > >> + > >> +return libxl__prepare_acpi(gc, info, state, dom); > >> } > >> > >> static void finalise_one_memory_node(libxl__gc *gc, void *fdt, > >> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c > >> new file mode 100644 > >> index 000..60c31f9 > >> --- /dev/null > >> +++ b/tools/libxl/libxl_arm_acpi.c > >> @@ -0,0 +1,85 @@ > >> +/* > >> + * ARM DmoU ACPI generation > >> + * > >> + * Copyright (C) 2008-2010 Kevin O'Connor > >> + * Copyright (C) 2006 Fabrice Bellard > >> + * Copyright (C) 2013 Red Hat Inc > >> + * > >> + * Author: Michael S. Tsirkin > >> + * > >> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. > >> + * > >> + * Author: Shannon Zhao > >> + * > >> + * Copyright (C) 2016 Linaro Ltd. > >> + * > >> + * Author: Shannon Zhao > > > > Uhm... If in doubt just remove all the Author lines: git log provides > > more than enough information about who wrote this code. > > > Ahm... Julien suggested I should import the copyright from QEMU since I > refer to that. While what I didn't say before is that even if I didn't > refer to QEMU codes, the implementation will be like this because for > the tables except DSDT we just fill in the contents of the tables. But > for DSDT generation, it's totally different compared with QEMU. > > I think I'll remove this. :) I see. It's important to keep all the right signed-off-by, acked-by and reviewed-by in the commit message. But the author list on source files doesn't make much sense these days.___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/17] libxl/arm: prepare for constructing ACPI tables
On 2016年06月23日 21:37, Stefano Stabellini wrote: > On Thu, 23 Jun 2016, Shannon Zhao wrote: >> + >> +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) { >> +LOG(DEBUG, "Do not generate ACPI tables for %s", dom->guest_type); >> +state->config.acpi = false; > > Shouldn't we return here? > Ah, right, thanks! > >> +} >> + >> +return libxl__prepare_acpi(gc, info, state, dom); >> } >> >> static void finalise_one_memory_node(libxl__gc *gc, void *fdt, >> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c >> new file mode 100644 >> index 000..60c31f9 >> --- /dev/null >> +++ b/tools/libxl/libxl_arm_acpi.c >> @@ -0,0 +1,85 @@ >> +/* >> + * ARM DmoU ACPI generation >> + * >> + * Copyright (C) 2008-2010 Kevin O'Connor >> + * Copyright (C) 2006 Fabrice Bellard >> + * Copyright (C) 2013 Red Hat Inc >> + * >> + * Author: Michael S. Tsirkin >> + * >> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. >> + * >> + * Author: Shannon Zhao >> + * >> + * Copyright (C) 2016 Linaro Ltd. >> + * >> + * Author: Shannon Zhao > > Uhm... If in doubt just remove all the Author lines: git log provides > more than enough information about who wrote this code. > Ahm... Julien suggested I should import the copyright from QEMU since I refer to that. While what I didn't say before is that even if I didn't refer to QEMU codes, the implementation will be like this because for the tables except DSDT we just fill in the contents of the tables. But for DSDT generation, it's totally different compared with QEMU. I think I'll remove this. :) Thanks, -- Shannon ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/17] libxl/arm: prepare for constructing ACPI tables
On Thu, 23 Jun 2016, Shannon Zhao wrote: > From: Shannon Zhao > > It only constructs the ACPI tables for 64-bit ARM DomU when user enables > acpi because 32-bit DomU doesn't support ACPI. > > Signed-off-by: Shannon Zhao > --- > tools/libxl/Makefile | 4 +++ > tools/libxl/libxl_arm.c | 19 +- > tools/libxl/libxl_arm_acpi.c | 85 > > tools/libxl/libxl_arm_acpi.h | 32 + > 4 files changed, 139 insertions(+), 1 deletion(-) > create mode 100644 tools/libxl/libxl_arm_acpi.c > create mode 100644 tools/libxl/libxl_arm_acpi.h > > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index 9fee752..264b6ef 100644 > --- a/tools/libxl/Makefile > +++ b/tools/libxl/Makefile > @@ -77,6 +77,10 @@ endif > > LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o > LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o > +LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o > + > +libxl_arm_acpi.o: libxl_arm_acpi.c > + $(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c > > ifeq ($(CONFIG_NetBSD),y) > LIBXL_OBJS-y += libxl_netbsd.o > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index cc5a717..f5db74b 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -1,6 +1,7 @@ > #include "libxl_internal.h" > #include "libxl_arch.h" > #include "libxl_libfdt_compat.h" > +#include "libxl_arm_acpi.h" > > #include > #include > @@ -888,8 +889,24 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, > libxl__domain_build_state *state, > struct xc_dom_image *dom) > { > +int rc; > + > assert(info->type == LIBXL_DOMAIN_TYPE_PV); > -return libxl__prepare_dtb(gc, info, state, dom); > +rc = libxl__prepare_dtb(gc, info, state, dom); > +if (rc) > +return rc; > + > +if (!state->config.acpi) { > +LOG(DEBUG, "Generating ACPI tables is disabled by user."); > +return 0; > +} > + > +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) { > +LOG(DEBUG, "Do not generate ACPI tables for %s", dom->guest_type); > +state->config.acpi = false; Shouldn't we return here? > +} > + > +return libxl__prepare_acpi(gc, info, state, dom); > } > > static void finalise_one_memory_node(libxl__gc *gc, void *fdt, > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c > new file mode 100644 > index 000..60c31f9 > --- /dev/null > +++ b/tools/libxl/libxl_arm_acpi.c > @@ -0,0 +1,85 @@ > +/* > + * ARM DmoU ACPI generation > + * > + * Copyright (C) 2008-2010 Kevin O'Connor > + * Copyright (C) 2006 Fabrice Bellard > + * Copyright (C) 2013 Red Hat Inc > + * > + * Author: Michael S. Tsirkin > + * > + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. > + * > + * Author: Shannon Zhao > + * > + * Copyright (C) 2016 Linaro Ltd. > + * > + * Author: Shannon Zhao Uhm... If in doubt just remove all the Author lines: git log provides more than enough information about who wrote this code. > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as published > + * by the Free Software Foundation; version 2.1 only. with the special > + * exception on linking described in file LICENSE. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU Lesser General Public License for more details. > + */ > + > +#include "libxl_arm_acpi.h" > + > +#include > +typedef uint8_t u8; > +typedef uint16_t u16; > +typedef uint32_t u32; > +typedef uint64_t u64; > +#include > +#include > + > +enum { > +RSDP, > +XSDT, > +GTDT, > +MADT, > +FADT, > +DSDT, > +NUMS, > +}; > + > +struct acpitable { > +void *table; > +size_t size; > +}; > + > +static struct acpitable acpitables[NUMS]; > + > +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, > +libxl__domain_build_state *state, > +struct xc_dom_image *dom) > +{ > +const libxl_version_info *vers; > +int rc; > + > +/* convenience aliases */ > +xc_domain_configuration_t *xc_config = &state->config; > + > +vers = libxl_get_version_info(CTX); > +if (vers == NULL) > +return ERROR_FAIL; > + > +LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d guest", > +vers->xen_version_major, vers->xen_version_minor); > + > +dom->acpitable_blob = NULL; > +dom->acpitable_size = 0; > + > +return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/tools/libxl/libxl_arm_acpi.h b/tools/libxl/libxl_
[Xen-devel] [PATCH v2 04/17] libxl/arm: prepare for constructing ACPI tables
From: Shannon Zhao It only constructs the ACPI tables for 64-bit ARM DomU when user enables acpi because 32-bit DomU doesn't support ACPI. Signed-off-by: Shannon Zhao --- tools/libxl/Makefile | 4 +++ tools/libxl/libxl_arm.c | 19 +- tools/libxl/libxl_arm_acpi.c | 85 tools/libxl/libxl_arm_acpi.h | 32 + 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 tools/libxl/libxl_arm_acpi.c create mode 100644 tools/libxl/libxl_arm_acpi.h diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 9fee752..264b6ef 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -77,6 +77,10 @@ endif LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o +LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o + +libxl_arm_acpi.o: libxl_arm_acpi.c + $(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c ifeq ($(CONFIG_NetBSD),y) LIBXL_OBJS-y += libxl_netbsd.o diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index cc5a717..f5db74b 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -1,6 +1,7 @@ #include "libxl_internal.h" #include "libxl_arch.h" #include "libxl_libfdt_compat.h" +#include "libxl_arm_acpi.h" #include #include @@ -888,8 +889,24 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, libxl__domain_build_state *state, struct xc_dom_image *dom) { +int rc; + assert(info->type == LIBXL_DOMAIN_TYPE_PV); -return libxl__prepare_dtb(gc, info, state, dom); +rc = libxl__prepare_dtb(gc, info, state, dom); +if (rc) +return rc; + +if (!state->config.acpi) { +LOG(DEBUG, "Generating ACPI tables is disabled by user."); +return 0; +} + +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) { +LOG(DEBUG, "Do not generate ACPI tables for %s", dom->guest_type); +state->config.acpi = false; +} + +return libxl__prepare_acpi(gc, info, state, dom); } static void finalise_one_memory_node(libxl__gc *gc, void *fdt, diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c new file mode 100644 index 000..60c31f9 --- /dev/null +++ b/tools/libxl/libxl_arm_acpi.c @@ -0,0 +1,85 @@ +/* + * ARM DmoU ACPI generation + * + * Copyright (C) 2008-2010 Kevin O'Connor + * Copyright (C) 2006 Fabrice Bellard + * Copyright (C) 2013 Red Hat Inc + * + * Author: Michael S. Tsirkin + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. + * + * Author: Shannon Zhao + * + * Copyright (C) 2016 Linaro Ltd. + * + * Author: Shannon Zhao + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + */ + +#include "libxl_arm_acpi.h" + +#include +typedef uint8_t u8; +typedef uint16_t u16; +typedef uint32_t u32; +typedef uint64_t u64; +#include +#include + +enum { +RSDP, +XSDT, +GTDT, +MADT, +FADT, +DSDT, +NUMS, +}; + +struct acpitable { +void *table; +size_t size; +}; + +static struct acpitable acpitables[NUMS]; + +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, +libxl__domain_build_state *state, +struct xc_dom_image *dom) +{ +const libxl_version_info *vers; +int rc; + +/* convenience aliases */ +xc_domain_configuration_t *xc_config = &state->config; + +vers = libxl_get_version_info(CTX); +if (vers == NULL) +return ERROR_FAIL; + +LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d guest", +vers->xen_version_major, vers->xen_version_minor); + +dom->acpitable_blob = NULL; +dom->acpitable_size = 0; + +return 0; +} + +/* + * Local variables: + * mode: C + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/tools/libxl/libxl_arm_acpi.h b/tools/libxl/libxl_arm_acpi.h new file mode 100644 index 000..5899210 --- /dev/null +++ b/tools/libxl/libxl_arm_acpi.h @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2016 Linaro Ltd. + * + * Author: Shannon Zhao + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in th