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

Rahul Singh posted 14 patches 3 years, 3 months ago
There is a newer version of this series
[PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.
Posted by Rahul Singh 3 years, 3 months ago
pci_init(..) will be called during xen startup to initialize and probe
the PCI host-bridge driver.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
 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>
 
 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();
+
+    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,
 };
-- 
2.17.1


Re: [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.
Posted by Julien Grall 3 years, 2 months ago
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@arm.com>
> ---
>   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

Re: [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.
Posted by Rahul Singh 3 years, 2 months ago
Hi Julien,

> On 7 Sep 2021, at 9:20 am, Julien Grall <julien@xen.org> wrote:
> 
> 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@arm.com>
>> ---
>>  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?).
 
Yes you are right this is not necessary .I will remove "param.h"
> 
>>    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?

Calling pci_segment_init(..) before or after PCI initialization will not make any 
difference as this is independent call. Anyway I will move the pci_segment_init(..)  
before PCI initialization.

Regards,
Rahul
Re: [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.
Posted by Stefano Stabellini 3 years, 2 months ago
On Thu, 19 Aug 2021, 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@arm.com>
> ---
>  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>
>  
>  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
                                                     ^ a

> +         *   - 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)

If the DT init function returns int, then it would make sense for the
ACPI init function to return int as well?


> +{
> +    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();
> +
> +    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,
>  };
> -- 
> 2.17.1
> 

Re: [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver.
Posted by Rahul Singh 3 years, 2 months ago
Hi Stefano,

> On 9 Sep 2021, at 2:16 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 19 Aug 2021, 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@arm.com>
>> ---
>> 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>
>> 
>> 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
>                                                     ^ a
> 
>> +         *   - 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)
> 
> If the DT init function returns int, then it would make sense for the
> ACPI init function to return int as well?

Ok. I will modify acpi_pci_init(..)  function to return int.

Regards,
Rahul