Re: [PATCH v1 12/29] xen/asm-generic: introduce stub header pci.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

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