Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 30 Mar 2023 10:55:51 +0800
From:   Wu Zongyong <wuzongyong@linux.alibaba.com>
To:     Tom Lendacky <thomas.lendacky@amd.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
        x86@kernel.org, linux-kernel@vger.kernel.org, tony.luck@intel.com,
        kirill.shutemov@linux.intel.com, wutu.xq2@linux.alibaba.com,
        gerry@linux.alibaba.com
Subject: Re: [RFC PATCH] x86/insn: support decode MOVSXD instruction for MMIO
Message-ID: <20230330025550.GA2522@L-PF27918B-1352.localdomain>
Reply-To: Wu Zongyong <wuzongyong@linux.alibaba.com>
References: <1655f5dc49ab77f94e350ecbdc93e8d9b31acf61.1680058548.git.wuzongyong@linux.alibaba.com>
 <17b65c2a-69a8-6348-da64-641d34be122e@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <17b65c2a-69a8-6348-da64-641d34be122e@amd.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Wed, Mar 29, 2023 at 08:49:24AM -0500, Tom Lendacky wrote:
> On 3/28/23 21:59, Wu Zongyong wrote:
> > It seems MOVSXD which opcode is 0x63 is not handled, support
> > to decode it in insn_decode_mmio().
> 
> Aren't there some caveats to worry about with this instruction based on the
> presence of the REX prefix 64-bit operand size bit? Sometimes it can be a
> sign extended and sometimes it can be a zero extended.

If I undertand right, the patch should like that?

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 558a605929db..a1272f1be35d 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1607,6 +1607,13 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes)
                return INSN_MMIO_DECODE_FAILED;

        switch (insn->opcode.bytes[0]) {
+       case 0x63:
+               *bytes = 4;
+               if (X86_REX_W(insn->rex_prefix.value))
+                       type = INSN_MMIO_READ_SIGN_EXTEND;
+               else
+                       type = INSN_MMIO_READ_ZERO_EXTEND;
+               break;
        case 0x88: /* MOV m8,r8 */
                *bytes = 1;
                fallthrough;
> 
> Thanks,
> Tom
> 
> > 
> > Signed-off-by: Wu Zongyong <wuzongyong@linux.alibaba.com>
> > ---
> >   arch/x86/lib/insn-eval.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
> > index 558a605929db..db6f93bad219 100644
> > --- a/arch/x86/lib/insn-eval.c
> > +++ b/arch/x86/lib/insn-eval.c
> > @@ -1607,6 +1607,10 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes)
> >   		return INSN_MMIO_DECODE_FAILED;
> >   	switch (insn->opcode.bytes[0]) {
> > +	case 0x63: /* MOVSXD r64, m32 */
> > +		*bytes = 4;
> > +		type = INSN_MMIO_READ_SIGN_EXTEND;
> > +		break;
> >   	case 0x88: /* MOV m8,r8 */
> >   		*bytes = 1;
> >   		fallthrough;