Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Andrii Anisov 
> I diffed the generated dsdt and it is the same before and after the
> patches when built natively. I did try cross-build, Andrii could you
> give a try?
Build passed smoothly.
Non-ACPI guests started well.

Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Wei Liu 
On Mon, Nov 28, 2016 at 06:30:58AM -0700, Jan Beulich wrote:
> >>> On 28.11.16 at 14:13,  wrote:
> > The tools (such as mk_dsdt) can be cross-built when it may not be
> > desirable to build them on the target.
> > 
> > The commit c4ac1077 "libxl/arm: Generate static ACPI DSDT table"
> > introduced support of ARM64 in mk_dsdt but also break cross-building
> > tools because the ACPI tables are not correct.
> > 
> > While mk_dsdt should generate ACPI table for the target architecture, it
> > currently generates the one for the host. This is because the source
> > code contains reference to the host architecture (__aarch64__,
> > __x86_64__, __i386__) when it should be the target architecture.
> > 
> > Replace all __aarch64__, __x86_64__, __i386__ by the corresponding
> > CONFIG_*.
> > 
> > Also expose the CONFIG_* to the source code as the currently only
> > exposed to the Makefile.
> > 
> > Reported-by: Andrii Anisov 
> > Suggested-by: Wei Liu 
> > Signed-off-by: Julien Grall 
> > Reviewed-by: Andrew Cooper 
> 
> Reviewed-by: Jan Beulich 
> 

Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Jan Beulich 
>>> On 28.11.16 at 14:37,  wrote:
> On 28/11/16 13:30, Jan Beulich wrote:
>> However, ...
>>
>>> --- a/tools/libacpi/mk_dsdt.c
>>> +++ b/tools/libacpi/mk_dsdt.c
>>> @@ -17,9 +17,9 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> -#if defined(__i386__) || defined(__x86_64__)
>>> +#if defined(CONFIG_X86)
>>>  #include 
>>> -#elif defined(__aarch64__)
>>> +#elif defined(CONFIG_ARM_64)
>>>  #include 
>>>  #endif
>>
>> .. for this and at least some of the others I wonder whether from an
>> abstract pov these shouldn't be CONFIG_ARM. Agreed, it won't
>> matter as long as there's no use of ACPI on ARM32, hence my R-b
>> stands either way, but I'd like you to clarify whether it really should
>> go in this way.
> 
> To answer the question, I am not aware of any plan to have support for 
> ACPI on ARM32.

Except that this wasn't the question.

> Regardless the answer, this would be a separate patch as CONFIG_ARM_64 
> is the correct define to match __aarch64__. Let's not mix improvement 
> and bug fix.

Well, it looks even more suspicious to me now than it did before
that both x86-32 and x86-64 are being taken care of, but only
arm64. But anyway ...

Jan


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

Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Julien Grall 

Hi,

On 28/11/16 13:30, Jan Beulich wrote:

On 28.11.16 at 14:13,  wrote:

The tools (such as mk_dsdt) can be cross-built when it may not be
desirable to build them on the target.

The commit c4ac1077 "libxl/arm: Generate static ACPI DSDT table"
introduced support of ARM64 in mk_dsdt but also break cross-building
tools because the ACPI tables are not correct.

While mk_dsdt should generate ACPI table for the target architecture, it
currently generates the one for the host. This is because the source
code contains reference to the host architecture (__aarch64__,
__x86_64__, __i386__) when it should be the target architecture.

Replace all __aarch64__, __x86_64__, __i386__ by the corresponding
CONFIG_*.

Also expose the CONFIG_* to the source code as the currently only
exposed to the Makefile.

Reported-by: Andrii Anisov 
Suggested-by: Wei Liu 
Signed-off-by: Julien Grall 
Reviewed-by: Andrew Cooper 


Reviewed-by: Jan Beulich 

However, ...


--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -17,9 +17,9 @@
 #include 
 #include 
 #include 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(CONFIG_X86)
 #include 
-#elif defined(__aarch64__)
+#elif defined(CONFIG_ARM_64)
 #include 
 #endif


.. for this and at least some of the others I wonder whether from an
abstract pov these shouldn't be CONFIG_ARM. Agreed, it won't
matter as long as there's no use of ACPI on ARM32, hence my R-b
stands either way, but I'd like you to clarify whether it really should
go in this way.


To answer the question, I am not aware of any plan to have support for 
ACPI on ARM32.


Regardless the answer, this would be a separate patch as CONFIG_ARM_64 
is the correct define to match __aarch64__. Let's not mix improvement 
and bug fix.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Jan Beulich 
>>> On 28.11.16 at 14:13,  wrote:
> The tools (such as mk_dsdt) can be cross-built when it may not be
> desirable to build them on the target.
> 
> The commit c4ac1077 "libxl/arm: Generate static ACPI DSDT table"
> introduced support of ARM64 in mk_dsdt but also break cross-building
> tools because the ACPI tables are not correct.
> 
> While mk_dsdt should generate ACPI table for the target architecture, it
> currently generates the one for the host. This is because the source
> code contains reference to the host architecture (__aarch64__,
> __x86_64__, __i386__) when it should be the target architecture.
> 
> Replace all __aarch64__, __x86_64__, __i386__ by the corresponding
> CONFIG_*.
> 
> Also expose the CONFIG_* to the source code as the currently only
> exposed to the Makefile.
> 
> Reported-by: Andrii Anisov 
> Suggested-by: Wei Liu 
> Signed-off-by: Julien Grall 
> Reviewed-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

However, ...

> --- a/tools/libacpi/mk_dsdt.c
> +++ b/tools/libacpi/mk_dsdt.c
> @@ -17,9 +17,9 @@
>  #include 
>  #include 
>  #include 
> -#if defined(__i386__) || defined(__x86_64__)
> +#if defined(CONFIG_X86)
>  #include 
> -#elif defined(__aarch64__)
> +#elif defined(CONFIG_ARM_64)
>  #include 
>  #endif

.. for this and at least some of the others I wonder whether from an
abstract pov these shouldn't be CONFIG_ARM. Agreed, it won't
matter as long as there's no use of ACPI on ARM32, hence my R-b
stands either way, but I'd like you to clarify whether it really should
go in this way.

Jan


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

[Xen-devel] [PATCH v2 for-4.8] tools/libacpi: Fix compilation when cross building the tools

2016-11-28 Thread Julien Grall 
The tools (such as mk_dsdt) can be cross-built when it may not be
desirable to build them on the target.

The commit c4ac1077 "libxl/arm: Generate static ACPI DSDT table"
introduced support of ARM64 in mk_dsdt but also break cross-building
tools because the ACPI tables are not correct.

While mk_dsdt should generate ACPI table for the target architecture, it
currently generates the one for the host. This is because the source
code contains reference to the host architecture (__aarch64__,
__x86_64__, __i386__) when it should be the target architecture.

Replace all __aarch64__, __x86_64__, __i386__ by the corresponding
CONFIG_*.

Also expose the CONFIG_* to the source code as the currently only
exposed to the Makefile.

Reported-by: Andrii Anisov 
Suggested-by: Wei Liu 
Signed-off-by: Julien Grall 
Reviewed-by: Andrew Cooper 

---
Changes in v2:
- Use defined(CONFIG_*) rather than CONFIG_*
- Use #if defined #elif defined construction
- Directly use MKDSDT_CFLAGS-y
- Add Andrew's reviewed-by

This was reported on the ML recently (see [1]) and affects only Xen
4.8. Without this patch, cross-building the tools will not work.

I think this patch is quite important for embedded users where they
tend to cross-build the rootfs (for instance using yocto).

The patch is fairly simple, exposing CONFIG_* to the source code and
replacing all reference to the host architecture to the
corresponding target architecture. It could be easy to test all the
configuration.

I diffed the generated dsdt and it is the same before and after the
patches when built natively. I did try cross-build, Andrii could you
give a try?

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg01903.html
---
 tools/libacpi/Makefile  |  5 -
 tools/libacpi/mk_dsdt.c | 14 +++---
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index ccc32c9..6d8445d 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -27,6 +27,9 @@ DSDT_FILES ?= $(C_SRC-y)
 C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
 H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h 
ssdt_tpm.h)
 
+MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
+MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86
+
 # Suffix for temporary files.
 #
 # We will also use this suffix to workaround a bug in older iasl
@@ -44,7 +47,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex)
  
 $(MK_DSDT): mk_dsdt.c
-   $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ 
mk_dsdt.c
+   $(HOSTCC) $(HOSTCFLAGS) $(MKDSDT_CFLAGS-y) $(CFLAGS_xeninclude) 
-D__XEN_TOOLS__ -o $@ mk_dsdt.c
 
 $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl 
$(MK_DSDT)
# Remove last bracket
diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index 16320a9..760d81b 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -17,9 +17,9 @@
 #include 
 #include 
 #include 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(CONFIG_X86)
 #include 
-#elif defined(__aarch64__)
+#elif defined(CONFIG_ARM_64)
 #include 
 #endif
 
@@ -111,9 +111,9 @@ int main(int argc, char **argv)
 unsigned int slot, dev, intx, link, cpu, max_cpus;
 dm_version dm_version = QEMU_XEN_TRADITIONAL;
 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(CONFIG_X86)
 max_cpus = HVM_MAX_VCPUS;
-#elif defined(__aarch64__)
+#elif defined(CONFIG_ARM_64)
 max_cpus = GUEST_MAX_VCPUS;
 #endif
 
@@ -169,7 +169,7 @@ int main(int argc, char **argv)
 / Processor start /
 push_block("Scope", "\\_SB");
 
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef CONFIG_X86
 /* MADT checksum */
 stmt("OperationRegion", "MSUM, SystemMemory, \\_SB.MSUA, 1");
 push_block("Field", "MSUM, ByteAcc, NoLock, Preserve");
@@ -193,7 +193,7 @@ int main(int argc, char **argv)
 stmt("Name", "_HID, \"ACPI0007\"");
 
 stmt("Name", "_UID, %d", cpu);
-#if defined(__aarch64__)
+#ifdef CONFIG_ARM_64
 pop_block();
 continue;
 #endif
@@ -235,7 +235,7 @@ int main(int argc, char **argv)
 pop_block();
 }
 
-#if defined(__aarch64__)
+#ifdef CONFIG_ARM_64
 pop_block();
 / Processor end /
 pop_block();
-- 
1.9.1


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