[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 33/39] ARM: new VGIC: Add preliminary stub implementation

Hi Stefano,

On 27/03/18 23:48, Stefano Stabellini wrote:
On Wed, 21 Mar 2018, Andre Przywara wrote:
The ARM arch code requires an interrupt controller emulation to implement
vgic_clear_pending_irqs(), although it is suspected that it is actually
not necessary. Go with a stub for now to make the linker happy.

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
Reviewed-by: Julien Grall <julien.grall@xxxxxxx>
  xen/arch/arm/vgic/vgic.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index 23b8abfc5e..b70fdaaecb 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -791,6 +791,14 @@ void gic_dump_vgic_info(struct vcpu *v)
      spin_unlock_irqrestore(&v->arch.vgic.ap_list_lock, flags);
+void vgic_clear_pending_irqs(struct vcpu *v)
+    /*
+     * TODO: It is unclear whether we really need this, so we might instead
+     * remove it on the caller site.
+     */

This is OK for now.

However, thinking about this issue, is it possible for a vcpu to send an
interrupt to an offline vcpu, maybe an SGI? What would happen in that
case? It looks like that vgic_mmio_write_sgir would allow it. Otherwise,
a vcpu could cause the generation of a physical interrupt, an SPI,
targeting an offline vcpu.

Maybe we should WARN in case ap_list is not empty?

The interrupts would be delivered when the vCPU is going online. It does not seem against the specification. Actually from the spec, it is valid to route an interrupt any vCPU (even non-existent one).

However, as I said on a previous version of this patch, the implementation on the current vGIC is just plain wrong for a few reasons: - lr_mask is reset but the LRs are not. This means when we context switch back, the LR might still be written and injecting unexpected interrupt (whoops). - both lists (inflight and pending) are cleared which means that a physical interrupt pending on that vCPU is lost forever (stay active in the physical so never going to fire again).

Furthermore, I don't think that Xen business to reset the GIC on cpu_on. If anything should be done, then is it on CPU_off to migrate the current interrupts to another vCPU. But IIRC the OS is responsible for that.

One the same note, similar problem would happen because of vgic_inject_irq would bail out on offline vCPU. This means that interrupt may be lost forever.

We are quite lucky that we don't use PSCI on/off very often in Xen.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.