| Message ID | 97d39d3ee398d6018bdcaf745f00d039df6a92ef.1629366665.git.rahul.singh@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI devices passthrough on Arm | expand |
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
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>
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(-)