| Message ID | 20210923125438.234162-3-andr2000@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI devices passthrough on Arm, part 2 | expand |
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 >
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
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 >
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 { };