On 04.02.2020 14:06, Julien Grall wrote: > From: Julien Grall <jgr...@amazon.com> > > Unlike shadow_enable(), hap_enable() can only be called once during > domain creation and with the mode equal to mode equal to > PG_external | PG_translate | PG_refcounts. > > If it were called twice, then we might have something interesting > problem as the p2m tables would be re-allocated (and therefore all the > mappings would be lost). > > Add code to sanity check the mode and that the function is only called > once. Take the opportunity to an if checking that PG_translate is set. > > Signed-off-by: Julien Grall <jgr...@amazon.com>
Acked-by: Jan Beulich <jbeul...@suse.com> preferably with the duplicate words on the second line of the description dropped. > I keep the check != 0 because this is consistent with the rest of the > file. If we want to omit comparison against 0, then this should be in a > separate patches converting the file. I disagree, but not enough to make the ack dependent upon the adjustment. > --- a/xen/arch/x86/mm/hap/hap.c > +++ b/xen/arch/x86/mm/hap/hap.c > @@ -445,6 +445,13 @@ int hap_enable(struct domain *d, u32 mode) > unsigned int i; > int rv = 0; > > + if ( mode != (PG_external | PG_translate | PG_refcounts) ) > + return -EINVAL; > + > + /* The function can only be called once */ > + if ( d->arch.paging.mode != 0 ) > + return -EEXIST; I think if such a comment gets added, it should be unambiguous. The function can be called once per domain, not just once in total. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
