Re: [PATCH v2] xen/arm: implement GICD_I[S/C]ACTIVER reads

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

On 9 Apr 2020, at 14:00, Julien Grall <julien@xxxxxxx> wrote:

On 09/04/2020 13:56, Julien Grall wrote:

On 09/04/2020 02:26, Stefano Stabellini wrote:

On Tue, 7 Apr 2020, Julien Grall wrote:

I don’t know what maintenance IRQs are, but if they only happen
intermittently, it’s possible that you’d never get more than a single
one in a latency-critical IRQ routine; and as such, the variatibility in
execution time (jitter) wouldn’t be an issue in practice.  But every
time you add a new unblockable IPI, you increase this jitter;
particularly if this unblockable IPI might be repeated an arbitrary
number of times.
(Stefano, let me know if I’ve misunderstood something.)
So stepping back a moment, here’s all the possible ideas that I think
have been discussed (or are there implicitly) so far.
1. [Default] Do nothing; guests using this register continue crashing
2. Make the I?ACTIVER registers RZWI.
3. Make I?ACTIVER return the most recent known value; i.e. KVM’s current
behavior (as far as we understand it)
4. Use a simple IPI with do_noop to update I?ACTIVER
4a.  Use an IPI, but come up with clever tricks to avoid interrupting
guests handling IRQs.
5. Trap to Xen on guest EOI, so that we know when the
6. Some clever paravirtualized option

7. Use an IPI if we are confident the interrupts may be active.

I don’t understand this one.  How is it different than 4 or 4a?  And in
particular, how does it evaluate on the “how much additional design work
would it take” criteria?

Let me start with, if we want to have a vGIC to rule them all, then I am
afraid you are going to have to compromise. We can discuss about tailoring the
vGIC but I think before that we need a vGIC that is faithfull with the spec
(e.g differentiating level vs edge interrupts, handling activer...). Note that
Arm spent some effort to get a new vGIC merged but this was never made a first
class citizen.

However, even if you tailor the vGIC, you are not going to be able to avoid
IPI in some occasions. This would happen when using event channels, in-guest
IPI... Another example is when enabling an interrupt, although I realize that
our vGIC does not do it today meaning that a pending interrupt while disabled
will not be forwarded until the vCPU exit.

Furthermore, implementing a write to I{C,S}ACTIVER (to activate/de-activate)
is going to be very difficult (to not say impossible) to do without IPI.

If you are worry about a vCPU been interrupted in critical section, then I
think you should tailor your guest to avoid using those registers.

Let's call it option 8 "tell the user that she needs to modify her
kernel."

An alternative would be to use spinlock/mutex within the code to prevent a
VCPU to access the vGIC registers while another vCPU don't want to be
interrupted.

Regarding, 4a. The only way I could think of would be to trap the instructions
that mask/unmask interrupts. If I read correctly the Armv8 spec, it is not
possible to do it.

7. is basically 4.a the goal would be to avoid interrupts the vCPU has much as
possible but you may be wrong sometimes. Obviously we want to be correct most
of the times.

I understand this may not be the ideal solution, but this is probably the best
we could come with and does not involve a lot of work because we have already
all the information in place (we know when an interrupt was injected to a
vCPU).

The next best solution is to prevent in your guest to modify some registers of
the vGIC at the same time as another vCPU is in a critical section.

Let's call this option 9.


I am just thinking out loud here :)

Thank you for thinking out loud. Sadly, as I pointed out before, there are other part of the vGIC facing the same problems (e.g I{S,C}ENABLER, sending SGIs, sending event channels).
So can you enlighten me why I{S,C}ENABLER is that much a concern over the other?

To be clear, I am not saying I{S,C}ENABLER should not be a concern. But I would prefer if we focus on a generic solution if the problem is wider.

It seems that the problem is a bit bigger then expected and will need more discussion and thinking.

I did some research on my side and there are several design possible depending on what should be the goal performance or real-time behaviour (going from just give the status we have to fire a service interrupts when any interrupts is acked by a guest to refresh our internal status).

In the short term, could we not agree to fix the typo by returning always 0 and start a discussion on the vgic implementation ?

Cheers

Bertrand