Re: [PATCH 0/2] MMIO emulation fixes

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 10 August 2018 13:56
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich'
> <JBeulich@suse.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
> 
> On 10/08/18 13:43, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 10 August 2018 13:37
> >> To: Paul Durrant <Paul.Durrant@citrix.com>
> >> Cc: xen-devel <xen-devel@lists.xenproject.org>
> >> Subject: RE: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
> >>
> >>>>> On 10.08.18 at 14:22, <Paul.Durrant@citrix.com> wrote:
> >>>>  -----Original Message-----
> >>>> From: Jan Beulich [mailto:JBeulich@suse.com]
> >>>> Sent: 10 August 2018 13:13
> >>>> To: Paul Durrant <Paul.Durrant@citrix.com>
> >>>> Cc: xen-devel <xen-devel@lists.xenproject.org>
> >>>> Subject: RE: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
> >>>>
> >>>>>>> On 10.08.18 at 14:08, <Paul.Durrant@citrix.com> wrote:
> >>>>>>  -----Original Message-----
> >>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
> >>>>>> Sent: 10 August 2018 13:02
> >>>>>> To: Paul Durrant <Paul.Durrant@citrix.com>
> >>>>>> Cc: xen-devel <xen-devel@lists.xenproject.org>
> >>>>>> Subject: Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
> >>>>>>
> >>>>>>>>> On 10.08.18 at 12:37, <paul.durrant@citrix.com> wrote:
> >>>>>>> These are probably both candidates for back-port.
> >>>>>>>
> >>>>>>> Paul Durrant (2):
> >>>>>>>   x86/hvm/ioreq: MMIO range checking completely ignores
> direction
> >> flag
> >>>>>>>   x86/hvm/emulate: make sure rep I/O emulation does not cross
> GFN
> >>>>>>>     boundaries
> >>>>>>>
> >>>>>>>  xen/arch/x86/hvm/emulate.c | 17 ++++++++++++++++-
> >>>>>>>  xen/arch/x86/hvm/ioreq.c   | 15 ++++++++++-----
> >>>>>>>  2 files changed, 26 insertions(+), 6 deletions(-)
> >>>>>> I take it this isn't yet what we've talked about yesterday on irc?
> >>>>>>
> >>>>> This is the band-aid fix. I can now show correct handling of a rep mov
> >>>>> walking off MMIO into RAM.
> >>>> But that's not the problem we're having. In our case the bad behavior
> >>>> is with a single MOV. That's why I had assumed that your plan to fiddle
> >>>> with null_handler would help in our case as well, while this series
> clearly
> >>>> won't (afaict).
> >>>>
> >>> Oh, I see. A single MOV spanning MMIO and RAM has undefined
> behaviour
> >> though
> >>> as I understand it. Am I incorrect?
> >> I'm not aware of SDM or PM saying anything like this. Anyway, the
> >> specific case where this is being observed as an issue is when
> >> accessing the last few bytes of a normal RAM page followed by a
> >> ballooned out one. The balloon driver doesn't remove the virtual
> >> mapping of such pages (presumably in order to not shatter super
> >> pages); observation is with the old XenoLinux one, but from code
> >> inspection the upstream one behaves the same.
> >>
> >> Unless we want to change the balloon driver's behavior, at least
> >> this specific case needs to be considered having defined behavior,
> >> I think.
> >>
> > Ok. I'll see what I can do.
> 
> It is a software error to try and cross boundaries.  Modern processors
> do their best to try and cause the correct behaviour to occur, albeit
> with a massive disclaimer about the performance hit.  Older processors
> didn't cope.
> 
> As far as I'm concerned, its fine to terminate a emulation which crosses
> a boundary with the null ops.

Alas we never even get as far as the I/O handlers in some circumstances...

I just set up a variant of an XTF test doing a backwards rep movsd into a well aligned stack buffer where source buffer starts 1 byte before a boundary between RAM and MMIO. The code in hvmemul_rep_movs() (rightly) detects that both the source and dest of the initial rep are RAM, skips over the I/O emulation calls, and then fails when the hvm_copy_from_guest_phys() (unsurprisingly) fails to grab the 8 bytes for the initial rep.
So, any logic we add to deal with handling page spanning ops is going to have to go in at the top level of instruction emulation... which I fear is going to be quite a major change and not something that's going to be easy to back-port.

  Paul

> 
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel