RISC-V: KVM: Fix IPI delivery for out-of-order vcpu_id
Subsystems (4)
PATCH 1/1 RISC-V: KVM: Fix IPI delivery for out-of-order vcpu_id
The SBI IPI handler walks vCPUs with kvm_for_each_vcpu(), which iterates by vcpu_idx (creation order) rather than vcpu_id order. Since vcpu_id can be assigned out of order by userspace, a vCPU whose hart_bit falls outside the XLEN-bit hart_mask range may be reached before vCPUs the mask actually targets. In that case the handler jumps to "done" and stops sending IPIs, leaving valid target vCPUs without an interrupt.
Replace the early "goto done" with "continue" so vCPUs outside the hart_mask range are skipped without aborting the loop.
Reproduced with a minimal userspace VMM driving KVM inside a QEMU (RISC-V virt) guest: three vCPUs are created with ids 100, 0, 1 (in creation order) and sbi_send_ipi(hart_mask=bit0, hbase=0) is called from vcpu_id 1. Before this change the IPI is dropped and the ecall returns SBI_ERR_INVALID_PARAM; with this change the IPI is delivered and the ecall returns SBI_SUCCESS.
--- arch/riscv/kvm/vcpu_sbi_replace.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
arch/riscv/kvm/vcpu_sbi_replace.c +10 -2
@@ -64,8 +64,17 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,if (tmp->vcpu_id < hbase)continue ; hart_bit = tmp->vcpu_id - hbase; +/* +* kvm_for_each_vcpu() walks kvm->vcpus[] by +* vcpu_idx, i.e. the creation order, which has +* nothing to do with the vcpu_id (hart id) space +* that the SBI IPI operates on. vcpu_ids need not +* increase along the iteration, so harts outside +* the hart_mask window must be skipped instead of +* aborting the loop. +*/ if (hart_bit >= __riscv_xlen) - goto done; +continue ;if (!(hmask & (1UL << hart_bit)))continue ; }
@@ -76,7 +85,6 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,kvm_riscv_vcpu_pmu_incr_fw (tmp,SBI_PMU_FW_IPI_RCVD ); } -done:if (hbase !=-1UL && (hmask ^ sentmask)) retdata->err_val =SBI_ERR_INVALID_PARAM ;
Signature · 1 line
2.34.1
Quoted 18 lines · Xiaofeng Yuan
The SBI IPI handler walks vCPUs with
kvm_for_each_vcpu(), which iterates byvcpu_idx(creation order) rather thanvcpu_idorder. Sincevcpu_idcan be assigned out of order by userspace, a vCPU whosehart_bitfalls outside the XLEN-bithart_maskrange may be reached before vCPUs the mask actually targets. In that case the handler jumps to "done" and stops sending IPIs, leaving valid target vCPUs without an interrupt.Replace the early "goto done" with "continue" so vCPUs outside the
hart_maskrange are skipped without aborting the loop.Reproduced with a minimal userspace VMM driving KVM inside a QEMU (RISC-V virt) guest: three vCPUs are created with ids 100, 0, 1 (in creation order) and
sbi_send_ipi(hart_mask=bit0, hbase=0)is called fromvcpu_id1. Before this change the IPI is dropped and the ecall returnsSBI_ERR_INVALID_PARAM; with this change the IPI is delivered and the ecall returnsSBI_SUCCESS.
Quoted 4 lines
Fixes 0611f78f83c9 ("riscv: KVM: Fix SBI IPI error generation")Suggested-by Andrew Jones <[email protected]>Signed-off-by Xiaofeng Yuan <[email protected]>
Signature · 1 line
Troy Mitchell
Signature · 3 lines
kvm-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kvm-riscv
Quoted 38 lines · Xiaofeng Yuan
The SBI IPI handler walks vCPUs with
kvm_for_each_vcpu(), which iterates byvcpu_idx(creation order) rather thanvcpu_idorder. Sincevcpu_idcan be assigned out of order by userspace, a vCPU whosehart_bitfalls outside the XLEN-bithart_maskrange may be reached before vCPUs the mask actually targets. In that case the handler jumps to "done" and stops sending IPIs, leaving valid target vCPUs without an interrupt.Replace the early "goto done" with "continue" so vCPUs outside the
hart_maskrange are skipped without aborting the loop.Reproduced with a minimal userspace VMM driving KVM inside a QEMU (RISC-V virt) guest: three vCPUs are created with ids 100, 0, 1 (in creation order) and
sbi_send_ipi(hart_mask=bit0, hbase=0)is called fromvcpu_id1. Before this change the IPI is dropped and the ecall returnsSBI_ERR_INVALID_PARAM; with this change the IPI is delivered and the ecall returnsSBI_SUCCESS.Fixes 0611f78f83c9 ("riscv: KVM: Fix SBI IPI error generation")Suggested-by Andrew Jones <[email protected]>Signed-off-by Xiaofeng Yuan <[email protected]>--- arch/riscv/kvm/vcpu_sbi_replace.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)arch/riscv/kvm/vcpu_sbi_replace.c +4
-0@@ -64,8 +64,17 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,if (tmp->vcpu_id < hbase)continue ; hart_bit = tmp->vcpu_id - hbase; +/* +* kvm_for_each_vcpu() walks kvm->vcpus[] by +* vcpu_idx, i.e. the creation order, which has +* nothing to do with the vcpu_id (hart id) space
arch/riscv/kvm/vcpu_sbi_replace.c static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run, · line 79
Quoted 10 lines
arch/riscv/kvm/vcpu_sbi_replace.c +6
-1+* that the SBI IPI operates on. vcpu_ids need not +* increase along the iteration, so harts outside +* the hart_mask window must be skipped instead of +* aborting the loop. +*/ if (hart_bit >= __riscv_xlen) - goto done; +continue ;if (!(hmask & (1UL << hart_bit)))continue ;
Seems the above two 'if' statements can be combined. Otherwise,
Thanks,
Nutty
Quoted 9 lines
arch/riscv/kvm/vcpu_sbi_replace.c +0
-1}@@ -76,7 +85,6 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,kvm_riscv_vcpu_pmu_incr_fw (tmp,SBI_PMU_FW_IPI_RCVD ); } -done:if (hbase !=-1UL && (hmask ^ sentmask)) retdata->err_val =SBI_ERR_INVALID_PARAM ;
Quoted 19 lines · Xiaofeng Yuan
The SBI IPI handler walks vCPUs with
kvm_for_each_vcpu(), which iterates byvcpu_idx(creation order) rather thanvcpu_idorder. Sincevcpu_idcan be assigned out of order by userspace, a vCPU whosehart_bitfalls outside the XLEN-bithart_maskrange may be reached before vCPUs the mask actually targets. In that case the handler jumps to "done" and stops sending IPIs, leaving valid target vCPUs without an interrupt.Replace the early "goto done" with "continue" so vCPUs outside the
hart_maskrange are skipped without aborting the loop.Reproduced with a minimal userspace VMM driving KVM inside a QEMU (RISC-V virt) guest: three vCPUs are created with ids 100, 0, 1 (in creation order) and
sbi_send_ipi(hart_mask=bit0, hbase=0)is called fromvcpu_id1. Before this change the IPI is dropped and the ecall returnsSBI_ERR_INVALID_PARAM; with this change the IPI is delivered and the ecall returnsSBI_SUCCESS.
Quoted 4 lines
Fixes 0611f78f83c9 ("riscv: KVM: Fix SBI IPI error generation")Suggested-by Andrew Jones <[email protected]>Signed-off-by Xiaofeng Yuan <[email protected]>
LGTM.
Queued this patch for Linux-7.4
Thanks,
Anup
Quoted 38 lines
--- arch/riscv/kvm/vcpu_sbi_replace.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)arch/riscv/kvm/vcpu_sbi_replace.c +10
-3@@ -64,8 +64,17 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,if (tmp->vcpu_id < hbase)continue ; hart_bit = tmp->vcpu_id - hbase; +/* +* kvm_for_each_vcpu() walks kvm->vcpus[] by +* vcpu_idx, i.e. the creation order, which has +* nothing to do with the vcpu_id (hart id) space +* that the SBI IPI operates on. vcpu_ids need not +* increase along the iteration, so harts outside +* the hart_mask window must be skipped instead of +* aborting the loop. +*/ if (hart_bit >= __riscv_xlen) - goto done; +continue ;if (!(hmask & (1UL << hart_bit)))continue ; }@@ -76,7 +85,6 @@ static int kvm_sbi_ext_ipi_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,kvm_riscv_vcpu_pmu_incr_fw (tmp,SBI_PMU_FW_IPI_RCVD ); } -done:if (hbase !=-1UL && (hmask ^ sentmask)) retdata->err_val =SBI_ERR_INVALID_PARAM ; --2.34.1