WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH 1/3] Introducing grant table V2 stucture

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
  
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>