Re: [PATCH v2 11/17] libxl/arm: Construct ACPI DSDT table

[Julien Grall <julien.grall@arm.com>]

(CC Boris and Doug)

Hi Shannon,

On 27/06/16 07:01, Shannon Zhao wrote:
> On 2016/6/24 1:03, Julien Grall wrote:
>> 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.

I am not sure if we plan to introduce Kconfig for tools. If not, you can 
add an option to the configure to enable/disable ACPI for guest.

This would be gated by the presence of "iasl".

[...]

>>> 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.

I would prefer if we discuss about it now in order to avoid code 
duplication (I have CCed Boris).

For instance we can create a new directory under tools for mk_dsdt.c. 
The main could be different, although it might be possible to gate ARM 
options, and the rest of the code would be shared.

Regards,

-- 
Julien Grall

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