* Re: [PATCH] KVM: Fix some obsolete comments
@ 2020-02-27 1:35 linmiaohe
0 siblings, 0 replies; 5+ messages in thread
From: linmiaohe @ 2020-02-27 1:35 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, kvm, linux-kernel, x86, pbonzini, rkrcmar,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>On Wed, Feb 26, 2020 at 01:48:28AM +0000, linmiaohe wrote:
>> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>> >linmiaohe <linmiaohe@huawei.com> writes:
>> >> throws
>> >> - * #UD or #GP.
>> >> + * #UD, #GP or #SS.
>> >
>> >Oxford comma, anyone? :-)))
>>
>> I have no strong preference. ^_^
>
>I'm also a fan of the Oxford comma when it comes to describing code.
So there are two votes for Oxford comma.:) Will do. Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Fix some obsolete comments
2020-02-26 1:48 linmiaohe
@ 2020-02-26 15:30 ` Sean Christopherson
0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2020-02-26 15:30 UTC (permalink / raw)
To: linmiaohe
Cc: Vitaly Kuznetsov, kvm, linux-kernel, x86, pbonzini, rkrcmar,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
On Wed, Feb 26, 2020 at 01:48:28AM +0000, linmiaohe wrote:
> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> >linmiaohe <linmiaohe@huawei.com> writes:
> >> throws
> >> - * #UD or #GP.
> >> + * #UD, #GP or #SS.
> >
> >Oxford comma, anyone? :-)))
>
> I have no strong preference. ^_^
I'm also a fan of the Oxford comma when it comes to describing code.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Fix some obsolete comments
@ 2020-02-26 1:48 linmiaohe
2020-02-26 15:30 ` Sean Christopherson
0 siblings, 1 reply; 5+ messages in thread
From: linmiaohe @ 2020-02-26 1:48 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>linmiaohe <linmiaohe@huawei.com> writes:
>
>> From: Miaohe Lin <linmiaohe@huawei.com>
>>
>
>Thank you for the cleanup, I looked at nested_svm_intercept() and I see room for improvement, e.g. (completely untested!)
>
>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 76c24b3491f6..fcb26d64d3c7 100644
>--- a/arch/x86/kvm/svm.c
>+++ b/arch/x86/kvm/svm.c
>@@ -3280,42 +3280,36 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
> case SVM_EXIT_IOIO:
> vmexit = nested_svm_intercept_ioio(svm);
> break;
>- case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8: {
>- u32 bit = 1U << (exit_code - SVM_EXIT_READ_CR0);
>- if (svm->nested.intercept_cr & bit)
>+ case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8:
>+ if (svm->nested.intercept_cr &
>+ BIT(exit_code - SVM_EXIT_READ_CR0))
>- default: {
>- u64 exit_bits = 1ULL << (exit_code - SVM_EXIT_INTR);
>- if (svm->nested.intercept & exit_bits)
>+ default:
>+ if (svm->nested.intercept & BIT_ULL(exit_code -
>+ SVM_EXIT_INTR))
> vmexit = NESTED_EXIT_DONE;
> }
>- }
>
> return vmexit;
> }
>
>Feel free to pick stuff you like and split your changes to this function in a separate patch.
Sounds good, many thanks for your improvement suggestion. Will do in a separate patch.
>> u32 exit_code = svm->vmcb->control.exit_code; diff --git
>> throws
>> - * #UD or #GP.
>> + * #UD, #GP or #SS.
>
>Oxford comma, anyone? :-)))
I have no strong preference. ^_^
>> */
>> int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
>
>All your changes look correct, so
>
>Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
Many thanks for your review, and nice suggestion again!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: Fix some obsolete comments
2020-02-25 3:05 linmiaohe
@ 2020-02-25 13:04 ` Vitaly Kuznetsov
0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-02-25 13:04 UTC (permalink / raw)
To: linmiaohe
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
linmiaohe <linmiaohe@huawei.com> writes:
> From: Miaohe Lin <linmiaohe@huawei.com>
>
> Remove some obsolete comments, fix wrong function name and description.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> arch/x86/kvm/svm.c | 3 ---
> arch/x86/kvm/vmx/nested.c | 4 ++--
> arch/x86/kvm/vmx/vmx.c | 2 +-
> 3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index fd3fc9fbefff..ee114a9913eb 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3228,9 +3228,6 @@ static int nested_svm_exit_special(struct vcpu_svm *svm)
> return NESTED_EXIT_CONTINUE;
> }
>
> -/*
> - * If this function returns true, this #vmexit was already handled
> - */
> static int nested_svm_intercept(struct vcpu_svm *svm)
> {
Thank you for the cleanup, I looked at nested_svm_intercept() and I see
room for improvement, e.g. (completely untested!)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 76c24b3491f6..fcb26d64d3c7 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3280,42 +3280,36 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
case SVM_EXIT_IOIO:
vmexit = nested_svm_intercept_ioio(svm);
break;
- case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8: {
- u32 bit = 1U << (exit_code - SVM_EXIT_READ_CR0);
- if (svm->nested.intercept_cr & bit)
+ case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8:
+ if (svm->nested.intercept_cr &
+ BIT(exit_code - SVM_EXIT_READ_CR0))
vmexit = NESTED_EXIT_DONE;
break;
- }
- case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
- u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
- if (svm->nested.intercept_dr & bit)
+ case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7:
+ if (svm->nested.intercept_dr &
+ BIT(exit_code - SVM_EXIT_READ_DR0))
vmexit = NESTED_EXIT_DONE;
break;
- }
- case SVM_EXIT_EXCP_BASE ... SVM_EXIT_EXCP_BASE + 0x1f: {
- u32 excp_bits = 1 << (exit_code - SVM_EXIT_EXCP_BASE);
- if (svm->nested.intercept_exceptions & excp_bits) {
+ case SVM_EXIT_EXCP_BASE ... SVM_EXIT_EXCP_BASE + 0x1f:
+ if (svm->nested.intercept_exceptions &
+ BIT(exit_code - SVM_EXIT_EXCP_BASE)) {
if (exit_code == SVM_EXIT_EXCP_BASE + DB_VECTOR)
vmexit = nested_svm_intercept_db(svm);
else
vmexit = NESTED_EXIT_DONE;
- }
- /* async page fault always cause vmexit */
- else if ((exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR) &&
- svm->vcpu.arch.exception.nested_apf != 0)
+ } else if ((exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR) &&
+ svm->vcpu.arch.exception.nested_apf != 0) {
+ /* async page fault always cause vmexit */
vmexit = NESTED_EXIT_DONE;
+ }
break;
- }
- case SVM_EXIT_ERR: {
+ case SVM_EXIT_ERR:
vmexit = NESTED_EXIT_DONE;
break;
- }
- default: {
- u64 exit_bits = 1ULL << (exit_code - SVM_EXIT_INTR);
- if (svm->nested.intercept & exit_bits)
+ default:
+ if (svm->nested.intercept & BIT_ULL(exit_code - SVM_EXIT_INTR))
vmexit = NESTED_EXIT_DONE;
}
- }
return vmexit;
}
Feel free to pick stuff you like and split your changes to this function
in a separate patch.
> u32 exit_code = svm->vmcb->control.exit_code;
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 0946122a8d3b..46c5f63136a8 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2960,7 +2960,7 @@ static int nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu)
> /*
> * Induce a consistency check VMExit by clearing bit 1 in GUEST_RFLAGS,
> * which is reserved to '1' by hardware. GUEST_RFLAGS is guaranteed to
> - * be written (by preparve_vmcs02()) before the "real" VMEnter, i.e.
> + * be written (by prepare_vmcs02()) before the "real" VMEnter, i.e.
> * there is no need to preserve other bits or save/restore the field.
> */
> vmcs_writel(GUEST_RFLAGS, 0);
> @@ -4382,7 +4382,7 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
> * Decode the memory-address operand of a vmx instruction, as recorded on an
> * exit caused by such an instruction (run by a guest hypervisor).
> * On success, returns 0. When the operand is invalid, returns 1 and throws
> - * #UD or #GP.
> + * #UD, #GP or #SS.
Oxford comma, anyone? :-)))
> */
> int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
> u32 vmx_instruction_info, bool wr, int len, gva_t *ret)
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 69948aa1b127..8d91fa9acbb2 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -808,7 +808,7 @@ void update_exception_bitmap(struct kvm_vcpu *vcpu)
> if (to_vmx(vcpu)->rmode.vm86_active)
> eb = ~0;
> if (enable_ept)
> - eb &= ~(1u << PF_VECTOR); /* bypass_guest_pf = 0 */
> + eb &= ~(1u << PF_VECTOR);
>
> /* When we are running a nested L2 guest and L1 specified for it a
> * certain exception bitmap, we must trap the same exceptions and pass
All your changes look correct, so
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] KVM: Fix some obsolete comments
@ 2020-02-25 3:05 linmiaohe
2020-02-25 13:04 ` Vitaly Kuznetsov
0 siblings, 1 reply; 5+ messages in thread
From: linmiaohe @ 2020-02-25 3:05 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
Remove some obsolete comments, fix wrong function name and description.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/svm.c | 3 ---
arch/x86/kvm/vmx/nested.c | 4 ++--
arch/x86/kvm/vmx/vmx.c | 2 +-
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index fd3fc9fbefff..ee114a9913eb 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3228,9 +3228,6 @@ static int nested_svm_exit_special(struct vcpu_svm *svm)
return NESTED_EXIT_CONTINUE;
}
-/*
- * If this function returns true, this #vmexit was already handled
- */
static int nested_svm_intercept(struct vcpu_svm *svm)
{
u32 exit_code = svm->vmcb->control.exit_code;
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 0946122a8d3b..46c5f63136a8 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2960,7 +2960,7 @@ static int nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu)
/*
* Induce a consistency check VMExit by clearing bit 1 in GUEST_RFLAGS,
* which is reserved to '1' by hardware. GUEST_RFLAGS is guaranteed to
- * be written (by preparve_vmcs02()) before the "real" VMEnter, i.e.
+ * be written (by prepare_vmcs02()) before the "real" VMEnter, i.e.
* there is no need to preserve other bits or save/restore the field.
*/
vmcs_writel(GUEST_RFLAGS, 0);
@@ -4382,7 +4382,7 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
* Decode the memory-address operand of a vmx instruction, as recorded on an
* exit caused by such an instruction (run by a guest hypervisor).
* On success, returns 0. When the operand is invalid, returns 1 and throws
- * #UD or #GP.
+ * #UD, #GP or #SS.
*/
int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
u32 vmx_instruction_info, bool wr, int len, gva_t *ret)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 69948aa1b127..8d91fa9acbb2 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -808,7 +808,7 @@ void update_exception_bitmap(struct kvm_vcpu *vcpu)
if (to_vmx(vcpu)->rmode.vm86_active)
eb = ~0;
if (enable_ept)
- eb &= ~(1u << PF_VECTOR); /* bypass_guest_pf = 0 */
+ eb &= ~(1u << PF_VECTOR);
/* When we are running a nested L2 guest and L1 specified for it a
* certain exception bitmap, we must trap the same exceptions and pass
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-27 1:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 1:35 [PATCH] KVM: Fix some obsolete comments linmiaohe
-- strict thread matches above, loose matches on Subject: below --
2020-02-26 1:48 linmiaohe
2020-02-26 15:30 ` Sean Christopherson
2020-02-25 3:05 linmiaohe
2020-02-25 13:04 ` Vitaly Kuznetsov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).