From: Paolo Bonzini pbonzini@redhat.com
[ Upstream commit fa13843d1565d4c5b3aeb9be3343b313416bef46 ]
If allocation of rmaps fails, but some of the pointers have already been written, those pointers can be cleaned up when the memslot is freed, or even reused later for another attempt at allocating the rmaps. Therefore there is no need to WARN, as done for example in memslot_rmap_alloc, but the allocation *must* be skipped lest KVM will overwrite the previous pointer and will indeed leak memory.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/kvm/x86.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4b0e866e9f08..60d9aa0ab389 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11341,7 +11341,8 @@ static int memslot_rmap_alloc(struct kvm_memory_slot *slot, int lpages = gfn_to_index(slot->base_gfn + npages - 1, slot->base_gfn, level) + 1;
- WARN_ON(slot->arch.rmap[i]); + if (slot->arch.rmap[i]) + continue;
slot->arch.rmap[i] = kvcalloc(lpages, sz, GFP_KERNEL_ACCOUNT); if (!slot->arch.rmap[i]) {
From: Sean Christopherson seanjc@google.com
[ Upstream commit 9139a7a64581c80d157027ae20e86f2f24d4292c ]
WARN if the static keys used to track if any vCPU has disabled its APIC are left elevated at module exit. Unlike the underflow case, nothing in the static key infrastructure will complain if a key is left elevated, and because an elevated key only affects performance, nothing in KVM will fail if either key is improperly incremented.
Signed-off-by: Sean Christopherson seanjc@google.com Message-Id: 20211013003554.47705-3-seanjc@google.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/kvm/lapic.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index ba5a27879f1d..18cb699b0a14 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2942,5 +2942,7 @@ int kvm_apic_accept_events(struct kvm_vcpu *vcpu) void kvm_lapic_exit(void) { static_key_deferred_flush(&apic_hw_disabled); + WARN_ON(static_branch_unlikely(&apic_hw_disabled.key)); static_key_deferred_flush(&apic_sw_disabled); + WARN_ON(static_branch_unlikely(&apic_sw_disabled.key)); }
On 25/10/21 22:38, Sasha Levin wrote:
From: Sean Christopherson seanjc@google.com
[ Upstream commit 9139a7a64581c80d157027ae20e86f2f24d4292c ]
WARN if the static keys used to track if any vCPU has disabled its APIC are left elevated at module exit. Unlike the underflow case, nothing in the static key infrastructure will complain if a key is left elevated, and because an elevated key only affects performance, nothing in KVM will fail if either key is improperly incremented.
Signed-off-by: Sean Christopherson seanjc@google.com Message-Id: 20211013003554.47705-3-seanjc@google.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/lapic.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index ba5a27879f1d..18cb699b0a14 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2942,5 +2942,7 @@ int kvm_apic_accept_events(struct kvm_vcpu *vcpu) void kvm_lapic_exit(void) { static_key_deferred_flush(&apic_hw_disabled);
- WARN_ON(static_branch_unlikely(&apic_hw_disabled.key)); static_key_deferred_flush(&apic_sw_disabled);
- WARN_ON(static_branch_unlikely(&apic_sw_disabled.key)); }
NACK
Paolo
From: Hao Xiang hao.xiang@linux.alibaba.com
[ Upstream commit d61863c66f9b443192997613cd6aeca3f65cc313 ]
Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
We can remove redundant handling of bus lock vmexit. Unconditionally Set exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
Signed-off-by: Hao Xiang hao.xiang@linux.alibaba.com Message-Id: 1634299161-30101-1-git-send-email-hao.xiang@linux.alibaba.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/kvm/vmx/vmx.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 55de1eb135f9..87150b3c9c5f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5551,9 +5551,13 @@ static int handle_encls(struct kvm_vcpu *vcpu)
static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu) { - vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK; - vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK; - return 0; + /* + * Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK + * VM-Exits. Unconditionally set the flag here and leave the handling to + * vmx_handle_exit(). + */ + to_vmx(vcpu)->exit_reason.bus_lock_detected = true; + return 1; }
/* @@ -6039,9 +6043,8 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) int ret = __vmx_handle_exit(vcpu, exit_fastpath);
/* - * Even when current exit reason is handled by KVM internally, we - * still need to exit to user space when bus lock detected to inform - * that there is a bus lock in guest. + * Exit to user space when bus lock detected to inform that there is + * a bus lock in guest. */ if (to_vmx(vcpu)->exit_reason.bus_lock_detected) { if (ret > 0)
On 25/10/21 22:38, Sasha Levin wrote:
From: Hao Xiang hao.xiang@linux.alibaba.com
[ Upstream commit d61863c66f9b443192997613cd6aeca3f65cc313 ]
Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
We can remove redundant handling of bus lock vmexit. Unconditionally Set exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
Signed-off-by: Hao Xiang hao.xiang@linux.alibaba.com Message-Id: 1634299161-30101-1-git-send-email-hao.xiang@linux.alibaba.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/vmx/vmx.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 55de1eb135f9..87150b3c9c5f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5551,9 +5551,13 @@ static int handle_encls(struct kvm_vcpu *vcpu) static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu) {
- vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
- vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
- return 0;
- /*
* Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
* VM-Exits. Unconditionally set the flag here and leave the handling to
* vmx_handle_exit().
*/
- to_vmx(vcpu)->exit_reason.bus_lock_detected = true;
- return 1; }
/* @@ -6039,9 +6043,8 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) int ret = __vmx_handle_exit(vcpu, exit_fastpath); /*
* Even when current exit reason is handled by KVM internally, we
* still need to exit to user space when bus lock detected to inform
* that there is a bus lock in guest.
* Exit to user space when bus lock detected to inform that there is
*/ if (to_vmx(vcpu)->exit_reason.bus_lock_detected) { if (ret > 0)* a bus lock in guest.
NACK
No functional change; this time the bot is not doing a good job. :)
Paolo
From: Peter Gonda pgonda@google.com
[ Upstream commit baa1e5ca172ce7bf9554070139482dd7ea919528 ]
The refactoring in commit bb18a6777465 ("KVM: SEV: Acquire vcpu mutex when updating VMSA") left behind the assignment to svm->vcpu.arch.guest_state_protected; add it back.
Signed-off-by: Peter Gonda pgonda@google.com [Delta between v2 and v3 of Peter's patch, which had already been committed; the commit message is my own. - Paolo] Fixes: bb18a6777465 ("KVM: SEV: Acquire vcpu mutex when updating VMSA") Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/kvm/svm/sev.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index cb166bde449b..50dabccd664e 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -619,7 +619,12 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu, vmsa.handle = to_kvm_svm(kvm)->sev_info.handle; vmsa.address = __sme_pa(svm->vmsa); vmsa.len = PAGE_SIZE; - return sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_VMSA, &vmsa, error); + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_VMSA, &vmsa, error); + if (ret) + return ret; + + vcpu->arch.guest_state_protected = true; + return 0; }
static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
On 25/10/21 22:38, Sasha Levin wrote:
From: Peter Gonda pgonda@google.com
[ Upstream commit baa1e5ca172ce7bf9554070139482dd7ea919528 ]
The refactoring in commit bb18a6777465 ("KVM: SEV: Acquire vcpu mutex when updating VMSA") left behind the assignment to svm->vcpu.arch.guest_state_protected; add it back.
Signed-off-by: Peter Gonda pgonda@google.com [Delta between v2 and v3 of Peter's patch, which had already been committed; the commit message is my own. - Paolo] Fixes: bb18a6777465 ("KVM: SEV: Acquire vcpu mutex when updating VMSA") Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/svm/sev.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index cb166bde449b..50dabccd664e 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -619,7 +619,12 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu, vmsa.handle = to_kvm_svm(kvm)->sev_info.handle; vmsa.address = __sme_pa(svm->vmsa); vmsa.len = PAGE_SIZE;
- return sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_VMSA, &vmsa, error);
- ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_VMSA, &vmsa, error);
- if (ret)
return ret;
- vcpu->arch.guest_state_protected = true;
- return 0; }
static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
Acked-by: Paolo Bonzini pbonzini@redhat.com
I missed that bb18a677746543e7f5eeb478129c92cedb0f9658 was Cc'd to stable. Good bot. :)
Paolo
From: Chenyi Qiang chenyi.qiang@intel.com
[ Upstream commit a3ca5281bb771d8103ea16f0a6a8a5df9a7fb4f3 ]
When updating mmu->pkru_mask, the value can only be added but it isn't reset in advance. This will make mmu->pkru_mask keep the stale data. Fix this issue.
Fixes: 2d344105f57c ("KVM, pkeys: introduce pkru_mask to cache conditions") Signed-off-by: Chenyi Qiang chenyi.qiang@intel.com Message-Id: 20211021071022.1140-1-chenyi.qiang@intel.com Reviewed-by: Sean Christopherson seanjc@google.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/kvm/mmu/mmu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c268fb59f779..6719a8041f59 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4465,10 +4465,10 @@ static void update_pkru_bitmask(struct kvm_mmu *mmu) unsigned bit; bool wp;
- if (!is_cr4_pke(mmu)) { - mmu->pkru_mask = 0; + mmu->pkru_mask = 0; + + if (!is_cr4_pke(mmu)) return; - }
wp = is_cr0_wp(mmu);
On 25/10/21 22:38, Sasha Levin wrote:
From: Chenyi Qiang chenyi.qiang@intel.com
[ Upstream commit a3ca5281bb771d8103ea16f0a6a8a5df9a7fb4f3 ]
When updating mmu->pkru_mask, the value can only be added but it isn't reset in advance. This will make mmu->pkru_mask keep the stale data. Fix this issue.
Fixes: 2d344105f57c ("KVM, pkeys: introduce pkru_mask to cache conditions") Signed-off-by: Chenyi Qiang chenyi.qiang@intel.com Message-Id: 20211021071022.1140-1-chenyi.qiang@intel.com Reviewed-by: Sean Christopherson seanjc@google.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/mmu/mmu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c268fb59f779..6719a8041f59 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4465,10 +4465,10 @@ static void update_pkru_bitmask(struct kvm_mmu *mmu) unsigned bit; bool wp;
- if (!is_cr4_pke(mmu)) {
mmu->pkru_mask = 0;
- mmu->pkru_mask = 0;
- if (!is_cr4_pke(mmu)) return;
- }
wp = is_cr0_wp(mmu);
Acked-by: Paolo Bonzini pbonzini@redhat.com
On 25/10/21 22:38, Sasha Levin wrote:
From: Paolo Bonzini pbonzini@redhat.com
[ Upstream commit fa13843d1565d4c5b3aeb9be3343b313416bef46 ]
If allocation of rmaps fails, but some of the pointers have already been written, those pointers can be cleaned up when the memslot is freed, or even reused later for another attempt at allocating the rmaps. Therefore there is no need to WARN, as done for example in memslot_rmap_alloc, but the allocation *must* be skipped lest KVM will overwrite the previous pointer and will indeed leak memory.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/x86.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4b0e866e9f08..60d9aa0ab389 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11341,7 +11341,8 @@ static int memslot_rmap_alloc(struct kvm_memory_slot *slot, int lpages = gfn_to_index(slot->base_gfn + npages - 1, slot->base_gfn, level) + 1;
WARN_ON(slot->arch.rmap[i]);
if (slot->arch.rmap[i])
continue;
slot->arch.rmap[i] = kvcalloc(lpages, sz, GFP_KERNEL_ACCOUNT); if (!slot->arch.rmap[i]) {
NACK
There is no lazy allocation of rmaps in 5.14, and any failure to allocate goes straight to memslot_rmap_free followed by return -ENOMEM. So the WARN_ON is justified there.
Paolo
On Tue, Oct 26, 2021 at 06:14:34PM +0200, Paolo Bonzini wrote:
On 25/10/21 22:38, Sasha Levin wrote:
From: Paolo Bonzini pbonzini@redhat.com
[ Upstream commit fa13843d1565d4c5b3aeb9be3343b313416bef46 ]
If allocation of rmaps fails, but some of the pointers have already been written, those pointers can be cleaned up when the memslot is freed, or even reused later for another attempt at allocating the rmaps. Therefore there is no need to WARN, as done for example in memslot_rmap_alloc, but the allocation *must* be skipped lest KVM will overwrite the previous pointer and will indeed leak memory.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org
arch/x86/kvm/x86.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4b0e866e9f08..60d9aa0ab389 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11341,7 +11341,8 @@ static int memslot_rmap_alloc(struct kvm_memory_slot *slot, int lpages = gfn_to_index(slot->base_gfn + npages - 1, slot->base_gfn, level) + 1;
WARN_ON(slot->arch.rmap[i]);
if (slot->arch.rmap[i])
slot->arch.rmap[i] = kvcalloc(lpages, sz, GFP_KERNEL_ACCOUNT); if (!slot->arch.rmap[i]) {continue;
NACK
There is no lazy allocation of rmaps in 5.14, and any failure to allocate goes straight to memslot_rmap_free followed by return -ENOMEM. So the WARN_ON is justified there.
I'll queue up the ones you've acked, thanks :)
linux-stable-mirror@lists.linaro.org