2022-02-09 13:20:58

by Hou Wenlong

Subject: [PATCH v2 3/3] KVM: x86/emulator: Move the unhandled outer privilege level logic of far return into __load_segment_descriptor()

Outer-privilege level return is not implemented in emulator,
move the unhandled logic into __load_segment_descriptor to
make it easier to understand why the checks for RET are
incomplete.

Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Hou Wenlong <[email protected]>
---
arch/x86/kvm/emulate.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 37c4213bdcc1..bd91b952eb0a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1631,9 +1631,14 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
if (!(seg_desc.type & 8))
goto exception;

- /* RET can never return to an inner privilege level. */
- if (transfer == X86_TRANSFER_RET && rpl < cpl)
- goto exception;
+ if (transfer == X86_TRANSFER_RET) {
+ /* RET can never return to an inner privilege level. */
+ if (rpl < cpl)
+ goto exception;
+ /* Outer-privilege level return is not implemented */
+ if (rpl > cpl)
+ return X86EMUL_UNHANDLEABLE;
+ }
if (transfer == X86_TRANSFER_RET || transfer == X86_TRANSFER_TASK_SWITCH) {
if (seg_desc.type & 4) {
/* conforming */
@@ -2228,9 +2233,6 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt)
rc = emulate_pop(ctxt, &cs, ctxt->op_bytes);
if (rc != X86EMUL_CONTINUE)
return rc;
- /* Outer-privilege level return is not implemented */
- if (ctxt->mode >= X86EMUL_MODE_PROT16 && (cs & 3) > cpl)
- return X86EMUL_UNHANDLEABLE;
rc = __load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS, cpl,
X86_TRANSFER_RET,
&new_desc);
--
2.31.1

2022-02-25 19:32:54

by Paolo Bonzini

Subject: Re: [PATCH v2 3/3] KVM: x86/emulator: Move the unhandled outer privilege level logic of far return into __load_segment_descriptor()

On 2/8/22 10:34, Hou Wenlong wrote:
> Outer-privilege level return is not implemented in emulator,
> move the unhandled logic into __load_segment_descriptor to
> make it easier to understand why the checks for RET are
> incomplete.
>
> Suggested-by: Sean Christopherson <[email protected]>
> Signed-off-by: Hou Wenlong <[email protected]>
> ---
> arch/x86/kvm/emulate.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 37c4213bdcc1..bd91b952eb0a 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1631,9 +1631,14 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
> if (!(seg_desc.type & 8))
> goto exception;
>
> - /* RET can never return to an inner privilege level. */
> - if (transfer == X86_TRANSFER_RET && rpl < cpl)
> - goto exception;
> + if (transfer == X86_TRANSFER_RET) {
> + /* RET can never return to an inner privilege level. */
> + if (rpl < cpl)
> + goto exception;
> + /* Outer-privilege level return is not implemented */
> + if (rpl > cpl)
> + return X86EMUL_UNHANDLEABLE;
> + }
> if (transfer == X86_TRANSFER_RET || transfer == X86_TRANSFER_TASK_SWITCH) {
> if (seg_desc.type & 4) {
> /* conforming */
> @@ -2228,9 +2233,6 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt)
> rc = emulate_pop(ctxt, &cs, ctxt->op_bytes);
> if (rc != X86EMUL_CONTINUE)
> return rc;
> - /* Outer-privilege level return is not implemented */
> - if (ctxt->mode >= X86EMUL_MODE_PROT16 && (cs & 3) > cpl)
> - return X86EMUL_UNHANDLEABLE;
> rc = __load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS, cpl,
> X86_TRANSFER_RET,
> &new_desc);

Queued all three, thanks!

Paolo