[PATCH v2 02/11] xen/arm: Add new device type for PCI

Oleksandr Andrushchenko posted 11 patches 3 years, 2 months ago
There is a newer version of this series
[PATCH v2 02/11] xen/arm: Add new device type for PCI
Posted by Oleksandr Andrushchenko 3 years, 2 months ago
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Add new device type (DEV_PCI) to distinguish PCI devices from platform
DT devices, so some drivers, like IOMMU, can handle PCI devices
differently.

Also add a helper which is when given a struct device returns the
corresponding struct pci_dev which this device is a part of.

Because of the header cross-dependencies, e.g. we need both
struct pci_dev and struct arch_pci_dev at the same time, this cannot be
done with an inline. Macro can be implemented, but looks scary:

 #define dev_to_pci_dev(dev) container_of((container_of((dev), \
                        struct arch_pci_dev, dev), struct pci_dev, arch)

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

---
Since v1:
 - Folded new device type (DEV_PCI) into this patch.
---
 xen/arch/arm/pci/pci.c       | 10 ++++++++++
 xen/include/asm-arm/device.h |  4 ++--
 xen/include/asm-arm/pci.h    |  7 +++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index bb15edbccc90..e0420d0d86c1 100644
--- a/xen/arch/arm/pci/pci.c
+++ b/xen/arch/arm/pci/pci.c
@@ -27,6 +27,16 @@ int arch_pci_clean_pirqs(struct domain *d)
     return 0;
 }
 
+struct pci_dev *dev_to_pci(struct device *dev)
+{
+    struct arch_pci_dev *arch_dev;
+
+    ASSERT(dev->type == DEV_PCI);
+
+    arch_dev = container_of((dev), struct arch_pci_dev, dev);
+    return container_of(arch_dev, struct pci_dev, arch);
+}
+
 static int __init dt_pci_init(void)
 {
     struct dt_device_node *np;
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index 64aaa2641b7f..12de217b36b9 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -4,6 +4,7 @@
 enum device_type
 {
     DEV_DT,
+    DEV_PCI,
 };
 
 struct dev_archdata {
@@ -27,8 +28,7 @@ typedef struct device device_t;
 
 #include <xen/device_tree.h>
 
-/* TODO: Correctly implement dev_is_pci when PCI is supported on ARM */
-#define dev_is_pci(dev) ((void)(dev), 0)
+#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
 #define dev_is_dt(dev)  ((dev)->type == DEV_DT)
 
 enum device_class
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index d2728a098a11..9e366ae67e83 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -27,6 +27,13 @@ struct arch_pci_dev {
     struct device dev;
 };
 
+/*
+ * Because of the header cross-dependencies, e.g. we need both
+ * struct pci_dev and struct arch_pci_dev at the same time, this cannot be
+ * done with an inline here. Macro can be implemented, but looks scary.
+ */
+struct pci_dev *dev_to_pci(struct device *dev);
+
 /* Arch-specific MSI data for vPCI. */
 struct vpci_arch_msi {
 };
-- 
2.25.1


Re: [PATCH v2 02/11] xen/arm: Add new device type for PCI
Posted by Stefano Stabellini 3 years, 2 months ago
On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> Add new device type (DEV_PCI) to distinguish PCI devices from platform
> DT devices, so some drivers, like IOMMU, can handle PCI devices
> differently.
> 
> Also add a helper which is when given a struct device returns the
> corresponding struct pci_dev which this device is a part of.
> 
> Because of the header cross-dependencies, e.g. we need both
> struct pci_dev and struct arch_pci_dev at the same time, this cannot be
> done with an inline. Macro can be implemented, but looks scary:
> 
>  #define dev_to_pci_dev(dev) container_of((container_of((dev), \
>                         struct arch_pci_dev, dev), struct pci_dev, arch)
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Since v1:
>  - Folded new device type (DEV_PCI) into this patch.
> ---
>  xen/arch/arm/pci/pci.c       | 10 ++++++++++
>  xen/include/asm-arm/device.h |  4 ++--
>  xen/include/asm-arm/pci.h    |  7 +++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index bb15edbccc90..e0420d0d86c1 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -27,6 +27,16 @@ int arch_pci_clean_pirqs(struct domain *d)
>      return 0;
>  }
>  
> +struct pci_dev *dev_to_pci(struct device *dev)
> +{
> +    struct arch_pci_dev *arch_dev;
> +
> +    ASSERT(dev->type == DEV_PCI);
> +
> +    arch_dev = container_of((dev), struct arch_pci_dev, dev);
> +    return container_of(arch_dev, struct pci_dev, arch);
> +}
> +
>  static int __init dt_pci_init(void)
>  {
>      struct dt_device_node *np;
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index 64aaa2641b7f..12de217b36b9 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -4,6 +4,7 @@
>  enum device_type
>  {
>      DEV_DT,
> +    DEV_PCI,
>  };
>  
>  struct dev_archdata {
> @@ -27,8 +28,7 @@ typedef struct device device_t;
>  
>  #include <xen/device_tree.h>
>  
> -/* TODO: Correctly implement dev_is_pci when PCI is supported on ARM */
> -#define dev_is_pci(dev) ((void)(dev), 0)
> +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
>  #define dev_is_dt(dev)  ((dev)->type == DEV_DT)
>  
>  enum device_class
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index d2728a098a11..9e366ae67e83 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -27,6 +27,13 @@ struct arch_pci_dev {
>      struct device dev;
>  };
>  
> +/*
> + * Because of the header cross-dependencies, e.g. we need both
> + * struct pci_dev and struct arch_pci_dev at the same time, this cannot be
> + * done with an inline here. Macro can be implemented, but looks scary.
> + */
> +struct pci_dev *dev_to_pci(struct device *dev);
> +
>  /* Arch-specific MSI data for vPCI. */
>  struct vpci_arch_msi {
>  };
> -- 
> 2.25.1
> 

Re: [PATCH v2 02/11] xen/arm: Add new device type for PCI
Posted by Jan Beulich 3 years, 2 months ago
On 23.09.2021 14:54, Oleksandr Andrushchenko wrote:
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -27,6 +27,16 @@ int arch_pci_clean_pirqs(struct domain *d)
>      return 0;
>  }
>  
> +struct pci_dev *dev_to_pci(struct device *dev)
> +{
> +    struct arch_pci_dev *arch_dev;
> +
> +    ASSERT(dev->type == DEV_PCI);
> +
> +    arch_dev = container_of((dev), struct arch_pci_dev, dev);

Nit: This not being a macro, the inner parentheses aren't needed.

> +    return container_of(arch_dev, struct pci_dev, arch);

Two successive container_of() on the same pointer look odd. Is
there a reason a single one wouldn't do?

    return container_of(dev, struct pci_dev, arch.dev);

Jan


Re: [PATCH v2 02/11] xen/arm: Add new device type for PCI
Posted by Oleksandr Andrushchenko 3 years, 2 months ago
On 27.09.21 10:41, Jan Beulich wrote:
> On 23.09.2021 14:54, Oleksandr Andrushchenko wrote:
>> --- a/xen/arch/arm/pci/pci.c
>> +++ b/xen/arch/arm/pci/pci.c
>> @@ -27,6 +27,16 @@ int arch_pci_clean_pirqs(struct domain *d)
>>       return 0;
>>   }
>>   
>> +struct pci_dev *dev_to_pci(struct device *dev)
>> +{
>> +    struct arch_pci_dev *arch_dev;
>> +
>> +    ASSERT(dev->type == DEV_PCI);
>> +
>> +    arch_dev = container_of((dev), struct arch_pci_dev, dev);
> Nit: This not being a macro, the inner parentheses aren't needed.
>
>> +    return container_of(arch_dev, struct pci_dev, arch);
> Two successive container_of() on the same pointer look odd. Is
> there a reason a single one wouldn't do?
>
>      return container_of(dev, struct pci_dev, arch.dev);
Good catch, thank you! No reason at all, will fix
>
> Jan
>