File 19268-page-get-owner.patch of Package xen
# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1236176930 0
# Node ID 4b7d638a8b89b6d49094e77d6295c6d8ffafc192
# Parent 7f573cb76db41a9fc46052867b02e9e0c107aa86
Be careful with page_get_owner() now that owner field can be clobbered
by some users. Introduce get_page_owner_and_reference() where that can
be more useful.
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1236245557 0
# Node ID 3673926b2375a10c55dafb1a6c478e22ee0b08f2
# Parent 4b7d638a8b89b6d49094e77d6295c6d8ffafc192
[IA64] Remove compilation warning and typo caused by 19268:4b7d638a8b89
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1237303760 0
# Node ID e655cb27d0857cb4ad45da1fbcef2824706d392e
# Parent c09a815400801bdbc933a6b6139ffaa1ec342ea5
x86: Fix get_page() to not drop reference count if it wasn't incremented.
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -349,14 +349,14 @@ void share_xen_page_with_privileged_gues
#else
/*
* In debug builds we shadow a selection of <4GB PDPTs to exercise code paths.
- * We cannot safely shadow the idle page table, nor shadow (v1) page tables
- * (detected by lack of an owning domain). As required for correctness, we
+ * We cannot safely shadow the idle page table, nor shadow page tables
+ * (detected by zero reference count). As required for correctness, we
* always shadow PDPTs above 4GB.
*/
-#define l3tab_needs_shadow(mfn) \
- (((((mfn) << PAGE_SHIFT) != __pa(idle_pg_table)) && \
- (page_get_owner(mfn_to_page(mfn)) != NULL) && \
- ((mfn) & 1)) || /* odd MFNs are shadowed */ \
+#define l3tab_needs_shadow(mfn) \
+ (((((mfn) << PAGE_SHIFT) != __pa(idle_pg_table)) && \
+ (mfn_to_page(mfn)->count_info & PGC_count_mask) && \
+ ((mfn) & 1)) || /* odd MFNs are shadowed */ \
((mfn) >= 0x100000))
#endif
@@ -646,7 +646,16 @@ get_##level##_linear_pagetable(
int is_iomem_page(unsigned long mfn)
{
- return (!mfn_valid(mfn) || (page_get_owner(mfn_to_page(mfn)) == dom_io));
+ struct page_info *page;
+
+ if ( !mfn_valid(mfn) )
+ return 1;
+
+ /* Caller must know that it is an iomem page, or a reference is held. */
+ page = mfn_to_page(mfn);
+ ASSERT((page->count_info & PGC_count_mask) != 0);
+
+ return (page_get_owner(page) == dom_io);
}
@@ -659,7 +668,6 @@ get_page_from_l1e(
uint32_t l1f = l1e_get_flags(l1e);
struct vcpu *curr = current;
struct domain *owner;
- int okay;
if ( !(l1f & _PAGE_PRESENT) )
return 1;
@@ -670,8 +678,13 @@ get_page_from_l1e(
return 0;
}
- if ( is_iomem_page(mfn) )
+ if ( !mfn_valid(mfn) ||
+ (owner = page_get_owner_and_reference(page)) == dom_io )
{
+ /* Only needed the reference to confirm dom_io ownership. */
+ if ( mfn_valid(mfn) )
+ put_page(page);
+
/* DOMID_IO reverts to caller for privilege checks. */
if ( d == dom_io )
d = curr->domain;
@@ -687,34 +700,29 @@ get_page_from_l1e(
return 1;
}
+ if ( owner == NULL )
+ goto could_not_pin;
+
/*
* Let privileged domains transfer the right to map their target
* domain's pages. This is used to allow stub-domain pvfb export to dom0,
* until pvfb supports granted mappings. At that time this minor hack
* can go away.
*/
- owner = page_get_owner(page);
- if ( unlikely(d != owner) && (owner != NULL) &&
- (d != curr->domain) && IS_PRIV_FOR(d, owner) )
+ if ( unlikely(d != owner) && (d != curr->domain) && IS_PRIV_FOR(d, owner) )
d = owner;
/* Foreign mappings into guests in shadow external mode don't
* contribute to writeable mapping refcounts. (This allows the
* qemu-dm helper process in dom0 to map the domain's memory without
* messing up the count of "real" writable mappings.) */
- okay = (((l1f & _PAGE_RW) &&
- !(unlikely(paging_mode_external(d) && (d != curr->domain))))
- ? get_page_and_type(page, d, PGT_writable_page)
- : get_page(page, d));
- if ( !okay )
- {
- MEM_LOG("Error getting mfn %lx (pfn %lx) from L1 entry %" PRIpte
- " for dom%d",
- mfn, get_gpfn_from_mfn(mfn),
- l1e_get_intpte(l1e), d->domain_id);
- }
- else if ( pte_flags_to_cacheattr(l1f) !=
- ((page->count_info >> PGC_cacheattr_base) & 7) )
+ if ( (l1f & _PAGE_RW) &&
+ !(paging_mode_external(d) && (d != curr->domain)) &&
+ !get_page_type(page, PGT_writable_page) )
+ goto could_not_pin;
+
+ if ( pte_flags_to_cacheattr(l1f) !=
+ ((page->count_info >> PGC_cacheattr_base) & 7) )
{
unsigned long x, nx, y = page->count_info;
unsigned long cacheattr = pte_flags_to_cacheattr(l1f);
@@ -743,7 +751,16 @@ get_page_from_l1e(
#endif
}
- return okay;
+ return 1;
+
+ could_not_pin:
+ MEM_LOG("Error getting mfn %lx (pfn %lx) from L1 entry %" PRIpte
+ " for dom%d",
+ mfn, get_gpfn_from_mfn(mfn),
+ l1e_get_intpte(l1e), d->domain_id);
+ if ( owner != NULL )
+ put_page(page);
+ return 0;
}
@@ -1116,7 +1133,7 @@ static int create_pae_xen_mappings(struc
for ( i = 0; i < PDPT_L2_ENTRIES; i++ )
{
l2e = l2e_from_page(
- virt_to_page(page_get_owner(page)->arch.mm_perdomain_pt) + i,
+ virt_to_page(d->arch.mm_perdomain_pt) + i,
__PAGE_HYPERVISOR);
l2e_write(&pl2e[l2_table_offset(PERDOMAIN_VIRT_START) + i], l2e);
}
@@ -1866,30 +1883,41 @@ void put_page(struct page_info *page)
}
-int get_page(struct page_info *page, struct domain *domain)
+struct domain *page_get_owner_and_reference(struct page_info *page)
{
unsigned long x, y = page->count_info;
do {
x = y;
- if ( unlikely((x & PGC_count_mask) == 0) || /* Not allocated? */
- /* Keep one spare reference to be acquired by get_page_light(). */
- unlikely(((x + 2) & PGC_count_mask) <= 1) ) /* Overflow? */
- goto fail;
+ /*
+ * Count == 0: Page is not allocated, so we cannot take a reference.
+ * Count == -1: Reference count would wrap, which is invalid.
+ * Count == -2: Remaining unused ref is reserved for get_page_light().
+ */
+ if ( unlikely(((x + 2) & PGC_count_mask) <= 2) )
+ return NULL;
}
while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x );
- if ( likely(page_get_owner(page) == domain) )
+ return page_get_owner(page);
+}
+
+
+int get_page(struct page_info *page, struct domain *domain)
+{
+ struct domain *owner = page_get_owner_and_reference(page);
+
+ if ( likely(owner == domain) )
return 1;
- put_page(page);
+ if ( owner != NULL )
+ put_page(page);
- fail:
if ( !_shadow_mode_refcounts(domain) && !domain->is_dying )
gdprintk(XENLOG_INFO,
- "Error pfn %lx: rd=%p, od=%p, caf=%08lx, taf=%" PRtype_info,
- page_to_mfn(page), domain, page_get_owner(page),
- y, page->u.inuse.type_info);
+ "Error pfn %lx: rd=%p, od=%p, caf=%08lx, taf=%" PRtype_info "\n",
+ page_to_mfn(page), domain, owner,
+ page->count_info, page->u.inuse.type_info);
return 0;
}
@@ -1915,7 +1943,6 @@ static void get_page_light(struct page_i
while ( unlikely(y != x) );
}
-
static int alloc_page_type(struct page_info *page, unsigned long type,
int preemptible)
{
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -195,7 +195,7 @@ static void
__gnttab_map_grant_ref(
struct gnttab_map_grant_ref *op)
{
- struct domain *ld, *rd;
+ struct domain *ld, *rd, *owner;
struct vcpu *led;
int handle;
unsigned long frame = 0, nr_gets = 0;
@@ -335,8 +335,13 @@ __gnttab_map_grant_ref(
spin_unlock(&rd->grant_table->lock);
- if ( is_iomem_page(frame) )
+ if ( !mfn_valid(frame) ||
+ (owner = page_get_owner_and_reference(mfn_to_page(frame))) == dom_io )
{
+ /* Only needed the reference to confirm dom_io ownership. */
+ if ( mfn_valid(frame) )
+ put_page(mfn_to_page(frame));
+
if ( !iomem_access_permitted(rd, frame, frame) )
{
gdprintk(XENLOG_WARNING,
@@ -351,20 +356,11 @@ __gnttab_map_grant_ref(
if ( rc != GNTST_okay )
goto undo_out;
}
- else
+ else if ( owner == rd )
{
- if ( unlikely(!mfn_valid(frame)) ||
- unlikely(!(gnttab_host_mapping_get_page_type(op, ld, rd) ?
- get_page_and_type(mfn_to_page(frame), rd,
- PGT_writable_page) :
- get_page(mfn_to_page(frame), rd))) )
- {
- if ( !rd->is_dying )
- gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n",
- frame);
- rc = GNTST_general_error;
- goto undo_out;
- }
+ if ( gnttab_host_mapping_get_page_type(op, ld, rd) &&
+ !get_page_type(mfn_to_page(frame), PGT_writable_page) )
+ goto could_not_pin;
nr_gets++;
if ( op->flags & GNTMAP_host_map )
@@ -382,6 +378,17 @@ __gnttab_map_grant_ref(
}
}
}
+ else
+ {
+ could_not_pin:
+ if ( !rd->is_dying )
+ gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n",
+ frame);
+ if ( owner != NULL )
+ put_page(mfn_to_page(frame));
+ rc = GNTST_general_error;
+ goto undo_out;
+ }
if ( need_iommu(ld) &&
!(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) &&
--- a/xen/common/xencomm.c
+++ b/xen/common/xencomm.c
@@ -51,24 +51,16 @@ xencomm_get_page(unsigned long paddr, st
return -EFAULT;
*page = maddr_to_page(maddr);
- if ( get_page(*page, current->domain) == 0 )
+ if ( !get_page(*page, current->domain) )
{
- if ( page_get_owner(*page) != current->domain )
- {
- /*
- * This page might be a page granted by another domain, or
- * this page is freed with decrease reservation hypercall at
- * the same time.
- */
- gdprintk(XENLOG_WARNING,
- "bad page is passed. paddr 0x%lx maddr 0x%lx\n",
- paddr, maddr);
- return -EFAULT;
- }
-
- /* Try again. */
- cpu_relax();
- return -EAGAIN;
+ /*
+ * This page might be a page granted by another domain, or this page
+ * is freed with decrease reservation hypercall at the same time.
+ */
+ gdprintk(XENLOG_WARNING,
+ "bad page is passed. paddr 0x%lx maddr 0x%lx\n",
+ paddr, maddr);
+ return -EFAULT;
}
return 0;
--- a/xen/include/asm-ia64/mm.h
+++ b/xen/include/asm-ia64/mm.h
@@ -156,28 +156,40 @@ static inline void put_page(struct page_
free_domheap_page(page);
}
+static inline struct domain *page_get_owner_and_reference(
+ struct page_info *page)
+{
+ unsigned long x, y = page->count_info;
+
+ do {
+ x = y;
+ if (unlikely((x & PGC_count_mask) == 0) || /* Not allocated? */
+ unlikely(((x + 1) & PGC_count_mask) == 0) ) {/* Count overflow? */
+ return NULL;
+ }
+ y = cmpxchg_acq(&page->count_info, x, x + 1);
+ } while (unlikely(y != x));
+
+ return page_get_owner(page);
+}
+
/* count_info and ownership are checked atomically. */
static inline int get_page(struct page_info *page,
struct domain *domain)
{
- u64 x, nx, y = *((u64*)&page->count_info);
- u32 _domain = pickle_domptr(domain);
+ struct domain *owner = page_get_owner_and_reference(page);
- do {
- x = y;
- nx = x + 1;
- if (unlikely((x & PGC_count_mask) == 0) || /* Not allocated? */
- unlikely((nx & PGC_count_mask) == 0) || /* Count overflow? */
- unlikely((x >> 32) != _domain)) { /* Wrong owner? */
-
- gdprintk(XENLOG_INFO, "Error pfn %lx: rd=%p, od=%p, caf=%016lx, taf=%"
- PRtype_info "\n", page_to_mfn(page), domain,
- unpickle_domptr(x >> 32), x, page->u.inuse.type_info);
- return 0;
- }
- }
- while(unlikely((y = cmpxchg_acq((u64*)&page->count_info, x, nx)) != x));
- return 1;
+ if (likely(owner == domain))
+ return 1;
+
+ put_page(page);
+
+ /* if (!domain->is_dying) */ /* XXX: header inclusion hell */
+ gdprintk(XENLOG_INFO,
+ "Error pfn %lx: rd=%p, od=%p, caf=%016lx, taf=%" PRtype_info "\n",
+ page_to_mfn(page), domain,
+ owner, page->count_info, page->u.inuse.type_info);
+ return 0;
}
int is_iomem_page(unsigned long mfn);
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -262,6 +262,7 @@ void cleanup_page_cacheattr(struct page_
int is_iomem_page(unsigned long mfn);
+struct domain *page_get_owner_and_reference(struct page_info *page);
void put_page(struct page_info *page);
int get_page(struct page_info *page, struct domain *domain);
void put_page_type(struct page_info *page);
