| Message ID | b6a9c007061f963332af63da544e5031e18a7850.1632307952.git.rahul.singh@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI devices passthrough on Arm | expand |
On Wed, 22 Sep 2021, Rahul Singh wrote: > Compilation error is observed when HAS_PCI is enabled for ARM > architecture. > > Add definition for arch_iommu_use_permitted() and > arch_pci_clean_pirqs(). > > pci.c: In function ‘deassign_device’: > pci.c:849:49: error: implicit declaration of function ‘pci_to_dev’; > did you mean ‘dt_to_dev’? [-Werror=implicit-function-declaration] > pci_to_dev(pdev)); > pci.c:880: undefined reference to `arch_pci_clean_pirqs’ > pci.c:1392: undefined reference to `arch_iommu_use_permitted' > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Change in v2: > - Remove pci_conf_read*(..) dummy implementation > - Add in code comment for arch_pci_clean_pirqs() and arch_iommu_use_permitted() > - Fixed minor comments > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/pci/Makefile | 1 + > xen/arch/arm/pci/pci.c | 33 +++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/iommu.c | 9 ++++++++ > xen/include/asm-arm/pci.h | 31 ++++++++++++++++++++++++--- > 5 files changed, 72 insertions(+), 3 deletions(-) > create mode 100644 xen/arch/arm/pci/Makefile > create mode 100644 xen/arch/arm/pci/pci.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 3d3b97b5b4..44d7cc81fa 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -2,6 +2,7 @@ obj-$(CONFIG_ARM_32) += arm32/ > obj-$(CONFIG_ARM_64) += arm64/ > obj-$(CONFIG_ARM_64) += efi/ > obj-$(CONFIG_ACPI) += acpi/ > +obj-$(CONFIG_HAS_PCI) += pci/ > ifneq ($(CONFIG_NO_PLAT),y) > obj-y += platforms/ > endif > diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile > new file mode 100644 > index 0000000000..a98035df4c > --- /dev/null > +++ b/xen/arch/arm/pci/Makefile > @@ -0,0 +1 @@ > +obj-y += pci.o > diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c > new file mode 100644 > index 0000000000..a7a7bc3213 > --- /dev/null > +++ b/xen/arch/arm/pci/pci.c > @@ -0,0 +1,33 @@ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <xen/pci.h> > + > +/* > + * PIRQ event channels are not supported on Arm, so nothing to do. > + */ > +int arch_pci_clean_pirqs(struct domain *d) > +{ > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c > index db3b07a571..ee653a9c48 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -135,3 +135,12 @@ void arch_iommu_domain_destroy(struct domain *d) > void __hwdom_init arch_iommu_hwdom_init(struct domain *d) > { > } > + > +/* > + * Unlike x86, Arm doesn't support mem-sharing, mem-paging and log-dirty (yet). > + * So there is no restriction to use the IOMMU. > + */ > +bool arch_iommu_use_permitted(const struct domain *d) > +{ > + return true; > +} > diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h > index de13359f65..7dd9eb3dba 100644 > --- a/xen/include/asm-arm/pci.h > +++ b/xen/include/asm-arm/pci.h > @@ -1,7 +1,32 @@ > -#ifndef __X86_PCI_H__ > -#define __X86_PCI_H__ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > > +#ifndef __ARM_PCI_H__ > +#define __ARM_PCI_H__ > + > +#ifdef CONFIG_HAS_PCI > + > +#define pci_to_dev(pcidev) (&(pcidev)->arch.dev) > + > +/* Arch pci dev struct */ > struct arch_pci_dev { > + struct device dev; > }; > > -#endif /* __X86_PCI_H__ */ > +#else /*!CONFIG_HAS_PCI*/ > + > +struct arch_pci_dev { }; > + > +#endif /*!CONFIG_HAS_PCI*/ > +#endif /* __ARM_PCI_H__ */ > -- > 2.17.1 >
Hi, On Wed, 22 Sep 2021, 16:36 Rahul Singh, <rahul.singh@arm.com> wrote: > Compilation error is observed when HAS_PCI is enabled for ARM > architecture. In general, when I read "compilation error" I interpret as a user can trigger it in the current staging. However, without the full series applied, HAS_PCI cannot be selected on Arm. So I think the message is a bit misleading because this is more implementing stubs to enable a feature rather than fixing compilation errors (AFAICT all of them are not fixed here). How about the following commit message: xen/arm: pci: Add stubs to allow selecting HAS_PCI In a follow-up we will enable PCI support in Xen on Arm (i.e select HAS_PCI). The generic code expects the arch to implement a few functions: <List the functions> Note that this is not yet sufficient to enable HAS_PCI and will be addressed in follow-ups. > Add definition for arch_iommu_use_permitted() and > arch_pci_clean_pirqs(). > > pci.c: In function ‘deassign_device’: > pci.c:849:49: error: implicit declaration of function ‘pci_to_dev’; > did you mean ‘dt_to_dev’? [-Werror=implicit-function-declaration] > pci_to_dev(pdev)); > pci.c:880: undefined reference to `arch_pci_clean_pirqs’ > pci.c:1392: undefined reference to `arch_iommu_use_permitted' > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> > --- > Change in v2: > - Remove pci_conf_read*(..) dummy implementation > - Add in code comment for arch_pci_clean_pirqs() and > arch_iommu_use_permitted() > - Fixed minor comments > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/pci/Makefile | 1 + > xen/arch/arm/pci/pci.c | 33 +++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/iommu.c | 9 ++++++++ > xen/include/asm-arm/pci.h | 31 ++++++++++++++++++++++++--- > 5 files changed, 72 insertions(+), 3 deletions(-) > create mode 100644 xen/arch/arm/pci/Makefile > create mode 100644 xen/arch/arm/pci/pci.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 3d3b97b5b4..44d7cc81fa 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -2,6 +2,7 @@ obj-$(CONFIG_ARM_32) += arm32/ > obj-$(CONFIG_ARM_64) += arm64/ > obj-$(CONFIG_ARM_64) += efi/ > obj-$(CONFIG_ACPI) += acpi/ > +obj-$(CONFIG_HAS_PCI) += pci/ > ifneq ($(CONFIG_NO_PLAT),y) > obj-y += platforms/ > endif > diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile > new file mode 100644 > index 0000000000..a98035df4c > --- /dev/null > +++ b/xen/arch/arm/pci/Makefile > @@ -0,0 +1 @@ > +obj-y += pci.o > diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c > new file mode 100644 > index 0000000000..a7a7bc3213 > --- /dev/null > +++ b/xen/arch/arm/pci/pci.c > @@ -0,0 +1,33 @@ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <xen/pci.h> > + > +/* > + * PIRQ event channels are not supported on Arm, so nothing to do. > + */ > +int arch_pci_clean_pirqs(struct domain *d) > +{ > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/drivers/passthrough/arm/iommu.c > b/xen/drivers/passthrough/arm/iommu.c > index db3b07a571..ee653a9c48 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -135,3 +135,12 @@ void arch_iommu_domain_destroy(struct domain *d) > void __hwdom_init arch_iommu_hwdom_init(struct domain *d) > { > } > + > +/* > + * Unlike x86, Arm doesn't support mem-sharing, mem-paging and log-dirty > (yet). > + * So there is no restriction to use the IOMMU. > + */ > +bool arch_iommu_use_permitted(const struct domain *d) > +{ > + return true; > +} > diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h > index de13359f65..7dd9eb3dba 100644 > --- a/xen/include/asm-arm/pci.h > +++ b/xen/include/asm-arm/pci.h > @@ -1,7 +1,32 @@ > -#ifndef __X86_PCI_H__ > -#define __X86_PCI_H__ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > > +#ifndef __ARM_PCI_H__ > +#define __ARM_PCI_H__ > + > +#ifdef CONFIG_HAS_PCI > + > +#define pci_to_dev(pcidev) (&(pcidev)->arch.dev) > + > +/* Arch pci dev struct */ > struct arch_pci_dev { > + struct device dev; > }; > > -#endif /* __X86_PCI_H__ */ > +#else /*!CONFIG_HAS_PCI*/ > + > +struct arch_pci_dev { }; > + > +#endif /*!CONFIG_HAS_PCI*/ > +#endif /* __ARM_PCI_H__ */ > -- > 2.17.1 > >
Hi Julien > On 23 Sep 2021, at 3:07 am, Julien Grall <julien.grall.oss@gmail.com> wrote: > > Hi, > > On Wed, 22 Sep 2021, 16:36 Rahul Singh, <rahul.singh@arm.com> wrote: > Compilation error is observed when HAS_PCI is enabled for ARM > architecture. > > In general, when I read "compilation error" I interpret as a user can trigger it in the current staging. > > However, without the full series applied, HAS_PCI cannot be selected on Arm. > > So I think the message is a bit misleading because this is more implementing stubs to enable a feature rather than fixing compilation errors (AFAICT all of them are not fixed here). > > How about the following commit message: > > xen/arm: pci: Add stubs to allow selecting HAS_PCI > > In a follow-up we will enable PCI support in Xen on Arm (i.e select HAS_PCI). > > The generic code expects the arch to implement a few functions: > > <List the functions> > > Note that this is not yet sufficient to enable HAS_PCI and will be addressed in follow-ups. > Ok. I will modify the commit message in next version. Regards, Rahul
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 3d3b97b5b4..44d7cc81fa 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_ARM_32) += arm32/ obj-$(CONFIG_ARM_64) += arm64/ obj-$(CONFIG_ARM_64) += efi/ obj-$(CONFIG_ACPI) += acpi/ +obj-$(CONFIG_HAS_PCI) += pci/ ifneq ($(CONFIG_NO_PLAT),y) obj-y += platforms/ endif diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile new file mode 100644 index 0000000000..a98035df4c --- /dev/null +++ b/xen/arch/arm/pci/Makefile @@ -0,0 +1 @@ +obj-y += pci.o diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c new file mode 100644 index 0000000000..a7a7bc3213 --- /dev/null +++ b/xen/arch/arm/pci/pci.c @@ -0,0 +1,33 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <xen/pci.h> + +/* + * PIRQ event channels are not supported on Arm, so nothing to do. + */ +int arch_pci_clean_pirqs(struct domain *d) +{ + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c index db3b07a571..ee653a9c48 100644 --- a/xen/drivers/passthrough/arm/iommu.c +++ b/xen/drivers/passthrough/arm/iommu.c @@ -135,3 +135,12 @@ void arch_iommu_domain_destroy(struct domain *d) void __hwdom_init arch_iommu_hwdom_init(struct domain *d) { } + +/* + * Unlike x86, Arm doesn't support mem-sharing, mem-paging and log-dirty (yet). + * So there is no restriction to use the IOMMU. + */ +bool arch_iommu_use_permitted(const struct domain *d) +{ + return true; +} diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h index de13359f65..7dd9eb3dba 100644 --- a/xen/include/asm-arm/pci.h +++ b/xen/include/asm-arm/pci.h @@ -1,7 +1,32 @@ -#ifndef __X86_PCI_H__ -#define __X86_PCI_H__ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef __ARM_PCI_H__ +#define __ARM_PCI_H__ + +#ifdef CONFIG_HAS_PCI + +#define pci_to_dev(pcidev) (&(pcidev)->arch.dev) + +/* Arch pci dev struct */ struct arch_pci_dev { + struct device dev; }; -#endif /* __X86_PCI_H__ */ +#else /*!CONFIG_HAS_PCI*/ + +struct arch_pci_dev { }; + +#endif /*!CONFIG_HAS_PCI*/ +#endif /* __ARM_PCI_H__ */
Compilation error is observed when HAS_PCI is enabled for ARM architecture. Add definition for arch_iommu_use_permitted() and arch_pci_clean_pirqs(). pci.c: In function ‘deassign_device’: pci.c:849:49: error: implicit declaration of function ‘pci_to_dev’; did you mean ‘dt_to_dev’? [-Werror=implicit-function-declaration] pci_to_dev(pdev)); pci.c:880: undefined reference to `arch_pci_clean_pirqs’ pci.c:1392: undefined reference to `arch_iommu_use_permitted' Signed-off-by: Rahul Singh <rahul.singh@arm.com> --- Change in v2: - Remove pci_conf_read*(..) dummy implementation - Add in code comment for arch_pci_clean_pirqs() and arch_iommu_use_permitted() - Fixed minor comments --- xen/arch/arm/Makefile | 1 + xen/arch/arm/pci/Makefile | 1 + xen/arch/arm/pci/pci.c | 33 +++++++++++++++++++++++++++++ xen/drivers/passthrough/arm/iommu.c | 9 ++++++++ xen/include/asm-arm/pci.h | 31 ++++++++++++++++++++++++--- 5 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 xen/arch/arm/pci/Makefile create mode 100644 xen/arch/arm/pci/pci.c