Re: [Xen-devel] [PATCH v3 04/18] x86/hvm: make sure translated MMIO reads or writes fall within a page

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 23 June 2015 16:53
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
> Subject: Re: [PATCH v3 04/18] x86/hvm: make sure translated MMIO reads or
> writes fall within a page
> 
> >>> On 23.06.15 at 12:39, <paul.durrant@xxxxxxxxxx> wrote:
> > ...otherwise they will simply carry on to the next page using a normal
> > linear-to-phys translation.
> 
> And what's wrong about this?
> 

Is it the right thing to do? It seems wrong.

> > --- a/xen/arch/x86/hvm/emulate.c
> > +++ b/xen/arch/x86/hvm/emulate.c
> > @@ -586,7 +586,6 @@ static int __hvmemul_read(
> >                                          p_data);
> >              if ( rc != X86EMUL_OKAY || bytes == chunk )
> >                  return rc;
> > -            addr += chunk;
> >              off += chunk;
> >              gpa += chunk;
> >              p_data += chunk;
> > @@ -594,6 +593,8 @@ static int __hvmemul_read(
> >              if ( bytes < chunk )
> >                  chunk = bytes;
> >          }
> > +
> > +        return X86EMUL_UNHANDLEABLE;
> >      }
> 
> All the loop above does is leverage that we don't need to do a
> translation, as we already know the physical address. Continuing
> if the access crosses a page boundary is not wrong at all afaict.
> 

In what circumstances would you expect this to happen. My cscope showed up 
handle_mmio_with_translation() being called by the shadow code and nested page 
fault code.
The nested code does not look like it expects the I/O to cross a page boundary. 
Particularly it appears to do checks on the gpa without considering the 
possibility that the I/O may spill on to a subsequent page. Similarly the 
shadow code appears to do a va to gpa lookup assuming again that the 
translation is valid for the whole I/O.

I'm willing to admit that I only have a very basic knowledge of the code in 
this area but, on the face of it, just walking onto the next linear address 
after translation does not seem like it's the right thing to do.

  Paul

> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel