[v1] KVM: x86: Swap the dst and src operand for MOVNTDQA

[PATCH] KVM: x86: Swap the dst and src operand for MOVNTDQA

Posted by Sean Christopherson 3 weeks, 2 days ago

Swap the MOVNTDQA operands, as MOVNTDQA does NOT in fact have "the same
characteristics as 0F E7 (MOVNTDQ)"; MOVNTDQA loads from memory and stores
to registers, while MOVNTDQ loads from registers and stores to memory.

Per the SDM:

 MOVNTDQ - Move packed integer values in xmm1 to m128 using non-temporal
           hint.

 MOVNTDQA - Move double quadword from m128 to xmm1 using non-temporal hint
            if WC memory type.

Reported-by: Josh Eads <josheads@google.com>
Fixes: c57d9bafbd0b ("KVM: x86: Add support for emulating MOVNTDQA")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

I have updates to the xsave KUT test to verify the bug and the fix.

 arch/x86/kvm/emulate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c8c6cc0406d6..0bb832a76868 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4450,7 +4450,7 @@ static const struct opcode twobyte_table[256] = {
 	/* 0xD0 - 0xDF */
 	N, N, N, N, N, N, N, N, N, N, N, N, N, N, N, N,
 	/* 0xE0 - 0xEF */
-	N, N, N, N, N, N, N, GP(SrcReg | DstMem | ModRM | Mov, &pfx_0f_e7_0f_38_2a),
+	N, N, N, N, N, N, N, GP(SrcMem | DstReg | ModRM | Mov, &pfx_0f_e7_0f_38_2a),
 	N, N, N, N, N, N, N, N,
 	/* 0xF0 - 0xFF */
 	N, N, N, N, N, N, N, N, N, N, N, N, N, N, N, N
@@ -4481,7 +4481,7 @@ static const struct opcode opcode_map_0f_38[256] = {
 	X16(N), X16(N),
 	/* 0x20 - 0x2f */
 	X8(N),
-	X2(N), GP(SrcReg | DstMem | ModRM | Mov | Aligned, &pfx_0f_e7_0f_38_2a), N, N, N, N, N,
+	X2(N), GP(SrcMem | DstReg | ModRM | Mov | Aligned, &pfx_0f_e7_0f_38_2a), N, N, N, N, N,
 	/* 0x30 - 0x7f */
 	X16(N), X16(N), X16(N), X16(N), X16(N),
 	/* 0x80 - 0xef */

base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH] KVM: x86: Swap the dst and src operand for MOVNTDQA

Posted by Sean Christopherson 3 weeks, 2 days ago

On Wed, May 06, 2026, Sean Christopherson wrote:
> Swap the MOVNTDQA operands, as MOVNTDQA does NOT in fact have "the same
> characteristics as 0F E7 (MOVNTDQ)"; MOVNTDQA loads from memory and stores
> to registers, while MOVNTDQ loads from registers and stores to memory.
> 
> Per the SDM:
> 
>  MOVNTDQ - Move packed integer values in xmm1 to m128 using non-temporal
>            hint.
> 
>  MOVNTDQA - Move double quadword from m128 to xmm1 using non-temporal hint
>             if WC memory type.
> 
> Reported-by: Josh Eads <josheads@google.com>
> Fixes: c57d9bafbd0b ("KVM: x86: Add support for emulating MOVNTDQA")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 
> I have updates to the xsave KUT test to verify the bug and the fix.
> 
>  arch/x86/kvm/emulate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c8c6cc0406d6..0bb832a76868 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4450,7 +4450,7 @@ static const struct opcode twobyte_table[256] = {
>  	/* 0xD0 - 0xDF */
>  	N, N, N, N, N, N, N, N, N, N, N, N, N, N, N, N,
>  	/* 0xE0 - 0xEF */
> -	N, N, N, N, N, N, N, GP(SrcReg | DstMem | ModRM | Mov, &pfx_0f_e7_0f_38_2a),
> +	N, N, N, N, N, N, N, GP(SrcMem | DstReg | ModRM | Mov, &pfx_0f_e7_0f_38_2a),

Aaaaand Josh pointed out that I wasn't paying attention and stomped all over MOVNTDQ.
I'll figure out why my KUT changes didn't detect this, and send a v2.

>  	N, N, N, N, N, N, N, N,
>  	/* 0xF0 - 0xFF */
>  	N, N, N, N, N, N, N, N, N, N, N, N, N, N, N, N
> @@ -4481,7 +4481,7 @@ static const struct opcode opcode_map_0f_38[256] = {
>  	X16(N), X16(N),
>  	/* 0x20 - 0x2f */
>  	X8(N),
> -	X2(N), GP(SrcReg | DstMem | ModRM | Mov | Aligned, &pfx_0f_e7_0f_38_2a), N, N, N, N, N,
> +	X2(N), GP(SrcMem | DstReg | ModRM | Mov | Aligned, &pfx_0f_e7_0f_38_2a), N, N, N, N, N,
>  	/* 0x30 - 0x7f */
>  	X16(N), X16(N), X16(N), X16(N), X16(N),
>  	/* 0x80 - 0xef */
> 
> base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a
> -- 
> 2.54.0.545.g6539524ca2-goog
>

Re: [PATCH] KVM: x86: Swap the dst and src operand for MOVNTDQA

Posted by Sean Christopherson 3 weeks, 2 days ago

On Wed, May 06, 2026, Sean Christopherson wrote:
> On Wed, May 06, 2026, Sean Christopherson wrote:
> > Swap the MOVNTDQA operands, as MOVNTDQA does NOT in fact have "the same
> > characteristics as 0F E7 (MOVNTDQ)"; MOVNTDQA loads from memory and stores
> > to registers, while MOVNTDQ loads from registers and stores to memory.
> > 
> > Per the SDM:
> > 
> >  MOVNTDQ - Move packed integer values in xmm1 to m128 using non-temporal
> >            hint.
> > 
> >  MOVNTDQA - Move double quadword from m128 to xmm1 using non-temporal hint
> >             if WC memory type.
> > 
> > Reported-by: Josh Eads <josheads@google.com>
> > Fixes: c57d9bafbd0b ("KVM: x86: Add support for emulating MOVNTDQA")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > 
> > I have updates to the xsave KUT test to verify the bug and the fix.
> > 
> >  arch/x86/kvm/emulate.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > index c8c6cc0406d6..0bb832a76868 100644
> > --- a/arch/x86/kvm/emulate.c
> > +++ b/arch/x86/kvm/emulate.c
> > @@ -4450,7 +4450,7 @@ static const struct opcode twobyte_table[256] = {
> >  	/* 0xD0 - 0xDF */
> >  	N, N, N, N, N, N, N, N, N, N, N, N, N, N, N, N,
> >  	/* 0xE0 - 0xEF */
> > -	N, N, N, N, N, N, N, GP(SrcReg | DstMem | ModRM | Mov, &pfx_0f_e7_0f_38_2a),
> > +	N, N, N, N, N, N, N, GP(SrcMem | DstReg | ModRM | Mov, &pfx_0f_e7_0f_38_2a),
> 
> Aaaaand Josh pointed out that I wasn't paying attention and stomped all over MOVNTDQ.
> I'll figure out why my KUT changes didn't detect this

*sigh*

I made the same mistake when testing MOVNTDQA, and didn't learn from that mistake
for the MOVNTDQ half.  Comparing the src and dst buffers on a "src => reg => dst"
sequence wont't detect mismatches if KVM wrongly does "reg => src, reg => dst".