Re: [PATCH v5 20/23] KVM: arm64: GICv4.1: Plumb SGI implementation selection in the distributor
From: Zenghui Yu <yuzenghui@huawei.com>
Date: 2020-03-23 08:11:20
Also in:
kvm, kvmarm, lkml
Hi Marc, On 2020/3/20 17:01, Marc Zyngier wrote:
Hi Zenghui, On 2020-03-20 03:53, Zenghui Yu wrote:quoted
Hi Marc, On 2020/3/19 20:10, Marc Zyngier wrote:quoted
quoted
But I wonder that should we use nassgireq to *only* keep track what the guest had written into the GICD_CTLR.nASSGIreq. If not, we may lose the guest-request bit after migration among hosts with different has_gicv4_1 settings.I'm unsure of what you're suggesting here. If userspace tries to set GICD_CTLR.nASSGIreq on a non-4.1 host, this bit will not latch.This is exactly what I *was* concerning about.quoted
Userspace can check that at restore time. Or we could fail the userspace write, which is a bit odd (the bit is otherwise RES0). Could you clarify your proposal?Let's assume two hosts below. 'has_gicv4_1' is true on host-A while it is false on host-B because of lack of HW support or the kernel parameter "kvm-arm.vgic_v4_enable=0". If we migrate guest through A->B->A, we may end-up lose the initial guest-request "nASSGIreq=1" and don't use direct vSGI delivery for this guest when it's migrated back to host-A.My point above is that we shouldn't allow the A->B migration the first place, and fail it as quickly as possible. We don't know what the guest has observed in terms of GIC capability, and it may not have enabled the new flavour of SGIs just yet.
Indeed. I didn't realize it.
quoted hunk ↗ jump to hunk
quoted
This can be "fixed" by keep track of what guest had written into nASSGIreq. And we need to evaluate the need for using direct vSGI for a specified guest by 'has_gicv4_1 && nassgireq'.It feels odd. It means we have more state than the HW normally has. I have an alternative proposal, see below.quoted
But if it's expected that "if userspace tries to set nASSGIreq on a non-4.1 host, this bit will not latch", then this shouldn't be a problem at all.Well, that is the semantics of the RES0 bit. It applies from both guest and userspace. And actually, maybe we can handle that pretty cheaply. If userspace tries to restore GICD_TYPER2 to a value that isn't what KVM can offer, we just return an error. Exactly like we do for GICD_IIDR. Hence the following patch:diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.cb/virt/kvm/arm/vgic/vgic-mmio-v3.c index 28b639fd1abc..e72dcc454247 100644--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c@@ -156,6 +156,7 @@ static int vgic_mmio_uaccess_write_v3_misc(structkvm_vcpu *vcpu, struct vgic_dist *dist = &vcpu->kvm->arch.vgic; switch (addr & 0x0c) { + case GICD_TYPER2: case GICD_IIDR: if (val != vgic_mmio_read_v3_misc(vcpu, addr, len)) return -EINVAL; Being a RO register, writing something that isn't compatible with the possible behaviour of the hypervisor will just return an error.
This is really a nice point to address my concern! I'm happy to see this in v6 now.
What do you think?
I agreed with you, with a bit nervous though. Some old guests (which have no knowledge about GICv4.1 vSGIs and don't care about nASSGIcap at all) will also fail to migrate from A to B, just because now we present two different (unused) GICD_TYPER2 registers to them. Is it a little unfair to them :-) ? Thanks, Zenghui _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel