[v2] x86/public: move XEN_ACPI_ in a new header

Commit Message

Bertrand Marquis Aug. 25, 2022, 9:48 a.m. UTC

When Xen is compiled for x86 on an arm machine, libacpi build is failing
due to a wrong include path:
- arch-x86/xen.h includes xen.h
- xen.h includes arch-arm.h (as __i386__ and __x86_64__ are not defined
but arm ones are).

To solve this issue move XEN_ACPI_ definitions in a new header
guest-acpi.h that can be included cleanly by mk_dsdt.c.
Inside this header, only protect the definitions using ifdef
__XEN_TOOLS__ as the defines are not used anywhere in the hypervisor and
are not expected to be.

Previous users needing any of the XEN_ACPI_ definitions will now need to
include arch-x86/guest-acpi.h instead of arch-x86/xen.h

Fixes: d6ac8e22c7c5 ("acpi/x86: define ACPI IO registers for PVH guests")
Signed-off-by: Bertrand Marquis <[e-mail]>
---
The x86 header is including ../xen.h before the ifndef/define so that it
gets included back by xen.h. This is wrongly making the assumption that
we are using an x86 compiler which is not the case when building the
tools for x86 on an arm host.
Moving the definitions to an independent header is making things cleaner
but some might need to include a new header but the risk is low.

For the release manager:
- risk: very low, the definitions moved are only used in mk_dsdt and
external users would just have to include the new header.
- advantage: we can now compile xen for x86 on arm build machines
---
Changes in v2:
- fix commit message
- remove ifdef __XEN__ protecting the definitions
- fix name in description and ifdef guards of the file
- fix description
Changes in v1:
- was "libacpi: Fix cross building x86 on arm"
- move XEN_ACPI_ definitions in a new header guest-acpi.h
- adapt mk_dsdt.c
- remove todo in public header
---
 tools/libacpi/mk_dsdt.c                  |  2 +-
 xen/include/public/arch-x86/guest-acpi.h | 50 ++++++++++++++++++++++++
 xen/include/public/arch-x86/xen.h        |  6 ---
 3 files changed, 51 insertions(+), 7 deletions(-)
 create mode 100644 xen/include/public/arch-x86/guest-acpi.h

Comments

Jan Beulich Aug. 25, 2022, 9:55 a.m. UTC | #1

On 25.08.2022 11:48, Bertrand Marquis wrote:
> When Xen is compiled for x86 on an arm machine, libacpi build is failing
> due to a wrong include path:
> - arch-x86/xen.h includes xen.h
> - xen.h includes arch-arm.h (as __i386__ and __x86_64__ are not defined
> but arm ones are).
> 
> To solve this issue move XEN_ACPI_ definitions in a new header
> guest-acpi.h that can be included cleanly by mk_dsdt.c.
> Inside this header, only protect the definitions using ifdef
> __XEN_TOOLS__ as the defines are not used anywhere in the hypervisor and
> are not expected to be.
> 
> Previous users needing any of the XEN_ACPI_ definitions will now need to
> include arch-x86/guest-acpi.h instead of arch-x86/xen.h
> 
> Fixes: d6ac8e22c7c5 ("acpi/x86: define ACPI IO registers for PVH guests")
> Signed-off-by: Bertrand Marquis <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

> For the release manager:
> - risk: very low, the definitions moved are only used in mk_dsdt and
> external users would just have to include the new header.
> - advantage: we can now compile xen for x86 on arm build machines

I'll give it a little for Henry to possibly release-ack this, but since
strictly speaking this is a bug fix, I think it could also go in without
(as long as not actually objected to, of course).

Jan

Henry Wang Aug. 30, 2022, 12:52 a.m. UTC | #2

Hi Jan and Bertrand,

Sorry for the late reply. I had a couple of days off.

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Subject: Re: [PATCH v2] x86/public: move XEN_ACPI_ in a new header
> 
> On 25.08.2022 11:48, Bertrand Marquis wrote:
> > When Xen is compiled for x86 on an arm machine, libacpi build is failing
> > due to a wrong include path:
> > - arch-x86/xen.h includes xen.h
> > - xen.h includes arch-arm.h (as __i386__ and __x86_64__ are not defined
> > but arm ones are).
> >
> > To solve this issue move XEN_ACPI_ definitions in a new header
> > guest-acpi.h that can be included cleanly by mk_dsdt.c.
> > Inside this header, only protect the definitions using ifdef
> > __XEN_TOOLS__ as the defines are not used anywhere in the hypervisor
> and
> > are not expected to be.
> >
> > Previous users needing any of the XEN_ACPI_ definitions will now need to
> > include arch-x86/guest-acpi.h instead of arch-x86/xen.h
> >
> > Fixes: d6ac8e22c7c5 ("acpi/x86: define ACPI IO registers for PVH guests")
> > Signed-off-by: Bertrand Marquis <[e-mail]>
> 
> Reviewed-by: Jan Beulich <[e-mail]>
> 
> > For the release manager:
> > - risk: very low, the definitions moved are only used in mk_dsdt and
> > external users would just have to include the new header.
> > - advantage: we can now compile xen for x86 on arm build machines
> 
> I'll give it a little for Henry to possibly release-ack this, but since
> strictly speaking this is a bug fix, I think it could also go in without
> (as long as not actually objected to, of course).

Thanks for informing. Yeah definitely no problem from my side, so:

Acked-by: Henry Wang <[e-mail]> # For the 4.17 release

Kind regards,
Henry


> 
> Jan

Jan Beulich Sept. 6, 2022, 2:07 p.m. UTC | #3

On 30.08.2022 02:52, Henry Wang wrote:
>> -----Original Message-----
>> From: Jan Beulich <[e-mail]>
>> Subject: Re: [PATCH v2] x86/public: move XEN_ACPI_ in a new header
>>
>> On 25.08.2022 11:48, Bertrand Marquis wrote:
>>> When Xen is compiled for x86 on an arm machine, libacpi build is failing
>>> due to a wrong include path:
>>> - arch-x86/xen.h includes xen.h
>>> - xen.h includes arch-arm.h (as __i386__ and __x86_64__ are not defined
>>> but arm ones are).
>>>
>>> To solve this issue move XEN_ACPI_ definitions in a new header
>>> guest-acpi.h that can be included cleanly by mk_dsdt.c.
>>> Inside this header, only protect the definitions using ifdef
>>> __XEN_TOOLS__ as the defines are not used anywhere in the hypervisor
>> and
>>> are not expected to be.
>>>
>>> Previous users needing any of the XEN_ACPI_ definitions will now need to
>>> include arch-x86/guest-acpi.h instead of arch-x86/xen.h
>>>
>>> Fixes: d6ac8e22c7c5 ("acpi/x86: define ACPI IO registers for PVH guests")
>>> Signed-off-by: Bertrand Marquis <[e-mail]>
>>
>> Reviewed-by: Jan Beulich <[e-mail]>
>>
>>> For the release manager:
>>> - risk: very low, the definitions moved are only used in mk_dsdt and
>>> external users would just have to include the new header.
>>> - advantage: we can now compile xen for x86 on arm build machines
>>
>> I'll give it a little for Henry to possibly release-ack this, but since
>> strictly speaking this is a bug fix, I think it could also go in without
>> (as long as not actually objected to, of course).
> 
> Thanks for informing. Yeah definitely no problem from my side, so:
> 
> Acked-by: Henry Wang <[e-mail]> # For the 4.17 release

I've translated this to Release-acked-by: (as was used for earlier releases).

Jan

Henry Wang Sept. 6, 2022, 2:09 p.m. UTC | #4

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> > Acked-by: Henry Wang <[e-mail]> # For the 4.17 release
> 
> I've translated this to Release-acked-by: (as was used for earlier releases).

Thanks for letting me know this! I will keep in mind and use this in the future.

Kind regards,
Henry


> 
> Jan

Patch

diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index c5ba4c0b2fd3..1176da80ef44 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -18,7 +18,7 @@ 
 #include <stdlib.h>
 #include <stdbool.h>
 #if defined(CONFIG_X86)
-#include <xen/arch-x86/xen.h>
+#include <xen/arch-x86/guest-acpi.h>
 #include <xen/hvm/hvm_info_table.h>
 #elif defined(CONFIG_ARM_64)
 #include <xen/arch-arm.h>
diff --git a/xen/include/public/arch-x86/guest-acpi.h b/xen/include/public/arch-x86/guest-acpi.h
new file mode 100644
index 000000000000..3d79a31fd865
--- /dev/null
+++ b/xen/include/public/arch-x86/guest-acpi.h
@@ -0,0 +1,50 @@ 
+/******************************************************************************
+ * arch-x86/guest-acpi.h
+ *
+ * Guest ACPI interface to x86 Xen.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_X86_GUEST_ACPI_H__
+#define __XEN_PUBLIC_ARCH_X86_GUEST_ACPI_H__
+
+#ifdef __XEN_TOOLS__
+
+/* Location of online VCPU bitmap. */
+#define XEN_ACPI_CPU_MAP             0xaf00
+#define XEN_ACPI_CPU_MAP_LEN         ((HVM_MAX_VCPUS + 7) / 8)
+
+/* GPE0 bit set during CPU hotplug */
+#define XEN_ACPI_GPE0_CPUHP_BIT      2
+
+#endif /* __XEN_TOOLS__ */
+
+#endif /* __XEN_PUBLIC_ARCH_X86_GUEST_ACPI_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 58a1e87ee971..546dd4496ac6 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -325,12 +325,6 @@  struct xen_arch_domainconfig {
 /* Max  XEN_X86_* constant. Used for ABI checking. */
 #define XEN_X86_MISC_FLAGS_MAX XEN_X86_ASSISTED_X2APIC
 
-/* Location of online VCPU bitmap. */
-#define XEN_ACPI_CPU_MAP             0xaf00
-#define XEN_ACPI_CPU_MAP_LEN         ((HVM_MAX_VCPUS + 7) / 8)
-
-/* GPE0 bit set during CPU hotplug */
-#define XEN_ACPI_GPE0_CPUHP_BIT      2
 #endif
 
 /*