Re: [PATCH v2 01/15] xen: Clean up asm-generic/device.h

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Fri Jun 6, 2025 at 12:03 PM CEST, Jan Beulich wrote:
> On 06.06.2025 11:55, Alejandro Vallejo wrote:
>> On Fri Jun 6, 2025 at 8:51 AM CEST, Jan Beulich wrote:
>>> On 05.06.2025 21:47, Alejandro Vallejo wrote:
>>>> --- a/xen/include/asm-generic/device.h
>>>> +++ b/xen/include/asm-generic/device.h
>>>> @@ -1,14 +1,20 @@
>>>>  /* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * This header helps DTB-based architectures abstract away where a 
>>>> particular
>>>> + * device came from, be it the DTB itself or enumerated on a PCI bus.
>>>> + */
>>>>  #ifndef __ASM_GENERIC_DEVICE_H__
>>>>  #define __ASM_GENERIC_DEVICE_H__
>>>>  
>>>> +#ifndef CONFIG_HAS_DEVICE_TREE
>>>> +#error "Header for exclusive use of DTB-based architectures"
>>>> +#endif
>>>> +
>>>>  #include <xen/stdbool.h>
>>>>  
>>>>  enum device_type
>>>>  {
>>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>>>      DEV_DT,
>>>> -#endif
>>>>      DEV_PCI
>>>>  };
>>>
>>> My objection to these changes remains; as a generic header it ought to be 
>>> what
>>> that attribute says - generic.
>> 
>> It is generic for any architecture where platform DTs exist (that is, 
>> anything
>> but x86).
>
> Here you're limiting things to what Xen presently "knows". I'm sure there are
> other architectures where DT is entirely unknown.

You'll struggle a lot to find one such arch unless you want to revive the
itanium port, and even then itanium would simply re-use x86's asm override.

>
> Furthermore isn't the work here part of the hyperlaunch effort, where DT will
> be introduced to x86? Hence "anything but" isn't quite right either then.
>
> Jan

Not a general DTB, no. A DTB defines the platform (in the same vein as a ACPI/
DSDT), with all platform devices enumerated there. x86 will not get any of that.

Without devices defined in the DTB, my original point still stands.

In general, if you don't have DT devices you typedef device_t to pci_dev to
avoid these wrappers altogether, and if you do you don't and the ifdef guards
stop having meaning.

If you remain unconvinced and others don't pitch in I'll just drop it from v3.
As I said, it's not essential.

Cheers,
Alejandro