linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
@ 2020-05-15 16:19 Paolo Bonzini
  2020-05-18 16:07 ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-05-15 16:19 UTC (permalink / raw)
  To: linux-kernel, kvm

Instructions starting with 0f18 up to 0f1f are reserved nops, except those
that were assigned to MPX.  These include the endbr markers used by CET.
List them correctly in the opcode table.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/emulate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index de5476f8683e..d0e2825ae617 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4800,8 +4800,12 @@ static const struct opcode twobyte_table[256] = {
 	GP(ModRM | DstReg | SrcMem | Mov | Sse, &pfx_0f_10_0f_11),
 	GP(ModRM | DstMem | SrcReg | Mov | Sse, &pfx_0f_10_0f_11),
 	N, N, N, N, N, N,
-	D(ImplicitOps | ModRM | SrcMem | NoAccess),
-	N, N, N, N, N, N, D(ImplicitOps | ModRM | SrcMem | NoAccess),
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 4 * prefetch + 4 * reserved NOP */
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), N, N,
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
+	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* NOP + 7 * reserved NOP */
 	/* 0x20 - 0x2F */
 	DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_read),
 	DIP(ModRM | DstMem | Priv | Op3264 | NoMod, dr_read, check_dr_read),
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-15 16:19 [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f Paolo Bonzini
@ 2020-05-18 16:07 ` Sean Christopherson
  2020-05-18 17:37   ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2020-05-18 16:07 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Fri, May 15, 2020 at 12:19:19PM -0400, Paolo Bonzini wrote:
> Instructions starting with 0f18 up to 0f1f are reserved nops, except those
> that were assigned to MPX.

Well, they're probably reserved NOPs again :-D.

> These include the endbr markers used by CET.

And RDSPP.  Wouldn't it make sense to treat RDSPP as a #UD even though it's
a NOP if CET is disabled?  The logic being that a sane guest will execute
RDSSP iff CET is enabled, and in that case it'd be better to inject a #UD
than to silently break the guest.

Extending that logic to future features, wouldn't it then make sense to
keep the current #UD behavior for all opcodes to avoid silently breakage?
I.e. change only the opcodes for endbr (which consume only 2 of 255 ModRMs
for 0f 1e) to be NOPs.

> List them correctly in the opcode table.
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/emulate.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index de5476f8683e..d0e2825ae617 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4800,8 +4800,12 @@ static const struct opcode twobyte_table[256] = {
>  	GP(ModRM | DstReg | SrcMem | Mov | Sse, &pfx_0f_10_0f_11),
>  	GP(ModRM | DstMem | SrcReg | Mov | Sse, &pfx_0f_10_0f_11),
>  	N, N, N, N, N, N,
> -	D(ImplicitOps | ModRM | SrcMem | NoAccess),
> -	N, N, N, N, N, N, D(ImplicitOps | ModRM | SrcMem | NoAccess),
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 4 * prefetch + 4 * reserved NOP */
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), N, N,
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
> +	D(ImplicitOps | ModRM | SrcMem | NoAccess), /* NOP + 7 * reserved NOP */
>  	/* 0x20 - 0x2F */
>  	DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_read),
>  	DIP(ModRM | DstMem | Priv | Op3264 | NoMod, dr_read, check_dr_read),
> -- 
> 2.18.2
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-18 16:07 ` Sean Christopherson
@ 2020-05-18 17:37   ` Paolo Bonzini
  2020-05-19  6:02     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-05-18 17:37 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On 18/05/20 18:07, Sean Christopherson wrote:
> On Fri, May 15, 2020 at 12:19:19PM -0400, Paolo Bonzini wrote:
>> Instructions starting with 0f18 up to 0f1f are reserved nops, except those
>> that were assigned to MPX.
> Well, they're probably reserved NOPs again :-D.

So are you suggesting adding them back to the list as well?

>> These include the endbr markers used by CET.
> And RDSPP.  Wouldn't it make sense to treat RDSPP as a #UD even though it's
> a NOP if CET is disabled?  The logic being that a sane guest will execute
> RDSSP iff CET is enabled, and in that case it'd be better to inject a #UD
> than to silently break the guest.

We cannot assume that guests will bother checking CPUID before invoking
RDSPP.  This is especially true userspace, which needs to check if CET
is enable for itself and can only use RDSPP to do so.

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-18 17:37   ` Paolo Bonzini
@ 2020-05-19  6:02     ` Sean Christopherson
  2020-05-19  7:43       ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2020-05-19  6:02 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Mon, May 18, 2020 at 07:37:08PM +0200, Paolo Bonzini wrote:
> On 18/05/20 18:07, Sean Christopherson wrote:
> > On Fri, May 15, 2020 at 12:19:19PM -0400, Paolo Bonzini wrote:
> >> Instructions starting with 0f18 up to 0f1f are reserved nops, except those
> >> that were assigned to MPX.
> > Well, they're probably reserved NOPs again :-D.
> 
> So are you suggesting adding them back to the list as well?

Doesn't KVM still support MPX?

> >> These include the endbr markers used by CET.
> > And RDSPP.  Wouldn't it make sense to treat RDSPP as a #UD even though it's
> > a NOP if CET is disabled?  The logic being that a sane guest will execute
> > RDSSP iff CET is enabled, and in that case it'd be better to inject a #UD
> > than to silently break the guest.
> 
> We cannot assume that guests will bother checking CPUID before invoking
> RDSPP.  This is especially true userspace, which needs to check if CET
> is enable for itself and can only use RDSPP to do so.

Ugh, yeah, just read through the CET enabling thread that showed code snippets
that do exactly this.

I assume it would be best to make SHSTK dependent on unrestricted guest?
Emulating RDSPP by reading vmcs.GUEST_SSP seems pointless as it will become
statle apart on the first emulated CALL/RET.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-19  6:02     ` Sean Christopherson
@ 2020-05-19  7:43       ` Paolo Bonzini
  2020-05-19  7:55         ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-05-19  7:43 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On 19/05/20 08:02, Sean Christopherson wrote:
> On Mon, May 18, 2020 at 07:37:08PM +0200, Paolo Bonzini wrote:
>> On 18/05/20 18:07, Sean Christopherson wrote:
>>> On Fri, May 15, 2020 at 12:19:19PM -0400, Paolo Bonzini wrote:
>>>> Instructions starting with 0f18 up to 0f1f are reserved nops, except those
>>>> that were assigned to MPX.
>>> Well, they're probably reserved NOPs again :-D.
>>
>> So are you suggesting adding them back to the list as well?
> 
> Doesn't KVM still support MPX?
> 
>>>> These include the endbr markers used by CET.
>>> And RDSPP.  Wouldn't it make sense to treat RDSPP as a #UD even though it's
>>> a NOP if CET is disabled?  The logic being that a sane guest will execute
>>> RDSSP iff CET is enabled, and in that case it'd be better to inject a #UD
>>> than to silently break the guest.
>>
>> We cannot assume that guests will bother checking CPUID before invoking
>> RDSPP.  This is especially true userspace, which needs to check if CET
>> is enable for itself and can only use RDSPP to do so.
> 
> Ugh, yeah, just read through the CET enabling thread that showed code snippets
> that do exactly this.
> 
> I assume it would be best to make SHSTK dependent on unrestricted guest?
> Emulating RDSPP by reading vmcs.GUEST_SSP seems pointless as it will become
> statle apart on the first emulated CALL/RET.

Running arbitrary code under the emulator is problematic anyway with
CET, since you won't be checking ENDBR markers or updating the state
machine.  So perhaps in addition to what you say we should have a mode
where, unless unrestricted guest is disabled, the emulator only accepts
I/O, MOV and ALU instructions.

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-19  7:43       ` Paolo Bonzini
@ 2020-05-19  7:55         ` Sean Christopherson
  2020-05-19  8:06           ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2020-05-19  7:55 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Tue, May 19, 2020 at 09:43:23AM +0200, Paolo Bonzini wrote:
> On 19/05/20 08:02, Sean Christopherson wrote:
> > On Mon, May 18, 2020 at 07:37:08PM +0200, Paolo Bonzini wrote:
> >> On 18/05/20 18:07, Sean Christopherson wrote:
> >>> On Fri, May 15, 2020 at 12:19:19PM -0400, Paolo Bonzini wrote:
> >>>> Instructions starting with 0f18 up to 0f1f are reserved nops, except those
> >>>> that were assigned to MPX.
> >>> Well, they're probably reserved NOPs again :-D.
> >>
> >> So are you suggesting adding them back to the list as well?
> > 
> > Doesn't KVM still support MPX?
> > 
> >>>> These include the endbr markers used by CET.
> >>> And RDSPP.  Wouldn't it make sense to treat RDSPP as a #UD even though it's
> >>> a NOP if CET is disabled?  The logic being that a sane guest will execute
> >>> RDSSP iff CET is enabled, and in that case it'd be better to inject a #UD
> >>> than to silently break the guest.
> >>
> >> We cannot assume that guests will bother checking CPUID before invoking
> >> RDSPP.  This is especially true userspace, which needs to check if CET
> >> is enable for itself and can only use RDSPP to do so.
> > 
> > Ugh, yeah, just read through the CET enabling thread that showed code snippets
> > that do exactly this.
> > 
> > I assume it would be best to make SHSTK dependent on unrestricted guest?
> > Emulating RDSPP by reading vmcs.GUEST_SSP seems pointless as it will become
> > statle apart on the first emulated CALL/RET.
> 
> Running arbitrary code under the emulator is problematic anyway with
> CET, since you won't be checking ENDBR markers or updating the state
> machine.  So perhaps in addition to what you say we should have a mode
> where, unless unrestricted guest is disabled, the emulator only accepts
> I/O, MOV and ALU instructions.

Doh, I forgot all about those pesky ENDBR markers.  I think a slimmed down
emulator makes sense?

Tangentially related, isn't the whole fastop thing doomed once CET kernel
support lands?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-19  7:55         ` Sean Christopherson
@ 2020-05-19  8:06           ` Paolo Bonzini
  2020-05-19 15:32             ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-05-19  8:06 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On 19/05/20 09:55, Sean Christopherson wrote:
>> Running arbitrary code under the emulator is problematic anyway with
>> CET, since you won't be checking ENDBR markers or updating the state
>> machine.  So perhaps in addition to what you say we should have a mode
>> where, unless unrestricted guest is disabled, the emulator only accepts
>> I/O, MOV and ALU instructions.
>
> Doh, I forgot all about those pesky ENDBR markers.  I think a slimmed down
> emulator makes sense?

Or just slimmed down opcode tables.

> Tangentially related, isn't the whole fastop thing doomed once CET kernel
> support lands?

Why?  You do need to add endbr markers and some of the fastop handlers
won't fit in 8 bytes, but that should be it.

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f
  2020-05-19  8:06           ` Paolo Bonzini
@ 2020-05-19 15:32             ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2020-05-19 15:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Tue, May 19, 2020 at 10:06:25AM +0200, Paolo Bonzini wrote:
> On 19/05/20 09:55, Sean Christopherson wrote:
> > Tangentially related, isn't the whole fastop thing doomed once CET kernel
> > support lands?
> 
> Why?  You do need to add endbr markers and some of the fastop handlers
> won't fit in 8 bytes, but that should be it.

Never mind, had a brain fart and forgot CALL_NOSPEC shouldn't get patched
to a retpoline on CET capable CPUs.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-19 15:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 16:19 [PATCH] KVM: x86: emulate reserved nops from 0f/18 to 0f/1f Paolo Bonzini
2020-05-18 16:07 ` Sean Christopherson
2020-05-18 17:37   ` Paolo Bonzini
2020-05-19  6:02     ` Sean Christopherson
2020-05-19  7:43       ` Paolo Bonzini
2020-05-19  7:55         ` Sean Christopherson
2020-05-19  8:06           ` Paolo Bonzini
2020-05-19 15:32             ` Sean Christopherson

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).