Re: [PATCH 06/19] xen: Clean up asm-generic/device.h

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Mon Jun 2, 2025 at 4:24 PM CEST, Jan Beulich wrote:
> On 02.06.2025 16:19, Alejandro Vallejo wrote:
>> On Mon Jun 2, 2025 at 9:51 AM CEST, Jan Beulich wrote:
>>> On 30.05.2025 14:02, Alejandro Vallejo wrote:
>>>> --- a/xen/include/asm-generic/device.h
>>>> +++ b/xen/include/asm-generic/device.h
>>>> @@ -6,9 +6,7 @@
>>>>  
>>>>  enum device_type
>>>>  {
>>>> -#ifdef CONFIG_HAS_DEVICE_TREE
>>>>      DEV_DT,
>>>> -#endif
>>>
>>> Why would this enumerator need exposing on a non-DT arch? In fact I would 
>>> have
>>> hoped for ...
>> 
>> A non-DT arch would not include this. x86 doesn't.
>
> Both here and ...
>
>>>>      DEV_PCI
>>>
>>> ... this to be hidden for arch-es not supporting PCI.
>>>
>>> Similar concerns elsewhere in this change.
>> 
>> This file is exclusively used by arches supporting DT to abstract away where
>> the device came from. x86 does not use it at all, and while it wouldn't be
>> impossible to compile-out DEV_PCI, it would needlessly pollute the codebase 
>> with
>> no measurable gain, because the abstractions still need to stay.
>
> ... here: In "xen/include/asm-generic/device.h" there's nothing at all saying
> that this file is a DT-only one. Instead there is something in there saying
> that it's suitable to use in the entirely "generic" case.
>
> Jan

Try to use it from x86 and observe the build system catch fire. It could be made
to not go on fire, but it implies heavy refactoring in x86 (particularly IOMMU
code) for no good reason because there's no devices in a DTB to disambiguate.

How about adding this to the top of the header?

```
 /*
  * This header helps DTB-based architectures abstract away where a particular
  * device comes from; be it the DTB itself or enumerated on a PCI bus. 
  */

  [snip]

 #ifndef CONFIG_HAS_DEVICE_TREE
 #error "Header meant to be used exclusively by DTB-base architectures."
 #endif
```

Cheers,
Alejandro