Skip to content

Commit

Permalink
Initial implementation of xen_bus_dma_tag_create()
Browse files Browse the repository at this point in the history
This is a rough implementation of the function. Needs more finishing.
  • Loading branch information
Pratyush Yadav committed May 25, 2018
1 parent 43c4401 commit 3c94ef4
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
55 changes: 55 additions & 0 deletions sys/dev/xen/grant_table/grant_table.c
Expand Up @@ -585,6 +585,61 @@ gnttab_expand(unsigned int req_entries)
return (error);
}

int
xen_bus_dma_tag_create(bus_dma_tag_t parent, bus_size_t alignment,
bus_addr_t boundary, bus_addr_t lowaddr, bus_addr_t highaddr,
bus_dma_filter_t *filtfunc, void *filtfuncarg, bus_size_t maxsize,
int nsegments, bus_size_t maxsegsz, int flags,
bus_dma_lock_t *lockfunc, void *lockfuncarg, bus_dma_tag_t *dmat,
grant_ref_t **refs)

This comment has been minimized.

Copy link
@royger

royger Jun 4, 2018

This extra 'refs' parameter is not part of the current busdma interface:

bus_dma_tag_create(bus_dma_tag_t parent, bus_size_t alignment,
bus_addr_t boundary, bus_addr_t lowaddr, bus_addr_t highaddr,
bus_dma_filter_t *filtfunc, void *filtfuncarg, bus_size_t maxsize,
int nsegments, bus_size_t maxsegsz, int flags,
bus_dma_lock_t *lockfunc, void *lockfuncarg, bus_dma_tag_t *dmat);

Are you planning to modify the busdma interface in order to add a new parameter?

IMO the Xen specific data should be hidden in bus_dma_tag_t.

{
domid_t domid;
int i, j, error;

if (maxsegsz < PAGE_SIZE) {
return (EINVAL);
}

domid = flags >> 16;
flags &= 0xffff;

/* Allocate a new dma tag. */
error = bus_dma_tag_create(parent, alignment, boundary, lowaddr, highaddr,
filtfunc, filterarg, maxsize, nsegments, maxsegsz, flags, lockfunc,
lockfuncarg, dmat);
if (error) {
return error;
}

/* Allocate the grant references for each segment. */
*refs = malloc(nsegments*sizeof(grant_ref_t), M_DEVBUF, M_NOWAIT);
if((*refs) == NULL) {
bus_dma_tag_destroy(dmat);
return (ENOMEM);
}

/*
* Create entries in the grant table for each segment. The frame number is set
* to 0 for now. It will be updated when the dma map is loaded.
*/
for (i = 0; i < nsegments; i++) {
error = gnttab_grant_foreign_access(domid, 0, 0, (*refs[i]));

This comment has been minimized.

Copy link
@royger

royger Jun 4, 2018

You are sharing the same address (0) multiple times here without any need, basically allowing the domain in ther domid parameter to access all the data in this page. Please don't do this, it's a security risk.

if (error) {
for (j = 0; j <= i; j++) {
gnttab_end_foreign_access_ref((*refs[j]));
free(*refs);
*refs = NULL;

This comment has been minimized.

Copy link
@royger

royger Jun 4, 2018

This is wrong, this for loop is only going to be executed once because you have an unconditional return inside the loop.

Note that doing 'free(*refs)' and '*refs = NULL' in the loop will lead to memory dereferencing if the loop is executed more than once.

bus_dma_tag_destroy(dmat);
return error;
}
}
}

/* XXX Leaving cleaning up the refs array to the caller. Is it ok? */

return 0;
}

static void
xen_bus_dmamap_load_callback(void *callback_arg, bus_dma_segment_t
*segs, int nseg, int error)
Expand Down
8 changes: 7 additions & 1 deletion sys/xen/gnttab.h
Expand Up @@ -124,7 +124,13 @@ int gnttab_resume(device_t);
* transparent integration with the rest of the system.
*/

int xen_bus_dmamap_load(bus_dma_tag_t dmat, bus_dmamap_t map, void *buf,

This comment has been minimized.

Copy link
@royger

royger Jun 4, 2018

Please fix this whitespace issue in the commit that you add it, not here.

int xen_bus_dma_tag_create(bus_dma_tag_t parent, bus_size_t alignment,
bus_addr_t boundary, bus_addr_t lowaddr, bus_addr_t highaddr,
bus_dma_filter_t *filtfunc, void *filtfuncarg, bus_size_t maxsize,
int nsegments, bus_size_t maxsegsz, int flags,
bus_dma_lock_t *lockfunc, void *lockfuncarg, bus_dma_tag_t *dmat);

int xen_bus_dmamap_load(bus_dma_tag_t dmat, bus_dmamap_t map, void *buf,
bus_size_t buflen, bus_dmamap_callback_t *callback,
void *callback_arg, int flags);

Expand Down

0 comments on commit 3c94ef4

Please sign in to comment.