| Message ID | 597a482c70fef196e245a5d898ea6314a0c479ca.1694702259.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce stub headers necessary for full Xen build | expand |
On 14.09.2023 16:56, Oleksii Kurochko wrote: > --- /dev/null > +++ b/xen/include/asm-generic/pci.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +#ifndef __ASM_GENERIC_PCI_H__ > +#define __ASM_GENERIC_PCI_H__ > + > +struct arch_pci_dev { > +}; > + > +#endif /* __ASM_GENERIC_PCI_H__ */ While more involved, I still wonder whether xen/pci.h could also avoid including asm/pci.h when !HAS_PCI. Of course there's more than just the #include which then would need #ifdef-ing out. Jan
On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: > While more involved, I still wonder whether xen/pci.h could also > avoid > including asm/pci.h when !HAS_PCI. Of course there's more than just > the > #include which then would need #ifdef-ing out. It looks like we can get with #ifdef-ing. I'll push a separate patch for xen/pci.h. It will probably need to remove usage of <asm/pci.h> everywhere or #ifdef-ing it too. Which option will be better? ~ Oleksii
On 23.10.2023 12:50, Oleksii wrote: > On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: >> While more involved, I still wonder whether xen/pci.h could also >> avoid >> including asm/pci.h when !HAS_PCI. Of course there's more than just >> the >> #include which then would need #ifdef-ing out. > It looks like we can get with #ifdef-ing. I'll push a separate patch > for xen/pci.h. > > It will probably need to remove usage of <asm/pci.h> everywhere or > #ifdef-ing it too. > Which option will be better? What's "everywhere" here? The only non-arch-dependent use I can spot is in xen/pci.h. Jan
On Mon, 2023-10-23 at 13:58 +0200, Jan Beulich wrote: > On 23.10.2023 12:50, Oleksii wrote: > > On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: > > > While more involved, I still wonder whether xen/pci.h could also > > > avoid > > > including asm/pci.h when !HAS_PCI. Of course there's more than > > > just > > > the > > > #include which then would need #ifdef-ing out. > > It looks like we can get with #ifdef-ing. I'll push a separate > > patch > > for xen/pci.h. > > > > It will probably need to remove usage of <asm/pci.h> everywhere or > > #ifdef-ing it too. > > Which option will be better? > > What's "everywhere" here? The only non-arch-dependent use I can spot > is in xen/pci.h. It looks you are right. I wrote everywhere because of "xen/drivers/passthrough/vtd/quirks.c" but it is arch-dependent. So , yes, only xen/pci.h should be updated. ~ Oleksii
Hello Jan, On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: > On 14.09.2023 16:56, Oleksii Kurochko wrote: > > --- /dev/null > > +++ b/xen/include/asm-generic/pci.h > > @@ -0,0 +1,18 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +#ifndef __ASM_GENERIC_PCI_H__ > > +#define __ASM_GENERIC_PCI_H__ > > + > > +struct arch_pci_dev { > > +}; > > + > > +#endif /* __ASM_GENERIC_PCI_H__ */ > > While more involved, I still wonder whether xen/pci.h could also > avoid > including asm/pci.h when !HAS_PCI. Of course there's more than just > the > #include which then would need #ifdef-ing out. > > Jan It looks like we can do that but only one question should be resolved. In ARM case, in <asm/pci.h> there is !HAS_PCI branch: #else /*!CONFIG_HAS_PCI*/ struct arch_pci_dev { }; static always_inline bool is_pci_passthrough_enabled(void) { return false; } struct pci_dev; static inline void arch_pci_init_pdev(struct pci_dev *pdev) {} static inline int pci_get_host_bridge_segment(const struct dt_device_node *node, uint16_t *segment) { ASSERT_UNREACHABLE(); return -EINVAL; } static inline int pci_get_new_domain_nr(void) { ASSERT_UNREACHABLE(); return -1; } #endif /*!CONFIG_HAS_PCI*/ And if is_pci_passthrough_enabled(), arch_pci_init_pdev() is used by all architrectures but pci_get_host_bridge_segment() and pci_get_new_domain_nr() is ARM specific. Does it make sense to add them to <xen/pci.h> and ifdef them? ~ Oleksii
On 30.10.2023 17:34, Oleksii wrote: > Hello Jan, > > On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: >> On 14.09.2023 16:56, Oleksii Kurochko wrote: >>> --- /dev/null >>> +++ b/xen/include/asm-generic/pci.h >>> @@ -0,0 +1,18 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +#ifndef __ASM_GENERIC_PCI_H__ >>> +#define __ASM_GENERIC_PCI_H__ >>> + >>> +struct arch_pci_dev { >>> +}; >>> + >>> +#endif /* __ASM_GENERIC_PCI_H__ */ >> >> While more involved, I still wonder whether xen/pci.h could also >> avoid >> including asm/pci.h when !HAS_PCI. Of course there's more than just >> the >> #include which then would need #ifdef-ing out. >> >> Jan > > It looks like we can do that but only one question should be resolved. > In ARM case, in <asm/pci.h> there is !HAS_PCI branch: > > #else /*!CONFIG_HAS_PCI*/ > > struct arch_pci_dev { }; > > static always_inline bool is_pci_passthrough_enabled(void) > { > return false; > } > > struct pci_dev; > > static inline void arch_pci_init_pdev(struct pci_dev *pdev) {} > > static inline int pci_get_host_bridge_segment(const struct > dt_device_node *node, > uint16_t *segment) > { > ASSERT_UNREACHABLE(); > return -EINVAL; > } > > static inline int pci_get_new_domain_nr(void) > { > ASSERT_UNREACHABLE(); > return -1; > } > > #endif /*!CONFIG_HAS_PCI*/ > > And if is_pci_passthrough_enabled(), arch_pci_init_pdev() is used by > all architrectures but pci_get_host_bridge_segment() and > pci_get_new_domain_nr() is ARM specific. > Does it make sense to add them to <xen/pci.h> and ifdef them? Counter question: Is the arch_pci_init_pdev() stub actually needed? The sole caller looks to be in a file which is only built when HAS_PCI=y. For the Arm-only stubs (which are called from Arm-specific code afaics) all it would take is that the respective .c files include asm/pci.h (possibly alongside xen/pci.h). Jan
On Mon, 2023-10-30 at 17:43 +0100, Jan Beulich wrote: > On 30.10.2023 17:34, Oleksii wrote: > > Hello Jan, > > > > On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote: > > > On 14.09.2023 16:56, Oleksii Kurochko wrote: > > > > --- /dev/null > > > > +++ b/xen/include/asm-generic/pci.h > > > > @@ -0,0 +1,18 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > > +#ifndef __ASM_GENERIC_PCI_H__ > > > > +#define __ASM_GENERIC_PCI_H__ > > > > + > > > > +struct arch_pci_dev { > > > > +}; > > > > + > > > > +#endif /* __ASM_GENERIC_PCI_H__ */ > > > > > > While more involved, I still wonder whether xen/pci.h could also > > > avoid > > > including asm/pci.h when !HAS_PCI. Of course there's more than > > > just > > > the > > > #include which then would need #ifdef-ing out. > > > > > > Jan > > > > It looks like we can do that but only one question should be > > resolved. > > In ARM case, in <asm/pci.h> there is !HAS_PCI branch: > > > > #else /*!CONFIG_HAS_PCI*/ > > > > struct arch_pci_dev { }; > > > > static always_inline bool is_pci_passthrough_enabled(void) > > { > > return false; > > } > > > > struct pci_dev; > > > > static inline void arch_pci_init_pdev(struct pci_dev *pdev) {} > > > > static inline int pci_get_host_bridge_segment(const struct > > dt_device_node *node, > > uint16_t *segment) > > { > > ASSERT_UNREACHABLE(); > > return -EINVAL; > > } > > > > static inline int pci_get_new_domain_nr(void) > > { > > ASSERT_UNREACHABLE(); > > return -1; > > } > > > > #endif /*!CONFIG_HAS_PCI*/ > > > > And if is_pci_passthrough_enabled(), arch_pci_init_pdev() is used > > by > > all architrectures but pci_get_host_bridge_segment() and > > pci_get_new_domain_nr() is ARM specific. > > Does it make sense to add them to <xen/pci.h> and ifdef them? > > Counter question: Is the arch_pci_init_pdev() stub actually needed? > The sole caller looks to be in a file which is only built when > HAS_PCI=y. You are right. It seems that there is no need for pci_init_pdev() stub. > > For the Arm-only stubs (which are called from Arm-specific code > afaics) > all it would take is that the respective .c files include asm/pci.h > (possibly alongside xen/pci.h). We can do in that way. Thanks. ~ Oleksii
diff --git a/xen/include/asm-generic/pci.h b/xen/include/asm-generic/pci.h new file mode 100644 index 0000000000..b577ee105f --- /dev/null +++ b/xen/include/asm-generic/pci.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_GENERIC_PCI_H__ +#define __ASM_GENERIC_PCI_H__ + +struct arch_pci_dev { +}; + +#endif /* __ASM_GENERIC_PCI_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: BSD + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ +
The patch introduces header stub necessry for full Xen build. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/include/asm-generic/pci.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 xen/include/asm-generic/pci.h