Re: GNTTABOP_setup_table yields -1 PFNs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2024 04:22, Taylor R Campbell wrote:
> On a Xen 4.14 host (with extraversion=.0.88.g1d1d1f53), with version 1
> grant tables, where GNTTABOP_query_size initially returns nr_frames=32
> max_nr_frames=64, a NetBSD guest repeatedly queries
> GNTTABOP_setup_table for successively larger nr_frames from 1 up.

First question: Is there some earlier GNTTABOP_setup_table that you invoke?
I'd expect (and also observe) nr_frames=1 initially.

Second: The version you name is pretty unclear from an upstream perspective.
Leaving aside that 4.14 is out of support, it's entirely unclear whether you
at least have all bug fixes in place that we have upstream (4.14.6). Without
that it's hard to see what you're asking for.

> The guest initially gets arrays of valid-looking PFNs.  But then at
> nr_frames=33, the PFNs [0] through [31] in the resulting array are
> valid but PFN [32] is -1, i.e., all bits set.
> 
> GNTTABOP_setup_table returned 0 and op.status = GNTST_okay, so it
> didn't fail -- it just returned an invalid PFN.  And _after_
> GNTTABOP_setup_table yields -1 as a PFN, GNTTABOP_query_size returns
> nr_frames=33 max_nr_frames=64, so the host thinks it has successfully
> allocated more frames.
> 
> What could cause the host to return a PFN -1?  Is there anything the
> guest does that could provoke this?  Are there any diagnostics that
> the guest could print to help track this down?  (I don't control the
> host.)  Should a guest just check for -1 and stop as if it had hit
> max_nr_frames?

I'm afraid for the moment, from just the information provided, I can't
reproduce this using a simple patch on top of XTF's self-test (see below).
Neither with a 64-bit PV guest, nor with a 32-bit one. I've been doing
this with a pretty recent 4.19 Xen, though.

Jan

--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -275,6 +275,23 @@ struct gnttab_setup_table
 };
 
 /*
+ * GNTTABOP_query_size: Query the current and maximum sizes of the shared
+ * grant table.
+ * NOTES:
+ *  1. <dom> may be specified as DOMID_SELF.
+ *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
+ */
+#define GNTTABOP_query_size           6
+struct gnttab_query_size {
+    /* IN parameters. */
+    domid_t  dom;
+    /* OUT parameters. */
+    uint32_t nr_frames;
+    uint32_t max_nr_frames;
+    int16_t  status;              /* => enum grant_status */
+};
+
+/*
  * GNTTABOP_unmap_and_replace: Destroy one or more grant-reference mappings
  * tracked by <handle> but atomically replace the page table entry with one
  * pointing to the machine address under <new_addr>.  <new_addr> will be
--- a/tests/selftest/main.c
+++ b/tests/selftest/main.c
@@ -230,6 +230,57 @@ static void test_driver_init(void)
     if ( rc )
         xtf_failure("Fail: xtf_init_grant_table(1) returned %d\n", rc);
 
+    struct gnttab_query_size query = { .dom = DOMID_SELF };
+    rc = hypercall_grant_table_op(GNTTABOP_query_size, &query, 1);
+    if ( rc )
+        xtf_failure("Fail: GNTTABOP_query_size returned %d\n", rc);
+    if ( query.status != GNTST_okay )
+        xtf_failure("Fail: GNTTABOP_query_size status %d\n", query.status);
+    printk("gnttab: nr=%u max=%u\n", query.nr_frames, query.max_nr_frames);
+
+    unsigned long frame_list[36], prev[ARRAY_SIZE(frame_list)];
+    if ( query.max_nr_frames > ARRAY_SIZE(frame_list) )
+        query.max_nr_frames = ARRAY_SIZE(frame_list);
+
+    if ( query.max_nr_frames > 32 )
+    {
+        struct gnttab_setup_table setup = {
+            .dom = DOMID_SELF,
+            .nr_frames = 32,
+            .frame_list = frame_list,
+        };
+
+        rc = hypercall_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+        if ( rc )
+            xtf_failure("Fail: GNTTABOP_setup_table(%u) returned %d (%u)\n",
+                        setup.nr_frames, rc, query.max_nr_frames);
+        if ( setup.status != GNTST_okay )
+            xtf_failure("Fail: GNTTABOP_setup_table(%u) status %d (%u)\n",
+                        setup.nr_frames, setup.status, query.max_nr_frames);
+        printk("gnttab: tbl[%u] @ %lx\n",
+               setup.nr_frames - 1, frame_list[setup.nr_frames - 1]);
+    }
+
+    for ( unsigned int n = 1; n <= query.max_nr_frames; ++n )
+    {
+        struct gnttab_setup_table setup = {
+            .dom = DOMID_SELF,
+            .nr_frames = n,
+            .frame_list = frame_list,
+        };
+
+        rc = hypercall_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+        if ( rc )
+            xtf_failure("Fail: GNTTABOP_setup_table(%u) returned %d\n", n, rc);
+        if ( setup.status != GNTST_okay )
+            xtf_failure("Fail: GNTTABOP_setup_table(%u) status %d\n", n, 
setup.status);
+        printk("gnttab: tbl[%u] @ %lx\n", n - 1, frame_list[n - 1]);
+        for ( unsigned int i = 0; i < n - 1; ++i )
+            if ( frame_list[i] != prev[i] )
+                printk("gnttab: %lx != %lx\n", frame_list[i], prev[i]);
+        memcpy(prev, frame_list, n * sizeof(*frame_list));
+    }
+
     rc = xtf_init_grant_table(2);
     if ( rc && rc != -ENODEV )
         xtf_failure("Fail: xtf_init_grant_table(2) returned %d\n", rc);