| Message ID | 20181022220911.9340-1-ubizjak@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM/nVMX: Remove uneeded forward jump in nested_vmx_check_vmentry_hw asm | expand |
On Mon, Oct 22, 2018 at 3:09 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > There is no need to jump just after the jump insn itself. Also, make > code similar to entering guest mode in vmx_vcpu_run. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> Reviewed-by: Jim Mattson <jmattson@google.com>
On 10/22/2018 03:09 PM, Uros Bizjak wrote: > There is no need to jump just after the jump insn itself. Also, make > code similar to entering guest mode in vmx_vcpu_run. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > --- > arch/x86/kvm/vmx.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 641a65b30685..5600a01fa6d7 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -13240,13 +13240,11 @@ static int __noclone nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu) > > /* Check if vmlaunch of vmresume is needed */ Not a big deal, but would you mind fixing the comment ? > "cmpl $0, %c[launched](%0)\n\t" > - "je 1f\n\t" > - __ex("vmresume") "\n\t" > - "jmp 2f\n\t" > - "1: " __ex("vmlaunch") "\n\t" > + "jne 1f\n\t" > + __ex("vmlaunch") "\n\t" > "jmp 2f\n\t" > + "1: " __ex("vmresume") "\n\t" > "2: " > - > /* Set vmx->fail accordingly */ > "setbe %c[fail](%0)\n\t" > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
On Tue, Oct 23, 2018 at 11:26 PM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > > > > On 10/22/2018 03:09 PM, Uros Bizjak wrote: > > There is no need to jump just after the jump insn itself. Also, make > > code similar to entering guest mode in vmx_vcpu_run. > > > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > --- > > arch/x86/kvm/vmx.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 641a65b30685..5600a01fa6d7 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -13240,13 +13240,11 @@ static int __noclone nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu) > > > > /* Check if vmlaunch of vmresume is needed */ > > Not a big deal, but would you mind fixing the comment ? I dont't like mixing whitespace/comment/whatever fixes with functional changes in the same patch. OTOH, I'm sure somebody is thrilling with the idea to fix this comment and the original one in vmx_vcpu_run. BR, Uros.
On 23/10/18 00:09, Uros Bizjak wrote: > There is no need to jump just after the jump insn itself. Also, make > code similar to entering guest mode in vmx_vcpu_run. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > --- > arch/x86/kvm/vmx.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 641a65b30685..5600a01fa6d7 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -13240,13 +13240,11 @@ static int __noclone nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu) > > /* Check if vmlaunch of vmresume is needed */ > "cmpl $0, %c[launched](%0)\n\t" > - "je 1f\n\t" > - __ex("vmresume") "\n\t" > - "jmp 2f\n\t" > - "1: " __ex("vmlaunch") "\n\t" > + "jne 1f\n\t" > + __ex("vmlaunch") "\n\t" > "jmp 2f\n\t" > + "1: " __ex("vmresume") "\n\t" > "2: " > - > /* Set vmx->fail accordingly */ > "setbe %c[fail](%0)\n\t" > > Queued, thanks. Paolo
On 23/10/18 23:26, Krish Sadhukhan wrote: > > > On 10/22/2018 03:09 PM, Uros Bizjak wrote: >> There is no need to jump just after the jump insn itself. Also, make >> code similar to entering guest mode in vmx_vcpu_run. >> >> Signed-off-by: Uros Bizjak <ubizjak@gmail.com> >> --- >> arch/x86/kvm/vmx.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 641a65b30685..5600a01fa6d7 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -13240,13 +13240,11 @@ static int __noclone >> nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu) >> /* Check if vmlaunch of vmresume is needed */ > > Not a big deal, but would you mind fixing the comment ? Did that myself when applying. Thanks for noticing it! Paolo >> "cmpl $0, %c[launched](%0)\n\t" >> - "je 1f\n\t" >> - __ex("vmresume") "\n\t" >> - "jmp 2f\n\t" >> - "1: " __ex("vmlaunch") "\n\t" >> + "jne 1f\n\t" >> + __ex("vmlaunch") "\n\t" >> "jmp 2f\n\t" >> + "1: " __ex("vmresume") "\n\t" >> "2: " >> - >> /* Set vmx->fail accordingly */ >> "setbe %c[fail](%0)\n\t" >> > > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 641a65b30685..5600a01fa6d7 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -13240,13 +13240,11 @@ static int __noclone nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu) /* Check if vmlaunch of vmresume is needed */ "cmpl $0, %c[launched](%0)\n\t" - "je 1f\n\t" - __ex("vmresume") "\n\t" - "jmp 2f\n\t" - "1: " __ex("vmlaunch") "\n\t" + "jne 1f\n\t" + __ex("vmlaunch") "\n\t" "jmp 2f\n\t" + "1: " __ex("vmresume") "\n\t" "2: " - /* Set vmx->fail accordingly */ "setbe %c[fail](%0)\n\t"
There is no need to jump just after the jump insn itself. Also, make code similar to entering guest mode in vmx_vcpu_run. Signed-off-by: Uros Bizjak <ubizjak@gmail.com> --- arch/x86/kvm/vmx.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)