Keir,
"Quite easily fixed." You've been doing this too long. I'm glad you looked at
it since you found all this and I likely wouldn't have without being slapped
upside the head to look deeper. I don't have a test case, so I have been
reading the code and testing the edge cases in my head and most of it looks
good. Questions:
Major:
1.) In hvmemul_virtual_to_linear(), you've added the min() (line 299) on reps
for reasons I don't understand and the ASSERT (line 304) in the reverse case. I
don't see anything, anywhere, that guarantee that the ASSERT is true...and it
needs to be for the code to be correct. If the min() is meant to guarantee this
somehow, I don't see how it does. If it isn't meant to do this, I don't
understand what it is for, as written.
Minor:
2.) In hvmemul_linear_to_phys(), you changed the exception injection (line 265)
to use (addr & PAGE_MASK) instead of addr? Seems wrong, but there could easily
be something I don't understand.
John Byrne
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
> Sent: Tuesday, August 19, 2008 8:05 AM
> To: Byrne, John (HP Labs); xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about
> direction-flag?
>
> This should be fixed as of c/s 18340. Feel free to see if you can spot
> any
> problems with it! I'll roll another release candidate tomorrow morning,
> after the tree has been through automated testing.
>
> Curently it's in staging only:
> http://xenbits.xensource.com/staging/xen-unstable.hg
>
> -- Keir
>
> On 19/8/08 11:48, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote:
>
> > This affects hvmemul_linear_to_phys() too, and would for example mean
> that a
> > backwards I/O string instruction in userspace that crosses a page
> boundary
> > would very likely cause I/O to/from the wrong physical pages. I've
> confirmed
> > this with a small testing patch to hvmloader. I think we need to work
> out how
> > to maintain a test suite of this kind of thing to check for
> regressions in
> > these kinds of rarer corner cases.
> >
> > Obviously I'll fix this for 3.3.0 and probably roll out another
> release
> > candidate.
> >
> > -- Keir
> >
> > On 19/8/08 09:20, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote:
> >
> >> Quite a nasty omission though, and quite easily fixed. Thanks for
> pointing
> >> it out.
> >>
> >> -- Keir
> >>
> >> On 18/8/08 19:41, "Byrne, John (HP Labs)" <john.l.byrne@xxxxxx>
> wrote:
> >>
> >>> I was following the emulation code around in xen-unstable cs 18335
> and I
> >>> noticed that the direction flag doesn't get taken into account for
> the
> >>> segment
> >>> bounds checking in the 32-bit case anywhere I could see. Does
> anyone know
> >>> better?
> >>>
> >>> Maybe no one will care, but I thought I'd mention it.
> >>>
> >>> John Byrne
> >>>
> >>>
> >>> _______________________________________________
> >>> Xen-devel mailing list
> >>> Xen-devel@xxxxxxxxxxxxxxxxxxx
> >>> http://lists.xensource.com/xen-devel
> >>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home