[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU
On Tue, Jul 31, 2018 at 5:53 AM Jan Beulich <JBeulich@xxxxxxxx> wrote: > > >>> On 25.07.18 at 13:49, <apop@xxxxxxxxxxxxxxx> wrote: > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -4467,6 +4467,30 @@ static int hvmop_get_param( > > return rc; > > } > > > > +/* > > + * Find the struct vcpu given a dom_id and vcpu_id. > > + * Return NULL if not found. > > + */ > > +static struct vcpu *__get_vcpu(domid_t domain_id, uint32_t vcpu_id) > > No double leading underscores please, and avoid the use of fixed > width types when you don't really need it (unsigned int is fine here). > > > +{ > > + struct domain *dom; > > We commonly call this just d. > > > + struct vcpu *v; > > + > > + dom = rcu_lock_domain_by_id(domain_id); > > + > > + for_each_vcpu( dom, v ) > > + { > > + if ( vcpu_id == v->vcpu_id ) > > + { > > + rcu_unlock_domain(dom); > > + return v; > > + } > > + } > > for_each_vcpu() looks excessive here - all you need is a bounds > check and an access into d->vcpus[]. Together with the fact > that your caller has already identified and locked d I wonder > whether this helper is needed in the first place. > > > @@ -4576,26 +4599,32 @@ static int do_altp2m_op( > > > > case HVMOP_altp2m_vcpu_enable_notify: > > { > > - struct vcpu *curr = current; > > + struct vcpu *v; > > p2m_type_t p2mt; > > > > - if ( a.u.enable_notify.pad || a.domain != DOMID_SELF || > > - a.u.enable_notify.vcpu_id != curr->vcpu_id ) > > + if ( a.u.enable_notify.pad ) > > { > > rc = -EINVAL; > > break; > > } > > > > - if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) || > > - mfn_eq(get_gfn_query_unlocked(curr->domain, > > + v = __get_vcpu(a.domain, a.u.enable_notify.vcpu_id); > > + if ( !v ) > > + { > > + rc = -EFAULT; > > Hardly an appropriate error indicator for the condition. > > > + break; > > + } > > + > > + if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) || > > + mfn_eq(get_gfn_query_unlocked(v->domain, > > a.u.enable_notify.gfn, &p2mt), INVALID_MFN) ) > > { > > rc = -EINVAL; > > break; > > } > > > > - vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn); > > - altp2m_vcpu_update_vmfunc_ve(curr); > > + vcpu_altp2m(v).veinfo_gfn = _gfn(a.u.enable_notify.gfn); > > + altp2m_vcpu_update_vmfunc_ve(v); > > I'd like you to outline in the description how you mean an external > agent to coordinate the use of this GFN with the guest (and in > particular without in-guest agent). Using #VE without an in-guest agent isn't really possible so this is really just about designating the page from dom0 instead of doing it with the in-guest agent. Something has to be present in the guest to handle the new interrupts coming from #VE after all. I could image a number of ways to coordinate that dom0 agent now setting this page with the guest, my immediate guess is that it would be done through the vm_event guest request channel. But it could also be done through xenstore or even ssh. Tamas _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |