On 2020/11/24 21:12, Shenming Lu wrote:

On 2020/11/24 16:44, Marc Zyngier wrote:

quoted

On 2020-11-24 08:10, Shenming Lu wrote:

quoted

On 2020/11/23 17:27, Marc Zyngier wrote:

quoted

On 2020-11-23 06:54, Shenming Lu wrote:

quoted

From: Zenghui Yu <yuzenghui@huawei.com>

When setting the forwarding path of a VLPI, it is more consistent to

I'm not sure it is more consistent. It is a *new* behaviour, because it only
matters for migration, which has been so far unsupported.

Alright, consistent may not be accurate...
But I have doubt that whether there is really no need to transfer the
pending states
from kvm'vgic to VPT in set_forwarding regardless of migration, and the similar
for unset_forwarding.

If you have to transfer that state outside of the a save/restore, it means that
you have missed the programming of the PCI endpoint. This is an established
restriction that the MSI programming must occur *after* the translation has
been established using MAPI/MAPTI (see the large comment at the beginning of
vgic-v4.c).

If you want to revisit this, fair enough. But you will need a lot more than
just opportunistically transfer the pending state.

Thanks, I will look at what you mentioned.

quoted

quoted

quoted

quoted

also transfer the pending state from irq->pending_latch to VPT (especially
in migration, the pending states of VLPIs are restored into kvm’s vgic
first). And we currently send "INT+VSYNC" to trigger a VLPI to pending.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Shenming Lu <redacted>
---
 arch/arm64/kvm/vgic/vgic-v4.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index b5fa73c9fd35..cc3ab9cea182 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c

@@ -418,6 +418,18 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq,

     irq->host_irq    = virq;
     atomic_inc(&map.vpe->vlpi_count);

+    /* Transfer pending state */
+    ret = irq_set_irqchip_state(irq->host_irq,
+                    IRQCHIP_STATE_PENDING,
+                    irq->pending_latch);
+    WARN_RATELIMIT(ret, "IRQ %d", irq->host_irq);
+
+    /*
+     * Let it be pruned from ap_list later and don't bother
+     * the List Register.
+     */
+    irq->pending_latch = false;

It occurs to me that calling into irq_set_irqchip_state() for a large
number of interrupts can take a significant amount of time. It is also
odd that you dump the VPT with the VPE unmapped, but rely on the VPE
being mapped for the opposite operation.

Shouldn't these be symmetric, all performed while the VPE is unmapped?
It would also save a lot of ITS traffic.

My thought was to use the existing interface directly without unmapping...

If you want to unmap the vPE and poke the VPT here, as I said in the cover
letter, set/unset_forwarding might also be called when all devices are running
at normal run time, in which case the unmapping of the vPE is not allowed...

No, I'm suggesting that you don't do anything here, but instead as a by-product
of restoring the ITS tables. What goes wrong if you use the
KVM_DEV_ARM_ITS_RESTORE_TABLE backend instead?

There is an issue if we do it in the restoring of the ITS tables: the transferring
of the pending state needs the irq to be marked as hw before, which is done by the
pass-through device, but the configuring of the forwarding path of the VLPI depends
on the restoring of the vgic first... It is a circular dependency.

Hi Marc,

We are pondering over this problem these days, but still don't get a good solution...
Could you give us some advice on this?

Or could we move the restoring of the pending states (include the sync from guest
RAM and the transfer to HW) to the GIC VM state change handler, which is completely
corresponding to save_pending_tables (more symmetric?) and don't expose GICv4...

Thanks,
Shenming

quoted

quoted

Another possible solution is to add a new dedicated interface to QEMU
to transfer
these pending states to HW in GIC VM state change handler corresponding to
save_pending_tables?

Userspace has no way to know we use GICv4, and I intend to keep it
completely out of the loop. The API is already pretty tortuous, and
I really don't want to add any extra complexity to it.

Thanks,

        M.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel