Re: [PATCH 4/8] Arm: prune #include-s needed by domain.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.07.2020 11:09, Julien Grall wrote:
> 
> 
> On 20/07/2020 09:17, Jan Beulich wrote:
>> On 17.07.2020 16:44, Julien Grall wrote:
>>> On 15/07/2020 11:39, Jan Beulich wrote:
>>>> --- a/xen/include/asm-arm/domain.h
>>>> +++ b/xen/include/asm-arm/domain.h
>>>> @@ -2,7 +2,7 @@
>>>>    #define __ASM_DOMAIN_H__
>>>>    
>>>>    #include <xen/cache.h>
>>>> -#include <xen/sched.h>
>>>> +#include <xen/timer.h>
>>>>    #include <asm/page.h>
>>>>    #include <asm/p2m.h>
>>>>    #include <asm/vfp.h>
>>>> @@ -11,8 +11,6 @@
>>>>    #include <asm/vgic.h>
>>>>    #include <asm/vpl011.h>
>>>>    #include <public/hvm/params.h>
>>>> -#include <xen/serial.h>
>>>
>>> While we don't need the rbtree.h, we technically need serial.h for using
>>> vuart_info.
>>
>> The only reference to it is
>>
>>          const struct vuart_info     *info;
>>
>> which doesn't require a definition nor even a forward declaration
>> of struct vuart_info. It should just be source files instantiating
>> a struct or de-referencing pointers to one that actually need to
>> see the full declaration. 
> 
> Ah yes. I got confused because you introduced a forward declaration of 
> struct vcpu. But this is because you need it to declare the function 
> prototype.

As a result - are you happy for the change to go in with Stefano's
ack then?

>> The only source file doing so (vuart.c)
>> already includes xen/serial.h. (In fact, it being just a single
>> source file doing so, the struct definition could [and imo should]
>> be moved there. The type can be entirely opaque to the rest of the
>> code base, as - over time - we've been doing for other structs.)
> 
> There are definitely more use of vuart_info within the code base. All 
> the UART on Arm will fill up the structure (see drivers/char/pl011.c) 
> for instance.
> 
> So the definition is in the correct place.

Hmm, I will admit I judged from the uses of ->arch.vuart.info alone.

Jan