Re: [PATCH v2 3/3] kvm: svm: Use the hardware provided GPA instead of page walk

[Brijesh Singh]

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;