At 20:34 +0000 on 16 Dec (1292531690), Keir Fraser wrote:
> On 16/12/2010 17:03, "Keir Fraser" <keir@xxxxxxx> wrote:
>
> > On 16/12/2010 16:50, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
> >
> >>> approach therefore. Perhaps *(volatile type *)px = x or, really, even
> >>> better
> >>> I should define some {read,write}_atomic{8,16,32,64} accessor functions
> >>> which use inline asm to absolutely definitely emit a single atomic 'mov'
> >>> instruction.
> >>>
> >>> Make sense?
> >>
> >> Yes.
> >
> > Excellent. I will lay groundwork and fix pte_{read,write}_atomic directly in
> > -unstable and -4.0-testing. I will then post a proposed fix for EPT to the
> > list. I don't know that code so well and I may not otherwise catch all
> > places that require use of the new accessor macros.
>
> Attached is a patch I've knocked up for p2m-ept.c. I don't know how complete
> it really is. Perhaps someone (George?) would like to Ack it as is, or
> develop it further.
George, I think the underlying logic is still racy - the
check-and-populate function is checking a pointer that was found outside
the lock. It needs to start again from the beginning to be safe, which
probably means just dropping the "check" part and letting the
p2m_pod_demand_populate handle lost races. Also, why doesn't
ept_get_entry use a single read at the lowest level?
Sorting out the locking and commenting in this code is on my
ever-expanding list of New Year's resolutions. In the meantime,
Keir's patch much should definitely go in:
Acked-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
and also this to fix one other very-non-atomic write:
Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
diff -r cded713fc3bd xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c Fri Dec 17 10:16:11 2010 +0000
+++ b/xen/arch/x86/mm/hap/p2m-ept.c Fri Dec 17 11:11:01 2010 +0000
@@ -713,7 +713,7 @@ void ept_change_entry_emt_with_range(str
static void ept_change_entry_type_page(mfn_t ept_page_mfn, int ept_page_level,
p2m_type_t ot, p2m_type_t nt)
{
- ept_entry_t *epte = map_domain_page(mfn_x(ept_page_mfn));
+ ept_entry_t e, *epte = map_domain_page(mfn_x(ept_page_mfn));
for ( int i = 0; i < EPT_PAGETABLE_ENTRIES; i++ )
{
@@ -725,11 +725,13 @@ static void ept_change_entry_type_page(m
ept_page_level - 1, ot, nt);
else
{
- if ( epte[i].sa_p2mt != ot )
+ e = epte[i];
+ if ( e.sa_p2mt != ot )
continue;
- epte[i].sa_p2mt = nt;
- ept_p2m_type_to_flags(epte + i, nt);
+ e.sa_p2mt = nt;
+ ept_p2m_type_to_flags(&e, nt);
+ atomic_write_ept_entry(epte + i, e);
}
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home