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

From: Rahul Singh <Rahul.Singh@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Paul Durrant <paul@xen.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v1 03/14] xen/pci: solve compilation error on ARM with ACPI && HAS_PCI
Date: Fri, 10 Sep 2021 10:22:36 +0000	[thread overview]
Message-ID: <4CECF64B-EFAC-43B6-9D47-4C3040C9A286@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2109081802190.10523@sstabellini-ThinkPad-T480s>

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