* [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu()
@ 2023-06-05 11:44 Michal Luczaj
2023-06-05 13:03 ` Yuan Yao
2023-06-07 0:53 ` Sean Christopherson
0 siblings, 2 replies; 5+ messages in thread
From: Michal Luczaj @ 2023-06-05 11:44 UTC (permalink / raw)
To: seanjc; +Cc: pbonzini, kvm, Michal Luczaj
Since c9d601548603 ("KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond")
'cond' is internally converted to boolean, so caller's explicit conversion
from void* is unnecessary.
Remove the double bang.
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
virt/kvm/kvm_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6a658f30af91..64dd940c549e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3975,7 +3975,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
if (r < 0)
goto kvm_put_xa_release;
- if (KVM_BUG_ON(!!xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
+ if (KVM_BUG_ON(xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
r = -EINVAL;
goto kvm_put_xa_release;
}
base-commit: 31b4fc3bc64aadd660c5bfa5178c86a7ba61e0f7
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu()
2023-06-05 11:44 [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu() Michal Luczaj
@ 2023-06-05 13:03 ` Yuan Yao
2023-06-05 14:54 ` Michal Luczaj
2023-06-07 0:53 ` Sean Christopherson
1 sibling, 1 reply; 5+ messages in thread
From: Yuan Yao @ 2023-06-05 13:03 UTC (permalink / raw)
To: Michal Luczaj; +Cc: seanjc, pbonzini, kvm
On Mon, Jun 05, 2023 at 01:44:19PM +0200, Michal Luczaj wrote:
> Since c9d601548603 ("KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond")
> 'cond' is internally converted to boolean, so caller's explicit conversion
> from void* is unnecessary.
>
> Remove the double bang.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
> virt/kvm/kvm_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6a658f30af91..64dd940c549e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3975,7 +3975,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
> if (r < 0)
> goto kvm_put_xa_release;
>
> - if (KVM_BUG_ON(!!xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
> + if (KVM_BUG_ON(xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
Looks the only one place for KVM_BUG_ON().
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
BTW: svm_get_lbr_msr() is using KVM_BUG(false, ...) and
handle_cr() is using KVM_BUG(1, ...), a chance to
change them to same style ?
> r = -EINVAL;
> goto kvm_put_xa_release;
> }
>
> base-commit: 31b4fc3bc64aadd660c5bfa5178c86a7ba61e0f7
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu()
2023-06-05 13:03 ` Yuan Yao
@ 2023-06-05 14:54 ` Michal Luczaj
2023-06-05 16:16 ` Sean Christopherson
0 siblings, 1 reply; 5+ messages in thread
From: Michal Luczaj @ 2023-06-05 14:54 UTC (permalink / raw)
To: Yuan Yao; +Cc: seanjc, pbonzini, kvm
On 6/5/23 15:03, Yuan Yao wrote:
> On Mon, Jun 05, 2023 at 01:44:19PM +0200, Michal Luczaj wrote:
>> Since c9d601548603 ("KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond")
>> 'cond' is internally converted to boolean, so caller's explicit conversion
>> from void* is unnecessary.
>>
>> Remove the double bang.
>> ...
>> - if (KVM_BUG_ON(!!xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
>> + if (KVM_BUG_ON(xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
>
> Looks the only one place for KVM_BUG_ON().
>
> Reviewed-by: Yuan Yao <yuan.yao@intel.com>
>
> BTW: svm_get_lbr_msr() is using KVM_BUG(false, ...) and
> handle_cr() is using KVM_BUG(1, ...), a chance to
> change them to same style ?
Sure, but KVM_BUG(false, ...) is a no-op, right? Would you like me to fix it
separately with KVM_BUG(1, ...) as a (hardly significant) functional change?
Also, am I correct to assume that (1, ) is the preferred style?
arch/powerpc/kvm/book3s_64_mmu_host.c:kvmppc_mmu_map_page() seems to be the only
exception (within KVM) with a `WARN_ON(true)`.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu()
2023-06-05 14:54 ` Michal Luczaj
@ 2023-06-05 16:16 ` Sean Christopherson
0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2023-06-05 16:16 UTC (permalink / raw)
To: Michal Luczaj; +Cc: Yuan Yao, pbonzini, kvm
On Mon, Jun 05, 2023, Michal Luczaj wrote:
> On 6/5/23 15:03, Yuan Yao wrote:
> > On Mon, Jun 05, 2023 at 01:44:19PM +0200, Michal Luczaj wrote:
> >> Since c9d601548603 ("KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond")
> >> 'cond' is internally converted to boolean, so caller's explicit conversion
> >> from void* is unnecessary.
> >>
> >> Remove the double bang.
> >> ...
> >> - if (KVM_BUG_ON(!!xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
> >> + if (KVM_BUG_ON(xa_store(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, 0), kvm)) {
> >
> > Looks the only one place for KVM_BUG_ON().
> >
> > Reviewed-by: Yuan Yao <yuan.yao@intel.com>
> >
> > BTW: svm_get_lbr_msr() is using KVM_BUG(false, ...) and
> > handle_cr() is using KVM_BUG(1, ...), a chance to
> > change them to same style ?
>
> Sure, but KVM_BUG(false, ...) is a no-op, right? Would you like me to fix it
> separately with KVM_BUG(1, ...) as a (hardly significant) functional change?
Heh, yeah, that's dead code and should be fixed separately. I think that should
just be a WARN_ON_ONCE(1) though, there's no reason to bug and kill the VM.
Actually, there's really no reason for double switch, KVM can simply provide a
helper to get the correct VMCB. That'd provide an excuse to clean up a few other
uglies in svm_update_lbrv() too. Tentative patch at the bottom.
> Also, am I correct to assume that (1, ) is the preferred style?
I don't think there's a preferred style, though (1, ...) is more prevalent, and
has the advantage of saving three chars for the message :-)
> arch/powerpc/kvm/book3s_64_mmu_host.c:kvmppc_mmu_map_page() seems to be the only
> exception (within KVM) with a `WARN_ON(true)`.
Yeah, I wouldn't worry about that one.
---
arch/x86/kvm/svm/svm.c | 56 ++++++++++++++----------------------------
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ff48cdea1fbf..406b318f2f0d 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -960,50 +960,24 @@ static void svm_disable_lbrv(struct kvm_vcpu *vcpu)
svm_copy_lbrs(svm->vmcb01.ptr, svm->vmcb);
}
-static int svm_get_lbr_msr(struct vcpu_svm *svm, u32 index)
+static struct vmcb *svm_get_lbr_vmcb(struct vcpu_svm *svm)
{
/*
- * If the LBR virtualization is disabled, the LBR msrs are always
- * kept in the vmcb01 to avoid copying them on nested guest entries.
- *
- * If nested, and the LBR virtualization is enabled/disabled, the msrs
- * are moved between the vmcb01 and vmcb02 as needed.
+ * If LBR virtualization is disabled, the LBR MSRs are always kept in
+ * vmcb01. If LBR virtualization is enabled and L1 is running VMs of
+ * its own, the MSRs are moved between vmcb01 and vmcb02 as needed.
*/
- struct vmcb *vmcb =
- (svm->vmcb->control.virt_ext & LBR_CTL_ENABLE_MASK) ?
- svm->vmcb : svm->vmcb01.ptr;
-
- switch (index) {
- case MSR_IA32_DEBUGCTLMSR:
- return vmcb->save.dbgctl;
- case MSR_IA32_LASTBRANCHFROMIP:
- return vmcb->save.br_from;
- case MSR_IA32_LASTBRANCHTOIP:
- return vmcb->save.br_to;
- case MSR_IA32_LASTINTFROMIP:
- return vmcb->save.last_excp_from;
- case MSR_IA32_LASTINTTOIP:
- return vmcb->save.last_excp_to;
- default:
- KVM_BUG(false, svm->vcpu.kvm,
- "%s: Unknown MSR 0x%x", __func__, index);
- return 0;
- }
+ return svm->vmcb->control.virt_ext & LBR_CTL_ENABLE_MASK ? svm->vmcb :
+ svm->vmcb01.ptr;
}
void svm_update_lbrv(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
-
- bool enable_lbrv = svm_get_lbr_msr(svm, MSR_IA32_DEBUGCTLMSR) &
- DEBUGCTLMSR_LBR;
-
- bool current_enable_lbrv = !!(svm->vmcb->control.virt_ext &
- LBR_CTL_ENABLE_MASK);
-
- if (unlikely(is_guest_mode(vcpu) && svm->lbrv_enabled))
- if (unlikely(svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))
- enable_lbrv = true;
+ bool current_enable_lbrv = svm->vmcb->control.virt_ext & LBR_CTL_ENABLE_MASK;
+ bool enable_lbrv = (svm_get_lbr_vmcb(svm)->save.dbgctl & DEBUGCTLMSR_LBR) ||
+ (is_guest_mode(vcpu) && svm->lbrv_enabled &&
+ (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK));
if (enable_lbrv == current_enable_lbrv)
return;
@@ -2808,11 +2782,19 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
msr_info->data = svm->tsc_aux;
break;
case MSR_IA32_DEBUGCTLMSR:
+ msr_info->data = svm_get_lbr_vmcb(svm)->save.dbgctl;
+ break;
case MSR_IA32_LASTBRANCHFROMIP:
+ msr_info->data = svm_get_lbr_vmcb(svm)->save.br_from;
+ break;
case MSR_IA32_LASTBRANCHTOIP:
+ msr_info->data = svm_get_lbr_vmcb(svm)->save.br_to;
+ break;
case MSR_IA32_LASTINTFROMIP:
+ msr_info->data = svm_get_lbr_vmcb(svm)->save.last_excp_from;
+ break;
case MSR_IA32_LASTINTTOIP:
- msr_info->data = svm_get_lbr_msr(svm, msr_info->index);
+ msr_info->data = svm_get_lbr_vmcb(svm)->save.last_excp_to;
break;
case MSR_VM_HSAVE_PA:
msr_info->data = svm->nested.hsave_msr;
base-commit: 76a17bf03a268bc342e08c05d8ddbe607d294eb4
--
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu()
2023-06-05 11:44 [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu() Michal Luczaj
2023-06-05 13:03 ` Yuan Yao
@ 2023-06-07 0:53 ` Sean Christopherson
1 sibling, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2023-06-07 0:53 UTC (permalink / raw)
To: Sean Christopherson, Michal Luczaj; +Cc: pbonzini, kvm
On Mon, 05 Jun 2023 13:44:19 +0200, Michal Luczaj wrote:
> Since c9d601548603 ("KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond")
> 'cond' is internally converted to boolean, so caller's explicit conversion
> from void* is unnecessary.
>
> Remove the double bang.
>
>
> [...]
Applied to kvm-x86 generic, thanks!
[1/1] KVM: Clean up kvm_vm_ioctl_create_vcpu()
https://github.com/kvm-x86/linux/commit/5f643e460ab1
--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-07 0:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-05 11:44 [PATCH] KVM: Clean up kvm_vm_ioctl_create_vcpu() Michal Luczaj
2023-06-05 13:03 ` Yuan Yao
2023-06-05 14:54 ` Michal Luczaj
2023-06-05 16:16 ` Sean Christopherson
2023-06-07 0:53 ` Sean Christopherson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.