Re: [PATCH 2/5] xen/arm: Add asm/domain.h include to kernel.h

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 11 Aug 2023, at 15:13, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 11/08/2023 14:40, Luca Fancellu wrote:
>>> On 11 Aug 2023, at 13:56, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> Hi Luca,
>>> 
>>> On 08/08/2023 09:00, Luca Fancellu wrote:
>>>> Add asm/domain.h that is defining the type 'enum domain_type', it
>>>> is needed on arm64 build where this type is used for a member of
>>>> the structure kernel_info.
>>> 
>>> I read "needed" as in it Xen build is broken. But AFAIK, this is more a 
>>> latent issue if someone else want to include the header. Is that correct?
>> Yes correct
>>> 
>>> If so, how about:
>>> 
>>> The 'enum domain_type' is defined by 'asm/domain.h' which is not included 
>>> (directly or indirectly) by 'asm/kernel.h'.
>>> 
>>> This currently doesn't break the compilation because asm/domain.h will 
>>> included by the user of 'kernel.h'. But it would be better to avoid relying 
>>> on it. So add the include in 'asm/domain.h'.
>> Yeah much better, should I push a v2?
> 
> No. I can deal with it on commit.

Ok thank you for doing that

> 
>>> 
>>>> Fixes: 66e994a5e74f ("xen: arm64: add guest type to domain field.")
>>> 
>>> While we aim to have header self-contained, this has never been a guarantee 
>>> in Xen. So I would argue this is not a fix in the sense it someone would 
>>> want to ingest it in there tree.
>> Ok I see, I thought it could be linked to the issue about sorting headers 
>> that led to build breakage, but I’ve
> 
> I am probably missing something here. Which issue are you referring to? Is it 
> a follow-up patch that will sort headers?

It’s an issue I’ve faced when trying to sort automatically the include using 
clang-format, I’ve seen issues building domain_build.c after sorting the 
headers in the way we expect from coding style, I thought was related to some 
headers not being self-contained.

> 
>> not investigated further so I would be ok to drop the Fixes:
> 
> Cheers,
> 
> -- 
> Julien Grall