Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] KVM: X86: Avoid explictly fetch instruction in
 x86_decode_insn()
To:     Peter Xu <peterx@redhat.com>, linmiaohe <linmiaohe@huawei.com>
Cc:     Sean Christopherson <sean.j.christopherson@intel.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>
References: <05ca4e7e070844dd92e4f673a1bc15d9@huawei.com>
 <20200304153253.GB7146@xz-x1>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <ad023c34-9a08-7d61-22de-911c4e8760ba@redhat.com>
Date:   Wed, 4 Mar 2020 19:19:58 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <20200304153253.GB7146@xz-x1>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 04/03/20 16:32, Peter Xu wrote:
>> Looks good, thanks. But it seems we should also take care of the comment in __do_insn_fetch_bytes(), as we do not
>> load instruction at the beginning of x86_decode_insn() now, which may be misleading:
>> 		/*
>>          * One instruction can only straddle two pages,
>>          * and one has been loaded at the beginning of
>>          * x86_decode_insn.  So, if not enough bytes
>>          * still, we must have hit the 15-byte boundary.
>>          */
>>         if (unlikely(size < op_size))
>>                 return emulate_gp(ctxt, 0);
> Right, thanks for spotting that (even if the patch to be dropped :).
> 
> I guess not only the comment, but the check might even fail if we
> apply the patch. Because when the fetch is the 1st attempt and
> unluckily that acrosses one page boundary (because we'll only fetch
> until either 15 bytes or the page boundary), so that single fetch
> could be smaller than op_size provided.

Right, priming the decode cache with one byte from the current page
cannot fail, and then we know that the next call must be at the
beginning of the next page.

Paolo