Annie,
Comments inline below...
> -----Original Message-----
[snip]
> -static struct grant_entry *shared;
> +static union {
> + struct grant_entry_v1 *v1;
> + void *ring_addr;
> +} shared;
> +
'ring_addr' seems like the wrong name here; how about 'raw'?
> +/*
> + * 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?
> +/*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?
> +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.
> - 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
|
Home