| Message ID | 20230920181731.2232453-1-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Heh, "and other fixes" is quite the understatement :) This looks good to me, only issues I've spotted so far are a few typos that are easty enough to fix when I apply the series. I'll give it a couple days for folks to have a look, but otherwise intend on picking this up for 6.7. On Wed, Sep 20, 2023 at 07:17:20PM +0100, Marc Zyngier wrote: > This is a follow-up on [1], which contains the original patches, only > augmented with a bunch of fixes after Zenghui pointed out that I was > inadvertently fixing bugs (yay!), but that there were plenty more. > > The core of the issue is that we tend to confuse vcpu_id with > vcpu_idx. The first is a userspace-provided value, from which we > derive the default MPIDR_EL1 value, while the second is something that > used to represent the position in the vcpu array, and is now more of > an internal identifier.
Hi Marc, On 2023/9/21 2:17, Marc Zyngier wrote: > So this series, on top of trying to optimise SGI injection, also aims > at disambiguating this mess, and documenting some of the implicit > requirements for userspace. Apart from the minor comments, this series looks good. Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
diff --git a/kvm-cpu.c b/kvm-cpu.c index 1c566b3..520d759 100644 --- a/kvm-cpu.c +++ b/kvm-cpu.c @@ -287,7 +287,7 @@ int kvm_cpu__init(struct kvm *kvm) return -ENOMEM; } - for (i = 0; i < kvm->nrcpus; i++) { + for (i = kvm->nrcpus - 1; i >= 0; i--) { kvm->cpus[i] = kvm_cpu__arch_init(kvm, i); if (!kvm->cpus[i]) { pr_err("unable to initialize KVM VCPU");
This is a follow-up on [1], which contains the original patches, only augmented with a bunch of fixes after Zenghui pointed out that I was inadvertently fixing bugs (yay!), but that there were plenty more. The core of the issue is that we tend to confuse vcpu_id with vcpu_idx. The first is a userspace-provided value, from which we derive the default MPIDR_EL1 value, while the second is something that used to represent the position in the vcpu array, and is now more of an internal identifier. To convince oneself that things are terminally broken, it is easy enough to run a kvmtool guest with the following patchlet: and witness that the resulting VM doesn't boot. The only combination we support is to have vcpu_id == vcpu_idx. B0rken. So this series, on top of trying to optimise SGI injection, also aims at disambiguating this mess, and documenting some of the implicit requirements for userspace. With that, a VM created with vcpu upside down boots correctly. * From v1 [1]: - Added a bunch of patches fixing the vcpu_id[x] ambiguity - Added a documentation update spelling out some extra ordering requirements - Collected RBs/TBs, with thanks [1] https://lore.kernel.org/r/20230907100931.1186690-1-maz@kernel.org Marc Zyngier (11): KVM: arm64: vgic: Make kvm_vgic_inject_irq() take a vcpu pointer KVM: arm64: vgic-its: Treat the collection target address as a vcpu_id KVM: arm64: vgic-v3: Refactor GICv3 SGI generation KVM: arm64: vgic-v2: Use cpuid from userspace as vcpu_id KVM: arm64: vgic: Use vcpu_idx for the debug information KVM: arm64: Use vcpu_idx for invalidation tracking KVM: arm64: Simplify kvm_vcpu_get_mpidr_aff() KVM: arm64: Build MPIDR to vcpu index cache at runtime KVM: arm64: Fast-track kvm_mpidr_to_vcpu() when mpidr_data is available KVM: arm64: vgic-v3: Optimize affinity-based SGI injection KVM: arm64: Clarify the ordering requirements for vcpu/RD creation .../virt/kvm/devices/arm-vgic-v3.rst | 7 + arch/arm64/include/asm/kvm_emulate.h | 2 +- arch/arm64/include/asm/kvm_host.h | 28 ++++ arch/arm64/kvm/arch_timer.c | 2 +- arch/arm64/kvm/arm.c | 90 +++++++++-- arch/arm64/kvm/pmu-emul.c | 2 +- arch/arm64/kvm/vgic/vgic-debug.c | 6 +- arch/arm64/kvm/vgic/vgic-irqfd.c | 2 +- arch/arm64/kvm/vgic/vgic-its.c | 21 ++- arch/arm64/kvm/vgic/vgic-kvm-device.c | 2 +- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 142 +++++++----------- arch/arm64/kvm/vgic/vgic.c | 12 +- include/kvm/arm_vgic.h | 4 +- 13 files changed, 195 insertions(+), 125 deletions(-)