On 09/08/18 11:41, Paul Durrant wrote: >> -----Original Message----- >> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] >> Sent: 09 August 2018 11:32 >> To: Xen-devel <xen-de...@lists.xen.org> >> Cc: Andrew Cooper <andrew.coop...@citrix.com>; Jan Beulich >> <jbeul...@suse.com>; Paul Durrant <paul.durr...@citrix.com>; Stefano >> Stabellini <sstabell...@kernel.org>; Julien Grall <julien.gr...@arm.com>; Wei >> Liu <wei.l...@citrix.com>; Roger Pau Monne <roger....@citrix.com>; >> George Dunlap <george.dun...@citrix.com> >> Subject: [PATCH] common/gnttab: Explicitly default to gnttab v1 during >> domain creation >> >> For reasons which appear to be exclusively down to poor review of the grant >> table v2 code, a grant table's version field was wasn't initialised during >> creation. >> >> A number of problems (including XSAs) have occurred in the past trying >> trying >> to use a grant table which hasn't been properly set up, and various areas of >> the code cope with v0 by defaulting to v1. >> >> In particular, the toolstack using GNTTABOP_setup_table to be able to fill in >> the store/console grants has a side effect of switching to v1. >> >> In hindsight however, this "fixup if we see 0" is a very poor, with a >> substantial degree of risk. Explicitly default to grant table v1 during >> domain create, and let the rest of the code work safely in the knowledge >> that >> the version is sensibly set. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Paul Durrant <paul.durr...@citrix.com> >> CC: Stefano Stabellini <sstabell...@kernel.org> >> CC: Julien Grall <julien.gr...@arm.com> >> CC: Wei Liu <wei.l...@citrix.com> >> CC: Roger Pau Monné <roger....@citrix.com> >> CC: George Dunlap <george.dun...@eu.citrix.com> >> >> This probably wants backporting, so I chose not to integrate it into my >> series >> which rearranges large chunks of DOMCTL_createdomain >> --- >> xen/common/grant_table.c | 40 +++++++--------------------------------- >> 1 file changed, 7 insertions(+), 33 deletions(-) >> >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c >> index d9ec711..8bae656 100644 >> --- a/xen/common/grant_table.c >> +++ b/xen/common/grant_table.c >> @@ -49,10 +49,7 @@ struct grant_table { >> percpu_rwlock_t lock; >> /* Lock protecting the maptrack limit */ >> spinlock_t maptrack_lock; >> - /* >> - * The defined versions are 1 and 2. Set to 0 if we don't know >> - * what version to use yet. >> - */ > Given that only the guest can set the version by hypercall, it might be worth > comment somewhere saying that grant table always start in v1 format and then > get morphed if the guest sets v2. I guess it might be inferred that a v1 > table is immutable (for those who don't look too hard).
Really? I don't think that's a reasonable inference to make, given GNTTABOP_set_version as a public interface. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
