Mailing List Archive

# HG changeset patch
# User kaf24@firebug.cl.cam.ac.uk
# Node ID 48eb10d7a2d608351023b14e45a9ddb7f99cab8a
# Parent f2a08a5a807acddced92db03a5a717bc66c95cc5
Increment page reference count for every host/device
mapping created via a grant reference, rather than one
increment for all uses of a single grant reference.
This avoids a nasty situation in put_page_from_l1e()
where we cannot reliably determine whether a mapping was
created via a grant reference and so we must always
decrement the mapped page's reference count.

Signed-off-by: Keir Fraser <keir@xensource.com>

diff -r f2a08a5a807a -r 48eb10d7a2d6 xen/common/grant_table.c
--- a/xen/common/grant_table.c Wed Dec 21 17:16:31 2005
+++ b/xen/common/grant_table.c Wed Dec 21 17:25:34 2005
@@ -177,17 +177,19 @@

spin_lock(&rd->grant_table->lock);

- if ( act->pin == 0 )
- {
- /* CASE 1: Activating a previously inactive entry. */
-
+ if ( !act->pin ||
+ (!(dev_hst_ro_flags & GNTMAP_readonly) &&
+ !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask))) )
+ {
sflags = sha->flags;
sdom = sha->domid;

- /* This loop attempts to set the access (reading/writing) flags
+ /*
+ * This loop attempts to set the access (reading/writing) flags
* in the grant table entry. It tries a cmpxchg on the field
* up to five times, and then fails under the assumption that
- * the guest is misbehaving. */
+ * the guest is misbehaving.
+ */
for ( ; ; )
{
u32 scombo, prev_scombo, new_scombo;
@@ -196,7 +198,7 @@
unlikely(sdom != led->domain->domain_id) )
PIN_FAIL(unlock_out, GNTST_general_error,
"Bad flags (%x) or dom (%d). (NB. expected dom %d)\n",
- sflags, sdom, led->domain->domain_id);
+ sflags, sdom, led->domain->domain_id);

/* Merge two 16-bit values into a 32-bit combined update. */
/* NB. Endianness! */
@@ -232,132 +234,50 @@
sdom = (u16)(prev_scombo >> 16);
}

- /* rmb(); */ /* not on x86 */
-
- frame = __gpfn_to_mfn(rd, sha->frame);
-
- if ( unlikely(!pfn_valid(frame)) ||
- unlikely(!((dev_hst_ro_flags & GNTMAP_readonly) ?
- get_page(pfn_to_page(frame), rd) :
- get_page_and_type(pfn_to_page(frame), rd,
- PGT_writable_page))) )
- {
- clear_bit(_GTF_writing, &sha->flags);
- clear_bit(_GTF_reading, &sha->flags);
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Could not pin the granted frame (%lx)!\n", frame);
+ if ( !act->pin )
+ {
+ act->domid = sdom;
+ act->frame = __gpfn_to_mfn(rd, sha->frame);
+ }
+ }
+ else if ( (act->pin & 0x80808080U) != 0 )
+ PIN_FAIL(unlock_out, ENOSPC,
+ "Risk of counter overflow %08x\n", act->pin);
+
+ if ( dev_hst_ro_flags & GNTMAP_device_map )
+ act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
+ GNTPIN_devr_inc : GNTPIN_devw_inc;
+ if ( dev_hst_ro_flags & GNTMAP_host_map )
+ act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
+ GNTPIN_hstr_inc : GNTPIN_hstw_inc;
+
+ spin_unlock(&rd->grant_table->lock);
+
+ frame = act->frame;
+ if ( unlikely(!pfn_valid(frame)) ||
+ unlikely(!((dev_hst_ro_flags & GNTMAP_readonly) ?
+ get_page(pfn_to_page(frame), rd) :
+ get_page_and_type(pfn_to_page(frame), rd,
+ PGT_writable_page))) )
+ PIN_FAIL(undo_out, GNTST_general_error,
+ "Could not pin the granted frame (%lx)!\n", frame);
+
+ if ( dev_hst_ro_flags & GNTMAP_host_map )
+ {
+ rc = create_grant_host_mapping(addr, frame, dev_hst_ro_flags);
+ if ( rc != GNTST_okay )
+ {
+ if ( !(dev_hst_ro_flags & GNTMAP_readonly) )
+ put_page_type(pfn_to_page(frame));
+ put_page(pfn_to_page(frame));
+ goto undo_out;
}

if ( dev_hst_ro_flags & GNTMAP_device_map )
- act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
- GNTPIN_devr_inc : GNTPIN_devw_inc;
- if ( dev_hst_ro_flags & GNTMAP_host_map )
- act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
- GNTPIN_hstr_inc : GNTPIN_hstw_inc;
- act->domid = sdom;
- act->frame = frame;
- }
- else
- {
- /* CASE 2: Active modications to an already active entry. */
-
- /*
- * A cheesy check for possible pin-count overflow.
- * A more accurate check cannot be done with a single comparison.
- */
- if ( (act->pin & 0x80808080U) != 0 )
- PIN_FAIL(unlock_out, ENOSPC,
- "Risk of counter overflow %08x\n", act->pin);
-
- sflags = sha->flags;
- frame = act->frame;
-
- if ( !(dev_hst_ro_flags & GNTMAP_readonly) &&
- !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
- {
- for ( ; ; )
- {
- u16 prev_sflags;
-
- if ( unlikely(sflags & GTF_readonly) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Attempt to write-pin a r/o grant entry.\n");
-
- prev_sflags = sflags;
-
- /* NB. prev_sflags is updated in place to seen value. */
- if ( unlikely(cmpxchg_user(&sha->flags, prev_sflags,
- prev_sflags | GTF_writing)) )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Fault while modifying shared flags.\n");
-
- if ( likely(prev_sflags == sflags) )
- break;
-
- if ( retries++ == 4 )
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Shared grant entry is unstable.\n");
-
- sflags = prev_sflags;
- }
-
- if ( unlikely(!get_page_type(pfn_to_page(frame),
- PGT_writable_page)) )
- {
- clear_bit(_GTF_writing, &sha->flags);
- PIN_FAIL(unlock_out, GNTST_general_error,
- "Attempt to write-pin a unwritable page.\n");
- }
- }
-
- if ( dev_hst_ro_flags & GNTMAP_device_map )
- act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
- GNTPIN_devr_inc : GNTPIN_devw_inc;
-
- if ( dev_hst_ro_flags & GNTMAP_host_map )
- act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
- GNTPIN_hstr_inc : GNTPIN_hstw_inc;
- }
-
- /*
- * At this point:
- * act->pin updated to reference count mappings.
- * sha->flags updated to indicate to granting domain mapping done.
- * frame contains the mfn.
- */
-
- spin_unlock(&rd->grant_table->lock);
-
- if ( dev_hst_ro_flags & GNTMAP_host_map )
- {
- rc = create_grant_host_mapping(addr, frame, dev_hst_ro_flags);
- if ( rc < 0 )
- {
- /* Failure: undo and abort. */
-
- spin_lock(&rd->grant_table->lock);
-
- if ( dev_hst_ro_flags & GNTMAP_readonly )
- {
- act->pin -= GNTPIN_hstr_inc;
- }
- else
- {
- act->pin -= GNTPIN_hstw_inc;
- if ( (act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) == 0 )
- {
- clear_bit(_GTF_writing, &sha->flags);
- put_page_type(pfn_to_page(frame));
- }
- }
-
- if ( act->pin == 0 )
- {
- clear_bit(_GTF_reading, &sha->flags);
- put_page(pfn_to_page(frame));
- }
-
- spin_unlock(&rd->grant_table->lock);
+ {
+ (void)get_page(pfn_to_page(frame), rd);
+ if ( !(dev_hst_ro_flags & GNTMAP_readonly) )
+ get_page_type(pfn_to_page(frame), PGT_writable_page);
}
}

@@ -375,6 +295,24 @@
put_domain(rd);
return rc;

+ undo_out:
+ spin_lock(&rd->grant_table->lock);
+
+ if ( dev_hst_ro_flags & GNTMAP_device_map )
+ act->pin -= (dev_hst_ro_flags & GNTMAP_readonly) ?
+ GNTPIN_devr_inc : GNTPIN_devw_inc;
+ if ( dev_hst_ro_flags & GNTMAP_host_map )
+ act->pin -= (dev_hst_ro_flags & GNTMAP_readonly) ?
+ GNTPIN_hstr_inc : GNTPIN_hstw_inc;
+
+ if ( !(dev_hst_ro_flags & GNTMAP_readonly) &&
+ !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
+ clear_bit(_GTF_writing, &sha->flags);
+
+ if ( !act->pin )
+ clear_bit(_GTF_reading, &sha->flags);
+
+ spin_unlock(&rd->grant_table->lock);

unlock_out:
spin_unlock(&rd->grant_table->lock);
@@ -465,27 +403,42 @@
PIN_FAIL(unmap_out, GNTST_general_error,
"Bad frame number doesn't match gntref.\n");
if ( flags & GNTMAP_device_map )
- act->pin -= (flags & GNTMAP_readonly) ? GNTPIN_devr_inc
- : GNTPIN_devw_inc;
-
- map->ref_and_flags &= ~GNTMAP_device_map;
- (void)__put_user(0, &uop->dev_bus_addr);
- }
-
- if ( (addr != 0) &&
- (flags & GNTMAP_host_map) &&
- ((act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask)) > 0))
+ {
+ ASSERT(act->pin & (GNTPIN_devw_mask | GNTPIN_devr_mask));
+ map->ref_and_flags &= ~GNTMAP_device_map;
+ if ( flags & GNTMAP_readonly )
+ {
+ act->pin -= GNTPIN_devr_inc;
+ put_page(pfn_to_page(frame));
+ }
+ else
+ {
+ act->pin -= GNTPIN_devw_inc;
+ put_page_and_type(pfn_to_page(frame));
+ }
+ }
+ }
+
+ if ( (addr != 0) && (flags & GNTMAP_host_map) )
{
if ( (rc = destroy_grant_host_mapping(addr, frame, flags)) < 0 )
goto unmap_out;

+ ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask));
map->ref_and_flags &= ~GNTMAP_host_map;
-
- act->pin -= (flags & GNTMAP_readonly) ? GNTPIN_hstr_inc
- : GNTPIN_hstw_inc;
- }
-
- if ( (map->ref_and_flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0)
+ if ( flags & GNTMAP_readonly )
+ {
+ act->pin -= GNTPIN_hstr_inc;
+ put_page(pfn_to_page(frame));
+ }
+ else
+ {
+ act->pin -= GNTPIN_hstw_inc;
+ put_page_and_type(pfn_to_page(frame));
+ }
+ }
+
+ if ( (map->ref_and_flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0 )
{
map->ref_and_flags = 0;
put_maptrack_handle(ld->grant_table, handle);
@@ -495,20 +448,12 @@
if ( !(flags & GNTMAP_readonly) )
gnttab_log_dirty(rd, frame);

- /* If the last writable mapping has been removed, put_page_type */
if ( ((act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0) &&
!(flags & GNTMAP_readonly) )
- {
clear_bit(_GTF_writing, &sha->flags);
- put_page_type(pfn_to_page(frame));
- }

if ( act->pin == 0 )
- {
- act->frame = 0xdeadbeef;
clear_bit(_GTF_reading, &sha->flags);
- put_page(pfn_to_page(frame));
- }

unmap_out:
(void)__put_user(rc, &uop->status);
@@ -989,42 +934,40 @@
{
if ( map->ref_and_flags & GNTMAP_device_map )
{
- BUG_ON((act->pin & GNTPIN_devr_mask) == 0);
+ BUG_ON(!(act->pin & GNTPIN_devr_mask));
act->pin -= GNTPIN_devr_inc;
+ put_page(pfn_to_page(act->frame));
}

if ( map->ref_and_flags & GNTMAP_host_map )
{
- BUG_ON((act->pin & GNTPIN_hstr_mask) == 0);
+ BUG_ON(!(act->pin & GNTPIN_hstr_mask));
act->pin -= GNTPIN_hstr_inc;
+ put_page(pfn_to_page(act->frame));
}
}
else
{
if ( map->ref_and_flags & GNTMAP_device_map )
{
- BUG_ON((act->pin & GNTPIN_devw_mask) == 0);
+ BUG_ON(!(act->pin & GNTPIN_devw_mask));
act->pin -= GNTPIN_devw_inc;
+ put_page_and_type(pfn_to_page(act->frame));
}

if ( map->ref_and_flags & GNTMAP_host_map )
{
- BUG_ON((act->pin & GNTPIN_hstw_mask) == 0);
+ BUG_ON(!(act->pin & GNTPIN_hstw_mask));
act->pin -= GNTPIN_hstw_inc;
+ put_page_and_type(pfn_to_page(act->frame));
}

if ( (act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0 )
- {
clear_bit(_GTF_writing, &sha->flags);
- put_page_type(pfn_to_page(act->frame));
- }
}

if ( act->pin == 0 )
- {
clear_bit(_GTF_reading, &sha->flags);
- put_page(pfn_to_page(act->frame));
- }

spin_unlock(&rd->grant_table->lock);


_______________________________________________
Xen-changelog mailing list
Xen-changelog@lists.xensource.com
http://lists.xensource.com/xen-changelog