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

[Julien Grall <julien@xxxxxxx>]

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.
> 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.

Cheers,

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


> > Cheers,
> Cheers,
> Michal
--
Julien Grall