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

[Jan Beulich <jbeulich@xxxxxxxx>]

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