Re: [Xen-devel] Xen-unstable-staging: Xen BUG at iommu_map.c:455

[Tim Deegan <tim@xxxxxxx>]

At 16:10 +0100 on 05 May (1430842206), Jan Beulich wrote:
> >>> On 16.04.15 at 11:28, <tim@xxxxxxx> wrote:
> > At 22:35 +0100 on 11 Apr (1428791713), Andrew Cooper wrote:
> >> I am not certain that it is the correct way to fix the issue, nor that
> >> the ioreq server code is the only way to trigger it.  There are several
> >> ways to shoot a gfn mapping from the guests physmap.
> >> 
> >> At least we now understand why it happens.  I will defer to others CC'd
> >> on this thread for their opinions in the matter.
> > 
> > The patch semes like a pretty good check to me, though I'm not
> > convinced it's race-free.  At the least I'd cache the m2p lookup so we
> > use the same value for the checks and the map_page() call. 
> 
> Did you have a chance to look at the patch Sander meanwhile
> successfully tested [1]?

Just looked at it now.

> I'm trying to understand where you see
> possible races here, and hence whether anything else needs to
> be done to that patch before formally submitting it.

It caches the m2p mlookup, which I like, but there's still a race
against concurrent p2m updates.

> From what I
> can tell (and assuming other code works correctly) the fact that
> arch_iommu_populate_page_table() sets d->need_iommu to -1
> first thing should make sure that any subsequent changes to the
> p2m get propagated to IOMMU code for setting up respective
> mappings.

Yes, but might they then be overridden by the previous mapping when
this new code calls map_page()?

This seems like a case where we should be using get_gfn()/put_gfn().

Cheers,

Tim.

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