Subject: Re: [PATCH v2 3/3] kvm: svm: Use the hardware provided GPA instead of
 page walk
To: Paolo Bonzini <pbonzini@redhat.com>
References: <147992048887.27638.17559991037474542240.stgit@brijesh-build-machine>
 <147992052008.27638.18095073174935903705.stgit@brijesh-build-machine>
 <65a10dd8-5fae-350f-b597-f8f0261da766@redhat.com>
 <9820037d-e3ca-0131-3b04-2e51f2abc883@amd.com>
 <de0d35c7-1aa1-f7f1-fc59-1020b6982558@amd.com>
 <657442146.2535029.1481298111651.JavaMail.zimbra@redhat.com>
 <6e1fd4ba-016c-99ea-5b98-24f89479da4b@amd.com>
 <057d8a2e-0a3a-38d3-d9bf-9301e3eb8238@redhat.com>
 <43e626f1-e9a6-cf5d-4771-c6f7ca07ec8e@amd.com>
 <ed5a3921-00b0-9214-0767-62585fb6b166@redhat.com>
CC: <brijesh.singh@amd.com>, <kvm@vger.kernel.org>,
        thomas lendacky <thomas.lendacky@amd.com>, <rkrcmar@redhat.com>,
        <joro@8bytes.org>, <x86@kernel.org>, <linux-kernel@vger.kernel.org>,
        <mingo@redhat.com>, <hpa@zytor.com>, <tglx@linutronix.de>,
        <bp@suse.de>
From: Brijesh Singh <brijesh.singh@amd.com>
Message-ID: <8ddf2c3b-833e-4c88-ff14-d1826171703e@amd.com>
Date: Wed, 14 Dec 2016 12:39:01 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.5.1
MIME-Version: 1.0
In-Reply-To: <ed5a3921-00b0-9214-0767-62585fb6b166@redhat.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Dec 2016 18:39:06.1939 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR12MB0668
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1951
Lines: 51


On 12/14/2016 11:23 AM, Paolo Bonzini wrote:
>
>
> On 14/12/2016 18:07, Brijesh Singh wrote:
>>>
>>
>> Since now we are going to perform multiple conditional checks before
>> concluding that its safe to use HW provided GPA. How about if we add two
>> functions "emulator_is_rep_string_op" and "emulator_is_two_mem_op" into
>> emulator.c and  use these functions inside the x86.c to determine if its
>> safe to use HW provided gpa?
>
> Why not export only emulator_can_use_gpa from emulate.c?  (So in the end
> leaving emulator_is_string_op in emulate.c was the right thing to do, it
> was just the test that was wrong :)).
>

Actually, I was not sure if putting emulator_can_use_gpa() in emulate.c 
was right thing - mainly because emulator.c does not deal with GPA. I 
will go with your advice and put it in emulator.c, it makes easy :)


> The patch below is still missing the check for cross-page MMIO.  Your
> reference to the BKDG only covers MMCONFIG (sometimes referred to as
> ECAM), not MMIO in general.  Doing AND or OR into video memory for
> example is perfectly legal, and I'm fairly sure that some obscure legacy
> software does PUSH/POP into vram as well!
>
>

I used your below code snippet to detect cross-page MMIO access. After 
applying these changes cross-page MMIO read/write unit test is passing 
just fine. I will include it in patch.

 > Actually there is a nice trick you can do to support cross-page
 > MMIO access detection:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 37cd31645d45..754d251dc611 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4549,6 +4549,7 @@ static int emulator_read_write_onepage(unsigned 
long addr, void *val,
  	 */
  	if (vcpu->arch.gpa_available &&
  	    !emulator_can_use_hw_gpa(ctxt) &&
+	    (addr & ~PAGE_MASK) == (exception->address & ~PAGE_MASK) &&
  	    vcpu_is_mmio_gpa(vcpu, addr, exception->address, write)) {
  		gpa = exception->address;
  		goto mmio;