[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V2 4/5] xen, libxc: Request page fault injection via libxc
> From: Razvan Cojocaru [mailto:rcojocaru@xxxxxxxxxxxxxxx] > Sent: Thursday, September 04, 2014 9:57 AM > > On 09/04/14 18:58, Tian, Kevin wrote: > >> From: Razvan Cojocaru [mailto:rcojocaru@xxxxxxxxxxxxxxx] > >> Sent: Thursday, September 04, 2014 1:02 AM > >> > >> On 09/04/2014 10:54 AM, Razvan Cojocaru wrote: > >>> On 09/04/2014 04:25 AM, Tian, Kevin wrote: > >>>>> From: Razvan Cojocaru [mailto:rcojocaru@xxxxxxxxxxxxxxx] > >>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > >>>>> index 5761ff9..5d3e4d4 100644 > >>>>> --- a/xen/arch/x86/hvm/hvm.c > >>>>> +++ b/xen/arch/x86/hvm/hvm.c > >>>>> @@ -420,6 +420,31 @@ static bool_t hvm_wait_for_io(struct > >>>>> hvm_ioreq_vcpu *sv, ioreq_t *p) > >>>>> return 1; > >>>>> } > >>>>> > >>>>> +static bool_t hvm_is_pf_requested(struct vcpu *v) > >>>>> +{ > >>>>> + const struct domain *d = v->domain; > >>>>> + struct segment_register seg; > >>>>> + uint64_t mask; > >>>>> + > >>>>> + hvm_get_segment_register(v, x86_seg_ss, &seg); > >>>>> + > >>>>> + if ( seg.attr.fields.dpl != 3 ) /* Guest is not in user mode */ > >>>>> + return 0; > >>>>> + > >>>>> + if ( hvm_long_mode_enabled(v) ) > >>>>> + mask = PADDR_MASK & PAGE_MASK; /* Bits 51:12. */ > >>>>> + else if ( hvm_pae_enabled(v) ) > >>>>> + mask = 0x00000000ffffffe0; /* Bits 31:5. */ > >>>>> + else > >>>>> + mask = (uint32_t)PAGE_MASK; /* Bits 31:12. */ > >>>>> + > >>>>> + if ( (v->arch.hvm_vcpu.guest_cr[3] & mask) != > >>>>> + (d->arch.hvm_domain.inject_trap.cr3 & mask) ) > >>>>> + return 0; > >>>>> + > >>>>> + return 1; > >>>>> +} > >>>>> + > >>>>> void hvm_do_resume(struct vcpu *v) > >>>>> { > >>>>> struct domain *d = v->domain; > >>>>> @@ -451,6 +476,15 @@ void hvm_do_resume(struct vcpu *v) > >>>>> } > >>>>> > >>>>> /* Inject pending hw/sw trap */ > >>>> > >>>> you want to make comment more introspection related. > >>>> > >>>>> + if ( d->arch.hvm_domain.inject_trap.vector != -1 && > >>>>> + v->arch.hvm_vcpu.inject_trap.vector == -1 && > >>>>> + hvm_is_pf_requested(v) ) > >>>>> + { > >>>>> + hvm_inject_trap(&d->arch.hvm_domain.inject_trap); > >>>>> + d->arch.hvm_domain.inject_trap.vector = -1; > >>>>> + } > >>>>> + > >>>>> + /* Inject pending hw/sw trap */ > >>>>> if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ) > >>>>> { > >>>>> hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap); > >>>>> @@ -1473,9 +1507,10 @@ int hvm_domain_initialise(struct domain > *d) > >>>>> printk(XENLOG_G_INFO "PVH guest must have HAP > >> on\n"); > >>>>> return -EINVAL; > >>>>> } > >>>>> - > >>>>> } > >>>>> > >>>>> + d->arch.hvm_domain.inject_trap.vector = -1; > >>>>> + > >>>> > >>>> A question here. With new design, now you have two places which may > >> cache > >>>> fault injection information. If unfortunately two consecutive hypercalls > with > >>>> one having vcpuid=-1 and the other having vcpuid=n, it's possible two > >> injections > >>>> will be handled together if same vcpu is chosen for two pending > injections. > >>>> > >>>> I think the hypercall needs a check of all previous pending requests, not > >> only > >>>> in domain specific structure as what you did in this version. > >> > >> If two consecutive hypercalls with a wildcard, and then a specific > >> vcpuid take place, the per-domain injection data will indeed be set > >> along with the VCPU-specific injection data. > > > > I'm thinking one hypercall with a wildchard, while another hypercall > > with a specific vcpuid. > > If I understand this correctly, that's what my original design did, with > xc_domain_request_pagefault(). Please see: > > http://lists.xen.org/archives/html/xen-devel/2014-08/msg02782.html > > But it has been argued that Xen already has HVMOP_inject_trap and that a > new hypercall should only be added if it's not possible to extend the > existing one. > > I have no problem at all reverting the patch back to that implementation > if the consensus becomes that that design is preferable. seems I didn't make clear. I meant two consecutive HVMOP_inject_trap invocations from user space, with one expecting wildcard while the other specifying a vcpuid. Currently logic allows both pending. > > >> It's difficult to check for this case beforehand, because at the time of > >> the per-domain hypercall we don't know which VCPU will end up having to > >> do the work of injecting the trap. > > > > If the policy is simply new injection needs get EBUSY as long as a valid > > earlier injection request is pending, then you need scan both domain > > structure and all vcpu structures. Currently you only do that for domain > > structure. I just want to see a consistent policy. :-) > > I'll run some tests tomorrow and switch to that policy for the next > version of the series if it works, and if you don't decide that the > original design (xc_domain_request_pagefault()) is more appropriate > here. Please let me know. > > > Thanks, > Razvan Cojocaru _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |