xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-4.16 v2] tests/resource: Extend to check that the grant frames are mapped correctly
@ 2021-11-12 14:48 Jane Malalane
  2021-11-12 15:16 ` Roger Pau Monné
  0 siblings, 1 reply; 3+ messages in thread
From: Jane Malalane @ 2021-11-12 14:48 UTC (permalink / raw)
  To: Xen-devel; +Cc: Jane Malalane, Ian Jackson, Roger Pau Monné, Wei Liu

Previously, we checked that we could map 40 pages with nothing
complaining. Now we're adding extra logic to check that those 40
frames are "correct".

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Cast the gnttab pointer to the correct type (void **)
 * Fixed comment style
---
 tools/tests/resource/Makefile        |  2 +
 tools/tests/resource/test-resource.c | 82 +++++++++++++++++++++++++++++++++---
 2 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/tools/tests/resource/Makefile b/tools/tests/resource/Makefile
index 1c3aee4ff7..b3cd70c06d 100644
--- a/tools/tests/resource/Makefile
+++ b/tools/tests/resource/Makefile
@@ -31,10 +31,12 @@ CFLAGS += -Werror
 CFLAGS += $(CFLAGS_xeninclude)
 CFLAGS += $(CFLAGS_libxenctrl)
 CFLAGS += $(CFLAGS_libxenforeginmemory)
+CFLAGS += $(CFLAGS_libxengnttab)
 CFLAGS += $(APPEND_CFLAGS)
 
 LDFLAGS += $(LDLIBS_libxenctrl)
 LDFLAGS += $(LDLIBS_libxenforeignmemory)
+LDFLAGS += $(LDLIBS_libxengnttab)
 LDFLAGS += $(APPEND_LDFLAGS)
 
 %.o: Makefile
diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
index 1caaa60e62..286d338c1f 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -6,6 +6,7 @@
 
 #include <xenctrl.h>
 #include <xenforeignmemory.h>
+#include <xengnttab.h>
 #include <xen-tools/libs.h>
 
 static unsigned int nr_failures;
@@ -17,13 +18,16 @@ static unsigned int nr_failures;
 
 static xc_interface *xch;
 static xenforeignmemory_handle *fh;
+static xengnttab_handle *gh;
 
-static void test_gnttab(uint32_t domid, unsigned int nr_frames)
+static void test_gnttab(uint32_t domid, unsigned int nr_frames, unsigned long gfn)
 {
     xenforeignmemory_resource_handle *res;
-    void *addr = NULL;
+    grant_entry_v1_t *gnttab;
     size_t size;
     int rc;
+    uint32_t refs[nr_frames], domids[nr_frames];
+    void *grants;
 
     printf("  Test grant table\n");
 
@@ -51,18 +55,53 @@ 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);
 
     /*
      * 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;
+    }
+
+    /* 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.
+     */
+    if ( grants == NULL )
+    {
+        fail("    Fail: Map grants %d - %s\n", errno, strerror(errno));
+        goto out;
+    }
+
+    /* Unmap grants. */
+    rc = xengnttab_unmap(gh, grants, nr_frames);
+
+    if ( rc )
+        fail("    Fail: Unmap grants %d - %s\n", errno, strerror(errno));
+
+    /* Unmap grant table. */
+ out:
     rc = xenforeignmemory_unmap_resource(fh, res);
     if ( rc )
-        return fail("    Fail: Unmap %d - %s\n", errno, strerror(errno));
+        return fail("    Fail: Unmap grant table %d - %s\n", errno, strerror(errno));
 }
 
 static void test_domain_configurations(void)
@@ -107,6 +146,7 @@ static void test_domain_configurations(void)
         struct test *t = &tests[i];
         uint32_t domid = 0;
         int rc;
+        xen_pfn_t ram[1] = { 0 };
 
         printf("Test %s\n", t->name);
 
@@ -123,8 +163,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);
+        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);
+        if ( rc )
+        {
+            fail("  Failed to populate physmap domain: %d - %s\n",
+                 errno, strerror(errno));
+            goto test_done;
+        }
+
+        test_gnttab(domid, t->create.max_grant_frames, ram[0]);
 
+    test_done:
         rc = xc_domain_destroy(xch, domid);
         if ( rc )
             fail("  Failed to destroy domain: %d - %s\n",
@@ -138,13 +195,26 @@ int main(int argc, char **argv)
 
     xch = xc_interface_open(NULL, NULL, 0);
     fh = xenforeignmemory_open(NULL, 0);
+    gh = xengnttab_open(NULL, 0);
 
     if ( !xch )
         err(1, "xc_interface_open");
     if ( !fh )
         err(1, "xenforeignmemory_open");
+    if ( !gh )
+        err(1, "xengnttab_open");
 
     test_domain_configurations();
 
     return !!nr_failures;
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH for-4.16 v2] tests/resource: Extend to check that the grant frames are mapped correctly
  2021-11-12 14:48 [PATCH for-4.16 v2] tests/resource: Extend to check that the grant frames are mapped correctly Jane Malalane
@ 2021-11-12 15:16 ` Roger Pau Monné
  2021-11-12 15:19   ` Andrew Cooper
  0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monné @ 2021-11-12 15:16 UTC (permalink / raw)
  To: Jane Malalane; +Cc: Xen-devel, Ian Jackson, Wei Liu

On Fri, Nov 12, 2021 at 02:48:21PM +0000, Jane Malalane wrote:
> Previously, we checked that we could map 40 pages with nothing
> complaining. Now we're adding extra logic to check that those 40
> frames are "correct".
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

There are a couple of lines that exceed the 80 column limit that we
have for other pieces of code, not sure if there's some kind of
exception for tests. If so it might be easy to either resend or fix at
commit.

> ---
> CC: Ian Jackson <iwj@xenproject.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> v2:
>  * Cast the gnttab pointer to the correct type (void **)
>  * Fixed comment style
> ---
>  tools/tests/resource/Makefile        |  2 +
>  tools/tests/resource/test-resource.c | 82 +++++++++++++++++++++++++++++++++---
>  2 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/tests/resource/Makefile b/tools/tests/resource/Makefile
> index 1c3aee4ff7..b3cd70c06d 100644
> --- a/tools/tests/resource/Makefile
> +++ b/tools/tests/resource/Makefile
> @@ -31,10 +31,12 @@ CFLAGS += -Werror
>  CFLAGS += $(CFLAGS_xeninclude)
>  CFLAGS += $(CFLAGS_libxenctrl)
>  CFLAGS += $(CFLAGS_libxenforeginmemory)
> +CFLAGS += $(CFLAGS_libxengnttab)
>  CFLAGS += $(APPEND_CFLAGS)
>  
>  LDFLAGS += $(LDLIBS_libxenctrl)
>  LDFLAGS += $(LDLIBS_libxenforeignmemory)
> +LDFLAGS += $(LDLIBS_libxengnttab)
>  LDFLAGS += $(APPEND_LDFLAGS)
>  
>  %.o: Makefile
> diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
> index 1caaa60e62..286d338c1f 100644
> --- a/tools/tests/resource/test-resource.c
> +++ b/tools/tests/resource/test-resource.c
> @@ -6,6 +6,7 @@
>  
>  #include <xenctrl.h>
>  #include <xenforeignmemory.h>
> +#include <xengnttab.h>
>  #include <xen-tools/libs.h>
>  
>  static unsigned int nr_failures;
> @@ -17,13 +18,16 @@ static unsigned int nr_failures;
>  
>  static xc_interface *xch;
>  static xenforeignmemory_handle *fh;
> +static xengnttab_handle *gh;
>  
> -static void test_gnttab(uint32_t domid, unsigned int nr_frames)
> +static void test_gnttab(uint32_t domid, unsigned int nr_frames, unsigned long gfn)

Nit: i think this line exceeds the 80 column limit.

>  {
>      xenforeignmemory_resource_handle *res;
> -    void *addr = NULL;
> +    grant_entry_v1_t *gnttab;
>      size_t size;
>      int rc;
> +    uint32_t refs[nr_frames], domids[nr_frames];
> +    void *grants;
>  
>      printf("  Test grant table\n");
>  
> @@ -51,18 +55,53 @@ 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);
>  
>      /*
>       * 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));

Likewise.

>  
> +    /* 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;
> +    }
> +
> +    /* Map grants. */
> +    grants = xengnttab_map_grant_refs(gh, nr_frames, domids, refs, PROT_READ | PROT_WRITE);

Here.

> +
> +    /*
> +     * 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.
> +     */
> +    if ( grants == NULL )
> +    {
> +        fail("    Fail: Map grants %d - %s\n", errno, strerror(errno));
> +        goto out;
> +    }
> +
> +    /* Unmap grants. */
> +    rc = xengnttab_unmap(gh, grants, nr_frames);
> +
> +    if ( rc )
> +        fail("    Fail: Unmap grants %d - %s\n", errno, strerror(errno));
> +
> +    /* Unmap grant table. */
> + out:
>      rc = xenforeignmemory_unmap_resource(fh, res);
>      if ( rc )
> -        return fail("    Fail: Unmap %d - %s\n", errno, strerror(errno));
> +        return fail("    Fail: Unmap grant table %d - %s\n", errno, strerror(errno));

Here.

>  }
>  
>  static void test_domain_configurations(void)
> @@ -107,6 +146,7 @@ static void test_domain_configurations(void)
>          struct test *t = &tests[i];
>          uint32_t domid = 0;
>          int rc;
> +        xen_pfn_t ram[1] = { 0 };
>  
>          printf("Test %s\n", t->name);
>  
> @@ -123,8 +163,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);
> +        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);

And here also.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH for-4.16 v2] tests/resource: Extend to check that the grant frames are mapped correctly
  2021-11-12 15:16 ` Roger Pau Monné
@ 2021-11-12 15:19   ` Andrew Cooper
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2021-11-12 15:19 UTC (permalink / raw)
  To: Roger Pau Monné, Jane Malalane; +Cc: Xen-devel, Ian Jackson, Wei Liu

On 12/11/2021 15:16, Roger Pau Monné wrote:
> On Fri, Nov 12, 2021 at 02:48:21PM +0000, Jane Malalane wrote:
>> Previously, we checked that we could map 40 pages with nothing
>> complaining. Now we're adding extra logic to check that those 40
>> frames are "correct".
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
> LGTM:
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> There are a couple of lines that exceed the 80 column limit that we
> have for other pieces of code, not sure if there's some kind of
> exception for tests. If so it might be easy to either resend or fix at
> commit.

I'll fix on commit.  Thanks.

~Andrew


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-12 15:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 14:48 [PATCH for-4.16 v2] tests/resource: Extend to check that the grant frames are mapped correctly Jane Malalane
2021-11-12 15:16 ` Roger Pau Monné
2021-11-12 15:19   ` Andrew Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).