Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
architecture. Move the code under CONFIG_X86 flag to gate the code for
ARM.

No functional change intended.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
 xen/drivers/passthrough/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 67f5686ab6..c23c8cb06b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1150,7 +1150,7 @@ void __hwdom_init setup_hwdom_pci_devices(
     pcidevs_unlock();
 }
 
-#ifdef CONFIG_ACPI
+#if defined(CONFIG_ACPI) && defined(CONFIG_X86)
 #include <acpi/acpi.h>
 #include <acpi/apei.h>
 
-- 
2.17.1

On 19.08.2021 14:02, Rahul Singh wrote:
> Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
> architecture. Move the code under CONFIG_X86 flag to gate the code for
> ARM.

Please give at least one example of what it is that fails to compile.
What an appropriate action is depends on the nature of the error(s),
and from looking at the enclosed code I cannot easily see what it
might be that breaks for Arm.

And as suggested by Julien for the earlier patch - you will want to
re-order things such that compilation doesn't "break" in the first
place. Title and description would then want adjusting accordingly.

Jan

Hi Jan

> On 20 Aug 2021, at 8:06 am, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 19.08.2021 14:02, Rahul Singh wrote:
>> Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
>> architecture. Move the code under CONFIG_X86 flag to gate the code for
>> ARM.
> 
> Please give at least one example of what it is that fails to compile.
> What an appropriate action is depends on the nature of the error(s),
> and from looking at the enclosed code I cannot easily see what it
> might be that breaks for Arm.

I am observing below error when enabled ACPI && HAS_PCI for ARM.

prelink.o: In function `pcie_aer_get_firmware_first’:
/xen/drivers/passthrough/pci.c:1251: undefined reference to `apei_hest_parse'
aarch64-linux-gnu-ld: /home/rahsin01/work/xen/pci-passthrough-upstream/xen/xen/.xen-syms.0: hidden symbol `apei_hest_parse' isn't defined

I found that apei/ is only enabled for x86 and pcie_aer_get_firmware_first() is only called from x86 code.
obj-$(CONFIG_X86) += apei/

I am not sure whether we need this code for ARM architecture 
that is why I gate the code for ARM via CONFIG_X86

> 
> And as suggested by Julien for the earlier patch - you will want to
> re-order things such that compilation doesn't "break" in the first
> place. Title and description would then want adjusting accordingly.

Let me reorder  the patch series in next version.
Regards,
Rahul
> 
> Jan
> 

On 20.08.2021 13:41, Rahul Singh wrote:
> Hi Jan
> 
>> On 20 Aug 2021, at 8:06 am, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 19.08.2021 14:02, Rahul Singh wrote:
>>> Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
>>> architecture. Move the code under CONFIG_X86 flag to gate the code for
>>> ARM.
>>
>> Please give at least one example of what it is that fails to compile.
>> What an appropriate action is depends on the nature of the error(s),
>> and from looking at the enclosed code I cannot easily see what it
>> might be that breaks for Arm.
> 
> I am observing below error when enabled ACPI && HAS_PCI for ARM.
> 
> prelink.o: In function `pcie_aer_get_firmware_first’:
> /xen/drivers/passthrough/pci.c:1251: undefined reference to `apei_hest_parse'
> aarch64-linux-gnu-ld: /home/rahsin01/work/xen/pci-passthrough-upstream/xen/xen/.xen-syms.0: hidden symbol `apei_hest_parse' isn't defined
> 
> I found that apei/ is only enabled for x86 and pcie_aer_get_firmware_first() is only called from x86 code.
> obj-$(CONFIG_X86) += apei/
> 
> I am not sure whether we need this code for ARM architecture 
> that is why I gate the code for ARM via CONFIG_X86

So you Arm folks will probably want to settle on that aspect first. What
is wanted to keep things building depends on that.

Jan

On Fri, 20 Aug 2021, Jan Beulich wrote:
> On 20.08.2021 13:41, Rahul Singh wrote:
> > Hi Jan
> > 
> >> On 20 Aug 2021, at 8:06 am, Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 19.08.2021 14:02, Rahul Singh wrote:
> >>> Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
> >>> architecture. Move the code under CONFIG_X86 flag to gate the code for
> >>> ARM.
> >>
> >> Please give at least one example of what it is that fails to compile.
> >> What an appropriate action is depends on the nature of the error(s),
> >> and from looking at the enclosed code I cannot easily see what it
> >> might be that breaks for Arm.
> > 
> > I am observing below error when enabled ACPI && HAS_PCI for ARM.
> > 
> > prelink.o: In function `pcie_aer_get_firmware_first’:
> > /xen/drivers/passthrough/pci.c:1251: undefined reference to `apei_hest_parse'
> > aarch64-linux-gnu-ld: /home/rahsin01/work/xen/pci-passthrough-upstream/xen/xen/.xen-syms.0: hidden symbol `apei_hest_parse' isn't defined
> > 
> > I found that apei/ is only enabled for x86 and pcie_aer_get_firmware_first() is only called from x86 code.
> > obj-$(CONFIG_X86) += apei/
> > 
> > I am not sure whether we need this code for ARM architecture 
> > that is why I gate the code for ARM via CONFIG_X86
> 
> So you Arm folks will probably want to settle on that aspect first. What
> is wanted to keep things building depends on that.

Reading the APEI description, it looks like there might be some use for
it on ARM but it would work a bit differently from x86 as there are no
NMIs on ARM. So enabling APEI on ARM is not just a matter of enabling
the build of apei/, it is not going to be straightforward.

For the scope of this series (which is actually about PCI), I would
leave it alone, and keep apei/ x86 only, which means #ifdefing
pcie_aer_get_firmware_first.

I would just add an in-code comment saying "APEI not supported on ARM
yet". Another option would be to introduce a symbol like HAS_ACPI_APEI
but it is a bit overkill for this.

Hi Stefano,

> On 9 Sep 2021, at 2:11 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Fri, 20 Aug 2021, Jan Beulich wrote:
>> On 20.08.2021 13:41, Rahul Singh wrote:
>>> Hi Jan
>>> 
>>>> On 20 Aug 2021, at 8:06 am, Jan Beulich <jbeulich@suse.com> wrote:
>>>> 
>>>> On 19.08.2021 14:02, Rahul Singh wrote:
>>>>> Compilation error is observed when ACPI and HAS_PCI is enabled for ARM
>>>>> architecture. Move the code under CONFIG_X86 flag to gate the code for
>>>>> ARM.
>>>> 
>>>> Please give at least one example of what it is that fails to compile.
>>>> What an appropriate action is depends on the nature of the error(s),
>>>> and from looking at the enclosed code I cannot easily see what it
>>>> might be that breaks for Arm.
>>> 
>>> I am observing below error when enabled ACPI && HAS_PCI for ARM.
>>> 
>>> prelink.o: In function `pcie_aer_get_firmware_first’:
>>> /xen/drivers/passthrough/pci.c:1251: undefined reference to `apei_hest_parse'
>>> aarch64-linux-gnu-ld: /home/rahsin01/work/xen/pci-passthrough-upstream/xen/xen/.xen-syms.0: hidden symbol `apei_hest_parse' isn't defined
>>> 
>>> I found that apei/ is only enabled for x86 and pcie_aer_get_firmware_first() is only called from x86 code.
>>> obj-$(CONFIG_X86) += apei/
>>> 
>>> I am not sure whether we need this code for ARM architecture 
>>> that is why I gate the code for ARM via CONFIG_X86
>> 
>> So you Arm folks will probably want to settle on that aspect first. What
>> is wanted to keep things building depends on that.
> 
> Reading the APEI description, it looks like there might be some use for
> it on ARM but it would work a bit differently from x86 as there are no
> NMIs on ARM. So enabling APEI on ARM is not just a matter of enabling
> the build of apei/, it is not going to be straightforward.
> 
> For the scope of this series (which is actually about PCI), I would
> leave it alone, and keep apei/ x86 only, which means #ifdefing
> pcie_aer_get_firmware_first.
> 
> I would just add an in-code comment saying "APEI not supported on ARM
> yet”.
 
Ok. I will add the comment in next version.

> Another option would be to introduce a symbol like HAS_ACPI_APEI
> but it is a bit overkill for this.

Regards,
Rahul