Re: [PATCH v1 03/14] xen/pci: solve compilation error on ARM with ACPI && HAS_PCI

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Stefano,

> On 9 Sep 2021, at 2:11 am, Stefano Stabellini <sstabellini@xxxxxxxxxx> 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@xxxxxxxx> 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