Hello Jan,
> On 17 Nov 2020, at 10:55 am, Jan Beulich <jbeul...@suse.com> wrote:
>
> On 16.11.2020 13:25, Rahul Singh wrote:
>> NS16550 driver has PCI support that is under HAS_PCI flag. When HAS_PCI
>> is enabled for ARM, compilation error is observed for ARM architecture
>> because ARM platforms do not have full PCI support available.
>
> While you've extended the sentence, it remains unclear to me what
> compilation error it is that results here. I've requested such
> clarification for v2 already.
Compilation error is related to the code that refer to x86 functions
(create_irq()..) and MSI implementation related error.
For more details please find the attached file for compilation error.
>
>> --- a/xen/drivers/char/Kconfig
>> +++ b/xen/drivers/char/Kconfig
>> @@ -4,6 +4,10 @@ config HAS_NS16550
>> help
>> This selects the 16550-series UART support. For most systems, say Y.
>>
>> +config HAS_NS16550_PCI
>> + def_bool y
>> + depends on X86 && HAS_NS16550 && HAS_PCI
>
> Looking at this again (in particular at all the #ifdef changes in
> the actual source file), I wonder whether an approach with less
> code churn and without such an extra Kconfig setting (with, as
> said, a bogus dependency on x86) couldn't be found. For example,
> how about ...
>
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -16,7 +16,7 @@
>> #include <xen/timer.h>
>> #include <xen/serial.h>
>> #include <xen/iocap.h>
>> -#ifdef CONFIG_HAS_PCI
>> +#ifdef CONFIG_HAS_NS16550_PCI
>> #include <xen/pci.h>
>> #include <xen/pci_regs.h>
>> #include <xen/pci_ids.h>
>
> ... #undef-ining CONFIG_HAS_PCI at a suitable position in this
> file (e.g. after all #include-s, to make sure all structure
> layouts remain correct)? This would then be far easier to revert
> down the road, and would confine the oddity to a single file
> (and there a single place) in the code base.
>
As for ARM platforms, PCI implementation is in the development process and I am
not sure if after completion of PCI work, ns16500 PCI part of code will work
out of the box. I think there is some effort required to test the ns16550 PCI
part of the code on ARM.
As this code is tested on X86 only so I make the options depends on X86 and
enable it by default for x86.
I feel that adding a new Kconfig options is ok to enable/disable the PCI
NS16550 support as compared to #undef CONFIG_HAS_PCI in the particular file. If
in future other architecture wants to implement the PCI they will face the
similar compilation error issues.
Please suggest how we can proceed on this.
> Jan
Regards,
Rahul
{\rtf1\ansi\ansicpg1252\cocoartf2513
\cocoatextscaling0\cocoaplatform0{\fonttbl\f0\fnil\fcharset0 Menlo-Bold;\f1\fnil\fcharset0 Menlo-Regular;}
{\colortbl;\red255\green255\blue255;\red180\green36\blue25;\red0\green0\blue0;\red47\green180\blue29;
\red46\green174\blue187;}
{\*\expandedcolortbl;;\cssrgb\c76409\c21698\c12524;\csgray\c0;\cssrgb\c20238\c73898\c14947;
\cssrgb\c20196\c73240\c78250;}
\paperw11900\paperh16840\margl1440\margr1440\vieww28600\viewh18000\viewkind0
\pard\tx560\tx1120\tx1680\tx2240\tx2800\tx3360\tx3920\tx4480\tx5040\tx5600\tx6160\tx6720\pardirnatural\partightenfactor0
\f0\b\fs36 \cf2 \CocoaLigature0 \cf3 ns16550.c:
\f1\b0 In function \'91
\f0\b ns16550_init_irq
\f1\b0 \'92:\
\pard\tx560\tx1120\tx1680\tx2240\tx2800\tx3360\tx3920\tx4480\tx5040\tx5600\tx6160\tx6720\pardirnatural\partightenfactor0
\f0\b ns16550.c:726:21:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b create_irq
\f1\b0 \'92; did you mean \'91
\f0\b release_irq
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
uart->irq =
\f0\b \cf2 create_irq
\f1\b0 \cf3 (0, false);\
\f0\b \cf2 ^~~~~~~~~~
\f1\b0 \cf3 \
\cf4 release_irq\cf3 \
\f0\b ns16550.c:726:21:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b create_irq
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:
\f1\b0 In function \'91
\f0\b ns16550_init_postirq
\f1\b0 \'92:\
\f0\b ns16550.c:768:33:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 \'91
\f0\b mmio_ro_ranges
\f1\b0 \'92 undeclared (first use in this function); did you mean \'91
\f0\b mmio_handler
\f1\b0 \'92?\
rangeset_add_range(
\f0\b \cf2 mmio_ro_ranges
\f1\b0 \cf3 , uart->io_base,\
\f0\b \cf2 ^~~~~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 mmio_handler\cf3 \
\f0\b ns16550.c:768:33:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 each undeclared identifier is reported only once for each function it appears in\
\f0\b ns16550.c:780:20:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 variable \'91
\f0\b msi
\f1\b0 \'92 has initializer but incomplete type\
struct
\f0\b \cf2 msi_info
\f1\b0 \cf3 msi = \{\
\f0\b \cf2 ^~~~~~~~
\f1\b0 \cf3 \
\f0\b ns16550.c:781:18:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 \'91
\f0\b struct msi_info
\f1\b0 \'92 has no member named \'91
\f0\b bus
\f1\b0 \'92\
.
\f0\b \cf2 bus
\f1\b0 \cf3 = uart->ps_bdf[0],\
\f0\b \cf2 ^~~
\f1\b0 \cf3 \
\f0\b ns16550.c:781:24:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 excess elements in struct initializer [
\f0\b \cf2 -Werror
\f1\b0 \cf3 ]\
.bus =
\f0\b \cf2 uart
\f1\b0 \cf3 ->ps_bdf[0],\
\f0\b \cf2 ^~~~
\f1\b0 \cf3 \
\f0\b ns16550.c:781:24:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 (near initialization for \'91
\f0\b msi
\f1\b0 \'92)\
\f0\b ns16550.c:782:18:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 \'91
\f0\b struct msi_info
\f1\b0 \'92 has no member named \'91
\f0\b devfn
\f1\b0 \'92\
.
\f0\b \cf2 devfn
\f1\b0 \cf3 = PCI_DEVFN(uart->ps_bdf[1], uart->ps_bdf[2]),\
\f0\b \cf2 ^~~~~
\f1\b0 \cf3 \
In file included from
\f0\b /home/rahsin01/work/xen-scm-git-code/fresh-test-code/xen/xen/include/xen/iommu.h:25:0
\f1\b0 ,\
from
\f0\b /home/rahsin01/work/xen-scm-git-code/fresh-test-code/xen/xen/include/xen/sched.h:12
\f1\b0 ,\
from
\f0\b ns16550.c:15
\f1\b0 :\
\f0\b /home/rahsin01/work/xen-scm-git-code/fresh-test-code/xen/xen/include/xen/pci.h:33:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 excess elements in struct initializer [
\f0\b \cf2 -Werror
\f1\b0 \cf3 ]\
#define PCI_DEVFN(d,f)
\f0\b \cf2 (
\f1\b0 \cf3 (((d) & 0x1f) << 3) | ((f) & 0x07))\
\f0\b \cf2 ^
\f1\b0 \cf3 \
\f0\b ns16550.c:782:26:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 in expansion of macro \'91
\f0\b PCI_DEVFN
\f1\b0 \'92\
.devfn =
\f0\b \cf5 PCI_DEVFN
\f1\b0 \cf3 (uart->ps_bdf[1], uart->ps_bdf[2]),\
\f0\b \cf5 ^~~~~~~~~
\f1\b0 \cf3 \
\f0\b /home/rahsin01/work/xen-scm-git-code/fresh-test-code/xen/xen/include/xen/pci.h:33:25:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 (near initialization for \'91
\f0\b msi
\f1\b0 \'92)\
#define PCI_DEVFN(d,f)
\f0\b \cf5 (
\f1\b0 \cf3 (((d) & 0x1f) << 3) | ((f) & 0x07))\
\f0\b \cf5 ^
\f1\b0 \cf3 \
\f0\b ns16550.c:782:26:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 in expansion of macro \'91
\f0\b PCI_DEVFN
\f1\b0 \'92\
.devfn =
\f0\b \cf5 PCI_DEVFN
\f1\b0 \cf3 (uart->ps_bdf[1], uart->ps_bdf[2]),\
\f0\b \cf5 ^~~~~~~~~
\f1\b0 \cf3 \
\f0\b ns16550.c:783:18:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 \'91
\f0\b struct msi_info
\f1\b0 \'92 has no member named \'91
\f0\b irq
\f1\b0 \'92\
.
\f0\b \cf2 irq
\f1\b0 \cf3 = rc = uart->irq,\
\f0\b \cf2 ^~~
\f1\b0 \cf3 \
\f0\b ns16550.c:783:24:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 excess elements in struct initializer [
\f0\b \cf2 -Werror
\f1\b0 \cf3 ]\
.irq =
\f0\b \cf2 rc
\f1\b0 \cf3 = uart->irq,\
\f0\b \cf2 ^~
\f1\b0 \cf3 \
\f0\b ns16550.c:783:24:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 (near initialization for \'91
\f0\b msi
\f1\b0 \'92)\
\f0\b ns16550.c:784:18:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 \'91
\f0\b struct msi_info
\f1\b0 \'92 has no member named \'91
\f0\b entry_nr
\f1\b0 \'92\
.
\f0\b \cf2 entry_nr
\f1\b0 \cf3 = 1\
\f0\b \cf2 ^~~~~~~~
\f1\b0 \cf3 \
\f0\b ns16550.c:784:29:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 excess elements in struct initializer [
\f0\b \cf2 -Werror
\f1\b0 \cf3 ]\
.entry_nr =
\f0\b \cf2 1
\f1\b0 \cf3 \
\f0\b \cf2 ^
\f1\b0 \cf3 \
\f0\b ns16550.c:784:29:
\f1\b0
\f0\b \cf5 note:
\f1\b0 \cf3 (near initialization for \'91
\f0\b msi
\f1\b0 \'92)\
\f0\b ns16550.c:780:29:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 storage size of \'91
\f0\b msi
\f1\b0 \'92 isn\'92t known\
struct msi_info
\f0\b \cf2 msi
\f1\b0 \cf3 = \{\
\f0\b \cf2 ^~~
\f1\b0 \cf3 \
\f0\b ns16550.c:793:22:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b pci_enable_msi
\f1\b0 \'92; did you mean \'91
\f0\b hap_enabled
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
rc =
\f0\b \cf2 pci_enable_msi
\f1\b0 \cf3 (&msi, &msi_desc);\
\f0\b \cf2 ^~~~~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 hap_enabled\cf3 \
\f0\b ns16550.c:793:22:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b pci_enable_msi
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:800:26:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b setup_msi_irq
\f1\b0 \'92; did you mean \'91
\f0\b setup_irq
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
rc =
\f0\b \cf2 setup_msi_irq
\f1\b0 \cf3 (desc, msi_desc);\
\f0\b \cf2 ^~~~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 setup_irq\cf3 \
\f0\b ns16550.c:800:26:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b setup_msi_irq
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:803:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b pci_disable_msi
\f1\b0 \'92; did you mean \'91
\f0\b gic_disable_cpu
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
\f0\b \cf2 pci_disable_msi
\f1\b0 \cf3 (msi_desc);\
\f0\b \cf2 ^~~~~~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 gic_disable_cpu\cf3 \
\f0\b ns16550.c:803:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b pci_disable_msi
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:812:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b msi_free_irq
\f1\b0 \'92; did you mean \'91
\f0\b vgic_free_virq
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
\f0\b \cf2 msi_free_irq
\f1\b0 \cf3 (msi_desc);\
\f0\b \cf2 ^~~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 vgic_free_virq\cf3 \
\f0\b ns16550.c:812:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b msi_free_irq
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:814:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 implicit declaration of function \'91
\f0\b destroy_irq
\f1\b0 \'92; did you mean \'91
\f0\b setup_irq
\f1\b0 \'92? [
\f0\b \cf2 -Werror=implicit-function-declaration
\f1\b0 \cf3 ]\
\f0\b \cf2 destroy_irq
\f1\b0 \cf3 (msi.irq);\
\f0\b \cf2 ^~~~~~~~~~~
\f1\b0 \cf3 \
\cf4 setup_irq\cf3 \
\f0\b ns16550.c:814:25:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 nested extern declaration of \'91
\f0\b destroy_irq
\f1\b0 \'92 [
\f0\b \cf2 -Werror=nested-externs
\f1\b0 \cf3 ]\
\f0\b ns16550.c:780:29:
\f1\b0
\f0\b \cf2 error:
\f1\b0 \cf3 unused variable \'91
\f0\b msi
\f1\b0 \'92 [
\f0\b \cf2 -Werror=unused-variable
\f1\b0 \cf3 ]\
struct msi_info
\f0\b \cf2 msi
\f1\b0 \cf3 = \{\
\f0\b \cf2 ^~~
\f1\b0 \cf3 \
}
