Re: [PATCH] xen/arm: Do not include in the image functions...

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 06.12.2021 17:40, Julien Grall wrote:
> 
> 
> On 06/12/2021 15:00, Michal Orzel wrote:
>> Hi Julien,
> 
> Hi Michal,
> 
>> On 06.12.2021 15:39, Julien Grall wrote:
>>> Hi Michal,
>>>
>>> On 06/12/2021 14:19, Michal Orzel wrote:
>>>> vtimer_update_irqs, vtimer_update_irq and vcpu_update_evtchn_irq if
>>>> CONFIG_NEW_VGIC is not set.
>>>>
>>>> enter_hypervisor_from_guest is protecting calls to these functions
>>>> with CONFIG_NEW_VGIC but their definitions and declarations are not > 
>>>> protected. This means that we are including them in the image even
>>>> though we are not making use of them. Fix that.
>>>
>>> While I agree, they are only used by the new vGIC, the implementation of 
>>> the functions are not. So I don't think they should be protected by 
>>> CONFIG_NEW_VGIC.
>>>
>>> Actually, I am not convinced they should be protected. But I guess you did 
>>> that for a reason. Would you be able to clarify what is your reason?
>>>
>>  From what I know + what the commit introducing these fucntions states 
>> (b9db96f71a74), the current vGIC does not handle level-triggered vIRQs.
>> The functionality of these functions is only related to new VGIC 
>> implementation which can handle level triggered vIRQs.
> 
> This is a known error in the vGIC implementation which should be resolved 
> before this leads to a disaster.

I just thought that if this error is present for such a long time, there are no 
plans to make current vgic handle level type irqs.
> 
>> So I do not think that these functions are generic and thus I believe they 
>> should be protected.
> 
> None of the functions rely on the internal of the new vGIC. In fact, as I 
> wrote above, the current vGIC ought to be able to handle level-trigger 
> interrupt properly.
> 
> They are not called for the current vGIC because there was concern about the 
> performance impact on each trap (see [1]).
> 
> So I think those functions ought to stay compiled in for everyone.
> 
I'm totally ok with that.

> Cheers,
> 
> [1] 
> https://lore.kernel.org/xen-devel/22601816-8235-7891-b634-4af5348a1337@xxxxxxx/
> 
> 
> 
>>> Cheers,
>>>
>>
>> Cheers,
>> Michal
>>
> 

Cheers,
Michal