[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-4.16 v4] gnttab: allow setting max version per-domain





On 29/10/2021 10:41, Roger Pau Monné wrote:
On Fri, Oct 29, 2021 at 09:58:55AM +0100, Julien Grall wrote:
Hi Roger,
Hi Roger,

On 29/10/2021 08:59, Roger Pau Monne wrote:
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index e510395d08..f94f0f272c 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -53,6 +53,7 @@ struct grant_table {
       percpu_rwlock_t       lock;
       /* Lock protecting the maptrack limit */
       spinlock_t            maptrack_lock;
+    unsigned int          max_version;
       /*
        * Defaults to v1.  May be changed with GNTTABOP_set_version.  All other
        * values are invalid.
@@ -1917,11 +1918,33 @@ active_alloc_failed:
   }
   int grant_table_init(struct domain *d, int max_grant_frames,
-                     int max_maptrack_frames)
+                     int max_maptrack_frames, unsigned int options)
   {
       struct grant_table *gt;
+    unsigned int max_grant_version = options & XEN_DOMCTL_GRANT_version_mask;
       int ret = -ENOMEM;
+    if ( max_grant_version == XEN_DOMCTL_GRANT_version_default )
+        max_grant_version = opt_gnttab_max_version;
+    if ( !max_grant_version )
+    {
+        dprintk(XENLOG_INFO, "%pd: invalid grant table version 0 requested\n",
+                d);
+        return -EINVAL;
+    }
+    if ( max_grant_version > opt_gnttab_max_version )
+    {
+        dprintk(XENLOG_INFO,
+                "%pd: requested grant version (%u) greater than supported 
(%u)\n",
+                d, max_grant_version, opt_gnttab_max_version);
+        return -EINVAL;
+    }
+    if ( unlikely(max_page >= PFN_DOWN(TB(16))) && is_pv_domain(d) &&

 From my understanding, the limit for the grant table v1 is based on the page
granularity used and the size of the fields.

So the limit you add is valid for 4KB but not 16KB/64KB. Therefore, I think
it would be better to use:

'max_page >= (1U << 32)'

I'm slightly confused. Isn't Xen always using a 4KB page granularity,

Yes. We only support 4KB today. But most of Xen is agnostic to the page granularity. I have actually started to look to allow 64KB/16KB page granularity for Xen on Arm in my spare time.

and that also applies to the grant table entries?
The page granularity for the hypercall interface is whatever the page granularity Xen is using. So...


I don't think it's possible to use correctly use a 16KB or 64KB page
as an entry for the grant table, as Xen assumes those to always be 4KB
based.

... if you build Xen with 16KB, then the grant table entries will be using 16KB.

So I would like to avoid making the assumption that we are always using 4KB. That said, the worse that can happen is a spurious message. So this is more to get an accurate check.


Furthermore, it would add a comment explaining where this limit comes from.

Lastly, did you check the compiler wouldn't throw an error on arm32?

I've tested a previous version (v2), but not this one. I assume it
doesn't build?

I haven't tried. But I remember in the past seen report for always true/false check. Maybe that was just on coverity?


I've pushed it to gitlab and will adjust as needed.

Thanks!

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.