[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V7 2/4] x86/altp2m: Add hypercall to set a range of sve bits
On 08.01.2020 15:08, Alexandru Stefan ISAILA wrote: > By default the sve bits are not set. > This patch adds a new hypercall, xc_altp2m_set_supress_ve_multi(), > to set a range of sve bits. > The core function, p2m_set_suppress_ve_multi(), does not break in case > of a error and it is doing a best effort for setting the bits in the > given range. A check for continuation is made in order to have > preemption on large ranges. > The gfn of the first error is stored in > xen_hvm_altp2m_suppress_ve_multi.first_error and the error code is > stored in xen_hvm_altp2m_suppress_ve_multi.first_error_code. > If no error occurred the values will be 0. These last two sentences must have been stale for a while. IOW ... > Changes since V6: > - Fix commit message ... has this really happened? > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -3030,45 +3030,87 @@ out: > */ > int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve, > unsigned int altp2m_idx) > +{ > + int rc; > + struct xen_hvm_altp2m_suppress_ve_multi sve = {0}; > + > + sve.view = altp2m_idx; > + sve.suppress_ve = suppress_ve; > + sve.first_gfn = gfn_x(gfn); > + sve.last_gfn = gfn_x(gfn); Can't all of these move into the initializer, instead of the somewhat odd 0? > + if ( !(rc = p2m_set_suppress_ve_multi(d, &sve)) && sve.first_error ) > + rc = sve.first_error; Why the right side of the && ? > +/* > + * Set/clear the #VE suppress bit for multiple pages. Only available on VMX. > + */ > +int p2m_set_suppress_ve_multi(struct domain *d, > + struct xen_hvm_altp2m_suppress_ve_multi *sve) > { > struct p2m_domain *host_p2m = p2m_get_hostp2m(d); > struct p2m_domain *ap2m = NULL; > - struct p2m_domain *p2m; > - mfn_t mfn; > - p2m_access_t a; > - p2m_type_t t; > - int rc; > + struct p2m_domain *p2m = host_p2m; > + uint64_t start = sve->first_gfn; > + int rc = 0; > > - if ( altp2m_idx > 0 ) > + if ( sve->view > 0 ) > { > - if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) || > - d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] == > + if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) || > + d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] == > mfn_x(INVALID_MFN) ) > return -EINVAL; > > - p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, > + p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(sve->view, > ARRAY_SIZE(d->arch.altp2m_p2m))]; > } > - else > - p2m = host_p2m; > > - gfn_lock(host_p2m, gfn, 0); > + p2m_lock(host_p2m); > > if ( ap2m ) > p2m_lock(ap2m); > > - rc = altp2m_get_effective_entry(p2m, gfn, &mfn, &t, &a, AP2MGET_query); > + while ( sve->last_gfn >= start ) > + { > + p2m_access_t a; > + p2m_type_t t; > + mfn_t mfn; > + int err = 0; > > - if ( rc ) > - goto out; > + if ( (err = altp2m_get_effective_entry(p2m, _gfn(start), &mfn, &t, > &a, > + AP2MGET_query)) && > + !sve->first_error ) > + { > + sve->first_error_gfn = start; /* Save the gfn of the first error > */ > + sve->first_error = err; /* Save the first error code */ > + } > > - rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, t, a, suppress_ve); > + if ( !err && (err = p2m->set_entry(p2m, _gfn(start), mfn, > + PAGE_ORDER_4K, t, a, > + sve->suppress_ve)) && > + !sve->first_error ) > + { > + sve->first_error_gfn = start; /* Save the gfn of the first error > */ > + sve->first_error = err; /* Save the first error code */ > + } I think it would help readability if you didn't do this error saving twice. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |