* [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417
@ 2011-03-15 15:41 George Dunlap
2011-03-16 9:40 ` Tim Deegan
2011-03-17 16:18 ` Ian Jackson
0 siblings, 2 replies; 5+ messages in thread
From: George Dunlap @ 2011-03-15 15:41 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
This fixes a bug in changeset 22526:7a5ee3800417, where the wrong
value is read when deciding whether to flush the VTd tables. The
effect is minor: in situations where the p2m entry is changed but the
mfn is the same, the VTd tables will be unnecessarily flushed.
old_entry is left untouched in the second case, since having a present
old_entry will cause the an unnecessary check to be taken at the end
of the function.
Spotted-by: Zhang, Peng Fei <zpfalpc23@gmail.com>
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r 3caed2112c65 -r 56b32e6fd315 xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c Tue Mar 15 10:14:27 2011 +0000
+++ b/xen/arch/x86/mm/hap/p2m-ept.c Tue Mar 15 15:41:17 2011 +0000
@@ -374,7 +374,9 @@
needs_sync = 0;
/* If we're replacing a non-leaf entry with a leaf entry (1GiB or 2MiB),
- * the intermediate tables will be freed below after the ept flush */
+ * the intermediate tables will be freed below after the ept flush
+ *
+ * Read-then-write is OK because we hold the p2m lock. */
old_entry = *ept_entry;
if ( mfn_valid(mfn_x(mfn)) || direct_mmio || p2m_is_paged(p2mt) ||
@@ -390,10 +392,10 @@
new_entry.access = p2ma;
new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop);
- if ( new_entry.mfn == mfn_x(mfn) )
+ new_entry.mfn = mfn_x(mfn);
+
+ if ( old_entry.mfn == new_entry.mfn )
need_modify_vtd_table = 0;
- else
- new_entry.mfn = mfn_x(mfn);
ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);
}
@@ -438,10 +440,12 @@
new_entry.access = p2ma;
new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop);
- if ( new_entry.mfn == mfn_x(mfn) )
+ /* the caller should take care of the previous page */
+ new_entry.mfn = mfn_x(mfn);
+
+ /* Safe to read-then-write because we hold the p2m lock */
+ if ( ept_entry->mfn == new_entry.mfn )
need_modify_vtd_table = 0;
- else /* the caller should take care of the previous page */
- new_entry.mfn = mfn_x(mfn);
ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417
2011-03-15 15:41 [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417 George Dunlap
@ 2011-03-16 9:40 ` Tim Deegan
2011-03-17 16:18 ` Ian Jackson
1 sibling, 0 replies; 5+ messages in thread
From: Tim Deegan @ 2011-03-16 9:40 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
At 15:41 +0000 on 15 Mar (1300203711), George Dunlap wrote:
> This fixes a bug in changeset 22526:7a5ee3800417, where the wrong
> value is read when deciding whether to flush the VTd tables. The
> effect is minor: in situations where the p2m entry is changed but the
> mfn is the same, the VTd tables will be unnecessarily flushed.
>
> old_entry is left untouched in the second case, since having a present
> old_entry will cause the an unnecessary check to be taken at the end
> of the function.
>
> Spotted-by: Zhang, Peng Fei <zpfalpc23@gmail.com>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Applied, thanks.
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417
2011-03-15 15:41 [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417 George Dunlap
2011-03-16 9:40 ` Tim Deegan
@ 2011-03-17 16:18 ` Ian Jackson
2011-03-17 16:30 ` Tim Deegan
1 sibling, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2011-03-17 16:18 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
George Dunlap writes ("[Xen-devel] [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417"):
> This fixes a bug in changeset 22526:7a5ee3800417, where the wrong
> value is read when deciding whether to flush the VTd tables. The
> effect is minor: in situations where the p2m entry is changed but the
> mfn is the same, the VTd tables will be unnecessarily flushed.
Does this need applying to 4.1 before the release ?
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417
2011-03-17 16:18 ` Ian Jackson
@ 2011-03-17 16:30 ` Tim Deegan
2011-03-17 17:45 ` George Dunlap
0 siblings, 1 reply; 5+ messages in thread
From: Tim Deegan @ 2011-03-17 16:30 UTC (permalink / raw)
To: Ian Jackson; +Cc: George Dunlap, xen-devel
At 16:18 +0000 on 17 Mar (1300378713), Ian Jackson wrote:
> George Dunlap writes ("[Xen-devel] [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417"):
> > This fixes a bug in changeset 22526:7a5ee3800417, where the wrong
> > value is read when deciding whether to flush the VTd tables. The
> > effect is minor: in situations where the p2m entry is changed but the
> > mfn is the same, the VTd tables will be unnecessarily flushed.
>
> Does this need applying to 4.1 before the release ?
I don't think so, but it should be backported afterwards.
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417
2011-03-17 16:30 ` Tim Deegan
@ 2011-03-17 17:45 ` George Dunlap
0 siblings, 0 replies; 5+ messages in thread
From: George Dunlap @ 2011-03-17 17:45 UTC (permalink / raw)
To: Tim Deegan; +Cc: George Dunlap, xen-devel, Ian Jackson
On Thu, 2011-03-17 at 16:30 +0000, Tim Deegan wrote:
> At 16:18 +0000 on 17 Mar (1300378713), Ian Jackson wrote:
> > George Dunlap writes ("[Xen-devel] [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417"):
> > > This fixes a bug in changeset 22526:7a5ee3800417, where the wrong
> > > value is read when deciding whether to flush the VTd tables. The
> > > effect is minor: in situations where the p2m entry is changed but the
> > > mfn is the same, the VTd tables will be unnecessarily flushed.
> >
> > Does this need applying to 4.1 before the release ?
>
> I don't think so, but it should be backported afterwards.
I tend to agree.
-George
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-17 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-15 15:41 [PATCH] ept: Fix bug in changeset 22526:7a5ee3800417 George Dunlap
2011-03-16 9:40 ` Tim Deegan
2011-03-17 16:18 ` Ian Jackson
2011-03-17 16:30 ` Tim Deegan
2011-03-17 17:45 ` George Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.