Thanks, Paul. See following,
On 2011-11-9 19:11, Paul Durrant wrote:
+/*
+ * This function is null for grant table v1, adding it here in
order to
+keep
+ * consistent with *_v2 interface.
+ */
+static int gnttab_map_status_v1(unsigned int nr_gframes);
+/*
+ * This function is null for grant table v1, adding it here in
order to
+keep
+ * consistent with *_v2 interface.
+ */
+static void gnttab_unmap_status_v1(void);
+
I don't really like the idea of having null operations. How about abstracting at the level of gnttab_map/unmap so that you can include the status mapping for v2 but just do the arch_gnttab_map_shared for v1?
I see. v2 function includes mapping and arch_gnttab_map_shared, v1
function only include arch_gnttab_map_sh, right? This will lead to some
code duplicated in two functions.
+/*This is a structure of function pointers for grant table v1*/
static
+struct {
+ /*
+ * Mapping a list of frames for storing grant entry status,
this
+ * mechanism can allow better synchronization using barriers.
Input
+ * parameter is frame number, returning GNTST_okay means
success and
+ * negative value means failure.
+ */
+ int (*_gnttab_map_status)(unsigned int);
+ /*
+ * Release a list of frames which are mapped in
_gnttab_map_status for
+ * grant entry status.
+ */
+ void (*_gnttab_unmap_status)(void);
+ /*
+ * Introducing a valid entry into the grant table, granting
the frame
+ * of this grant entry to domain for accessing, or
transfering, or
+ * transitively accessing. First input parameter is reference
of this
+ * introduced grant entry, second one is domid of granted
domain, third
+ * one is the frame to be granted, and the last one is status
of the
+ * grant entry to be updated.
+ */
+ void (*_update_grant_entry)(grant_ref_t, domid_t,
+ unsigned long, unsigned);
+ /*
+ * Stop granting a grant entry to domain for accessing. First
input
+ * parameter is reference of a grant entry whose grant access
will be
+ * stopped, second one is not in use now. If the grant entry
is
+ * currently mapped for reading or writing, just return
failure(==0)
+ * directly and don't tear down the grant access. Otherwise,
stop grant
+ * access for this entry and return success(==1).
+ */
+ int (*_gnttab_end_foreign_access_ref)(grant_ref_t, int);
+ /*
+ * Stop granting a grant entry to domain for transfer. If
tranfer has
+ * not started, just reclaim the grant entry and return
failure(==0).
+ * Otherwise, wait for the transfer to complete and then
return the
+ * frame.
+ */
+ unsigned long
(*_gnttab_end_foreign_transfer_ref)(grant_ref_t);
+ /*
+ * Query the status of a grant entry. Input parameter is
reference of
+ * queried grant entry, return value is the status of queried
entry.
+ * Detailed status(writing/reading) can be gotten from the
return value
+ * by bit operations.
+ */
+ int (*_gnttab_query_foreign_access)(grant_ref_t);
+} gnttab_interface;
+
Why the leading '_' in the names?
Just in order to differ those function pointers from the original
functions.
+static int grant_table_version;
static struct gnttab_free_callback *gnttab_free_callback_list;
@@ -142,6 +207,15 @@ static void put_free_entry(grant_ref_t ref)
spin_unlock_irqrestore(&gnttab_list_lock, flags); }
+static void update_grant_entry_v1(grant_ref_t ref, domid_t domid,
+ unsigned long frame, unsigned flags) {
+ shared.v1[ref].frame = frame;
+ shared.v1[ref].domid = domid;
+ wmb();
+ shared.v1[ref].flags = flags;
+}
+
static void update_grant_entry(grant_ref_t ref, domid_t domid,
unsigned long frame, unsigned flags) {
@@ -155,12 +229,10 @@ static void update_grant_entry(grant_ref_t
ref, domid_t domid,
* 3. Write memory barrier (WMB).
* 4. Write ent->flags, inc. valid type.
*/
The comment above probably should be moved into the v1 function itself since the v2 code differs.
Yes, you are right. Comments for v2 should be added too.
Thanks
Annie
- shared[ref].frame = frame;
- shared[ref].domid = domid;
- wmb();
- shared[ref].flags = flags;
+ gnttab_interface._update_grant_entry(ref, domid, frame,
flags);
}
[snip]
Paul
|