|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 11/17] libxl/arm: Construct ACPI DSDT table
On 2016/6/24 1:03, Julien Grall wrote:
> Hi Shannon,
>
> On 23/06/16 04:16, Shannon Zhao wrote:
>
> [...]
>
>> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
>> index 264b6ef..5347480 100644
>> --- a/tools/libxl/Makefile
>> +++ b/tools/libxl/Makefile
>> @@ -77,7 +77,29 @@ 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_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o libxl_dsdt_anycpu_arm.o
>> +
>> +vpath iasl $(PATH)
>> +libxl_mk_dsdt_arm: libxl_mk_dsdt_arm.c
>> + $(CC) $(CFLAGS) -o $@ libxl_mk_dsdt_arm.c
>> +
>> +libxl_dsdt_anycpu_arm.asl: libxl_empty_dsdt_arm.asl libxl_mk_dsdt_arm
>> + awk 'NR > 1 {print s} {s=$$0}' $< > $@
>> + ./libxl_mk_dsdt_arm >> $@
>> +
>> +libxl_dsdt_anycpu_arm.c: %.c: iasl %.asl
>> + iasl -vs -p $* -tc $*.asl
>> + sed -e 's/AmlCode/$*/g' $*.hex >$@
>> + echo "int $*_len=sizeof($*);" >>$@
>> + rm -f $*.aml $*.hex
>> +
>
> I don't like the idea to add iasl as a dependency for all ARM platforms.
> For instance ARMv7 platform will not use ACPI, but we still ask users to
> install iasl. So I think we should allow the user to opt-in/opt-out for
> ACPI.
>
> Any opinions?
>
I agree. But how to exclude for ARMv7. I notice it only has the option
CONFIG_ARM which doesn't distinguish ARM32 and ARM64.
>> +iasl:
>> + @echo
>> + @echo "ACPI ASL compiler (iasl) is needed"
>> + @echo "Download and install Intel ACPI CA from"
>> + @echo "http://acpica.org/downloads/";
>> + @echo
>> + @exit 1
>
> It is really a pain to discover the dependency in the middle of a build.
> The presence of iasl should be done by the configure.
>
>>
>> libxl_arm_acpi.o: libxl_arm_acpi.c
>> $(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>> index 353d774..45fc354 100644
>> --- a/tools/libxl/libxl_arm_acpi.c
>> +++ b/tools/libxl/libxl_arm_acpi.c
>> @@ -54,6 +54,9 @@ enum {
>> NUMS,
>> };
>>
>> +extern unsigned char libxl_dsdt_anycpu_arm[];
>> +extern int libxl_dsdt_anycpu_arm_len;
>
> Not sure this is the right place to mention it, but I don't find the
> actual declaration.
>
The declarations are in libxl_dsdt_anycpu_arm.c which is generated by
iasl. You can see that in the Makefile above.
> Both variables should be const and _hidden. Also, the *_len should be at
> least const int.
>
>> +
>> struct acpitable {
>> void *table;
>> size_t size;
>> @@ -256,6 +259,17 @@ static void make_acpi_fadt(libxl__gc *gc, struct
>> xc_dom_image *dom)
>> dom->acpitable_size += ROUNDUP(acpitables[FADT].size, 3);
>> }
>>
>> +static void make_acpi_dsdt(libxl__gc *gc, struct xc_dom_image *dom)
>> +{
>> + acpitables[DSDT].table = libxl__zalloc(gc,
>> libxl_dsdt_anycpu_arm_len);
>> + memcpy(acpitables[DSDT].table, libxl_dsdt_anycpu_arm,
>> + libxl_dsdt_anycpu_arm_len);
>> +
>> + acpitables[DSDT].size = libxl_dsdt_anycpu_arm_len;
>> + /* Align to 64bit. */
>> + dom->acpitable_size += ROUNDUP(acpitables[DSDT].size, 3);
>> +}
>> +
>> int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>> libxl__domain_build_state *state,
>> struct xc_dom_image *dom)
>> @@ -284,6 +298,7 @@ int libxl__prepare_acpi(libxl__gc *gc,
>> libxl_domain_build_info *info,
>> return rc;
>>
>> make_acpi_fadt(gc, dom);
>> + make_acpi_dsdt(gc, dom);
>>
>> return 0;
>> }
>> diff --git a/tools/libxl/libxl_arm_acpi.h b/tools/libxl/libxl_arm_acpi.h
>> index 9b58de6..b0fd9ce 100644
>> --- a/tools/libxl/libxl_arm_acpi.h
>> +++ b/tools/libxl/libxl_arm_acpi.h
>> @@ -19,6 +19,8 @@
>>
>> #include <xc_dom.h>
>>
>> +#define DOMU_MAX_VCPUS 128
>> +
>
> I would rather define the maximum number of VCPUS in public/arch_arm.h
> to avoid defining the current number of vCPUs supported in multiple places.
>
>> int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>> libxl__domain_build_state *state,
>> struct xc_dom_image *dom);
>> diff --git a/tools/libxl/libxl_empty_dsdt_arm.asl
>> b/tools/libxl/libxl_empty_dsdt_arm.asl
>> new file mode 100644
>> index 0000000..005fa6a
>> --- /dev/null
>> +++ b/tools/libxl/libxl_empty_dsdt_arm.asl
>> @@ -0,0 +1,22 @@
>> +/******************************************************************************
>>
>> + * DSDT for Xen ARM DomU
>> + *
>> + * Copyright (c) 2004, Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but
>> WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
>> License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along with
>> + * this program; If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +DefinitionBlock ("DSDT.aml", "DSDT", 3, "XenARM", "Xen DSDT", 1)
>> +{
>> +
>> +}
>> diff --git a/tools/libxl/libxl_mk_dsdt_arm.c
>> b/tools/libxl/libxl_mk_dsdt_arm.c
>> new file mode 100644
>> index 0000000..96fadbd
>> --- /dev/null
>> +++ b/tools/libxl/libxl_mk_dsdt_arm.c
>
> Can we share the code from tools/firmware/acpi/mk_dsdt.c?
>
Yeah, we can share push_block(), pop_block() stmt() and indent() but the
main() function is totally different since there are only the processor
device objects for ARM DSDT but there are many other things in x86.
I think that since Boris will move the codes under
tools/firmware/hvmloader/acpi to other place, after that we could see
how to share codes then.
>> @@ -0,0 +1,94 @@
>> +#include <stdio.h>
>> +#include <stdarg.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <stdlib.h>
>> +#include "libxl_arm_acpi.h"
>> +
>> +static unsigned int indent_level;
>> +
>> +static void indent(void)
>> +{
>> + unsigned int i;
>> + for ( i = 0; i < indent_level; i++ )
>> + printf(" ");
>> +}
>> +
>> +static __attribute__((format(printf, 2, 3)))
>> +void _stmt(const char *name, const char *fmt, ...)
>> +{
>> + va_list args;
>> +
>> + indent();
>> + printf("%s", name);
>> +
>> + if ( !fmt )
>> + return;
>> +
>> + printf(" ( ");
>> + va_start(args, fmt);
>> + vprintf(fmt, args);
>> + va_end(args);
>> + printf(" )");
>> +}
>> +
>> +#define stmt(n, f, a...) \
>> + do { \
>> + _stmt(n, f , ## a ); \
>> + printf("\n"); \
>> + } while (0)
>> +
>> +#define push_block(n, f, a...) \
>> + do { \
>> + _stmt(n, f , ## a ); \
>> + printf(" {\n"); \
>> + indent_level++; \
>> + } while (0)
>> +
>> +static void pop_block(void)
>> +{
>> + indent_level--;
>> + indent();
>> + printf("}\n");
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> + unsigned int cpu, max_cpus = DOMU_MAX_VCPUS;
>> +
>> + /**** DSDT DefinitionBlock start ****/
>> + /* (we append to existing DSDT definition block) */
>> + indent_level++;
>> +
>> + /**** Processor start ****/
>> + push_block("Scope", "\\_SB");
>> +
>> + /* Define processor objects and control methods. */
>> + for ( cpu = 0; cpu < max_cpus; cpu++)
>> + {
>> + push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu,
>> cpu);
>> +
>> + stmt("Name", "_HID, \"ACPI0007\"");
>> + stmt("Name", "_UID, %d", cpu);
>> +
>> + pop_block();
>> + }
>> +
>> + pop_block();
>> + /**** Processor end ****/
>> +
>> + pop_block();
>> + /**** DSDT DefinitionBlock end ****/
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * tab-width: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>>
>
> Regards,
>
--
Shannon
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |