On 18.10.2021 12:08, Jane Malalane wrote:
> @@ -51,18 +55,52 @@ static void test_gnttab(uint32_t domid, unsigned int
> nr_frames)
> res = xenforeignmemory_map_resource(
> fh, domid, XENMEM_resource_grant_table,
> XENMEM_resource_grant_table_id_shared, 0, size >> XC_PAGE_SHIFT,
> - &addr, PROT_READ | PROT_WRITE, 0);
> + (void *)&gnttab, PROT_READ | PROT_WRITE, 0);
While in testing code I'm not as concerned about casts, I think it would
be better to cast to a visibly correct type, i.e. maintaining the levels
of indirection (void **). Alternatively you could (ab)use grants here,
allowing to drop the cast, by then assigning grants to gnttab.
> /*
> * Failure here with E2BIG indicates Xen is missing the bugfix to map
> * resources larger than 32 frames.
> */
> if ( !res )
> - return fail(" Fail: Map %d - %s\n", errno, strerror(errno));
> + return fail(" Fail: Map grant table %d - %s\n", errno,
> strerror(errno));
>
> + /* Put each gref at a unique offset in its frame. */
> + for ( unsigned int i = 0; i < nr_frames; i++ )
> + {
> + unsigned int gref = i * (XC_PAGE_SIZE / sizeof(*gnttab)) + i;
> +
> + refs[i] = gref;
> + domids[i] = domid;
> +
> + gnttab[gref].domid = 0;
> + gnttab[gref].frame = gfn;
> + gnttab[gref].flags = GTF_permit_access;
> + }
To make obvious that you're done with gnttab[], perhaps better unmap it
here rather than at the bottom?
> + /* Map grants. */
> + grants = xengnttab_map_grant_refs(gh, nr_frames, domids, refs, PROT_READ
> | PROT_WRITE);
> +
> + /* Failure here indicates either that the frames were not mapped
> + * in the correct order or xenforeignmemory_map_resource() didn't
> + * give us the frames we asked for to begin with.
> + */
Nit: Comment style.
> @@ -123,8 +162,25 @@ static void test_domain_configurations(void)
>
> printf(" Created d%u\n", domid);
>
> - test_gnttab(domid, t->create.max_grant_frames);
> + rc = xc_domain_setmaxmem(xch, domid, -1);
That's an unbelievably large upper bound that you set. Since you
populate ...
> + if ( rc )
> + {
> + fail(" Failed to set max memory for domain: %d - %s\n",
> + errno, strerror(errno));
> + goto test_done;
> + }
> +
> + rc = xc_domain_populate_physmap_exact(xch, domid, ARRAY_SIZE(ram),
> 0, 0, ram);
... only a single page, can't you get away with a much smaller value?
And without engaging truncation from uint64_t to unsigned int in
XEN_DOMCTL_max_mem handling in the hypervisor (which imo would better
yield an error)?
Jan
