Re: [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.

[Julien Grall <julien@xxxxxxx>]

Hi Rahul,

On 19/08/2021 13:02, Rahul Singh wrote:
> pci_init(..) will be called during xen startup to initialize and probe
> the PCI host-bridge driver.
>
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> ---
>   xen/arch/arm/pci/pci.c       | 54 ++++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/device.h |  1 +
>   2 files changed, 55 insertions(+)
>
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index dc55d23778..d1c9cf997d 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -14,13 +14,67 @@
>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#include <xen/acpi.h>
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
> +#include <xen/init.h>
>   #include <xen/pci.h>
> +#include <xen/param.h>
This include doesn't look to be necessary (yet?).

>   
>   int arch_pci_clean_pirqs(struct domain *d)
>   {
>       return 0;
>   }
>   
> +static int __init dt_pci_init(void)
> +{
> +    struct dt_device_node *np;
> +    int rc;
> +
> +    dt_for_each_device_node(dt_host, np)
> +    {
> +        rc = device_init(np, DEVICE_PCI, NULL);
> +        if( !rc )
> +            continue;
> +        /*
> +         * Ignore the following error codes:
> +         *   - EBADF: Indicate the current is not an pci
> +         *   - ENODEV: The pci device is not present or cannot be used by
> +         *     Xen.
> +         */
> +        else if ( rc != -EBADF && rc != -ENODEV )
> +        {
> +            printk(XENLOG_ERR "No driver found in XEN or driver init 
> error.\n");
> +            return rc;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +#ifdef CONFIG_ACPI
> +static void __init acpi_pci_init(void)
> +{
> +    printk(XENLOG_ERR "ACPI pci init not supported \n");
> +    return;
> +}
> +#else
> +static inline void __init acpi_pci_init(void) { }
> +#endif
> +
> +static int __init pci_init(void)
> +{
> +    if ( acpi_disabled )
> +        dt_pci_init();
> +    else
> +        acpi_pci_init();
> +
> +    pci_segments_init();
Shouldn't this happen before the PCI initialization?

> +
> +    return 0;
> +}
> +__initcall(pci_init);
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index ee7cff2d44..5ecd5e7bd1 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -34,6 +34,7 @@ enum device_class
>       DEVICE_SERIAL,
>       DEVICE_IOMMU,
>       DEVICE_GIC,
> +    DEVICE_PCI,
>       /* Use for error */
>       DEVICE_UNKNOWN,
>   };
Cheers,

--
Julien Grall