From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH V4 4/5] xen, libxc: Request page fault injection via libxc Date: Tue, 09 Sep 2014 10:08:12 +0100 Message-ID: <540EDF9C02000078000326CA@mail.emea.novell.com> References: <1409911297-3360-1-git-send-email-rcojocaru@bitdefender.com> <1409911297-3360-5-git-send-email-rcojocaru@bitdefender.com> <540DEB9702000078000322A5@mail.emea.novell.com> <540DD793.9060101@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XRHPO-0000Hk-Rk for xen-devel@lists.xenproject.org; Tue, 09 Sep 2014 09:08:18 +0000 In-Reply-To: <540DD793.9060101@bitdefender.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Razvan Cojocaru Cc: kevin.tian@intel.com, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, tim@xen.org, jun.nakajima@intel.com, xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org >>> On 08.09.14 at 18:21, wrote: > On 09/08/14 18:47, Jan Beulich wrote: >>>>> On 05.09.14 at 12:01, wrote: >>> + { >>> + hvm_inject_trap(&d->arch.hvm_domain.inject_trap); >>> + d->arch.hvm_domain.inject_trap.vector = -1; >>> + } >> >> And this is clearly lacking serialization (or a comment saying why >> serialization isn't needed here). > > If I understood this correctly, this is what Kevin has recommended: that > only one HVMOP_trap_request should be pending at a time. Or have I > misunderstood your comment? Indeed you have - the comment is about dealing with races of e.g. two CPUs processing a request for trap injection at the same time. >>> --- a/xen/include/public/hvm/hvm_op.h >>> +++ b/xen/include/public/hvm/hvm_op.h >>> @@ -197,6 +197,13 @@ struct xen_hvm_inject_trap { >>> uint32_t insn_len; >>> /* CR2 for page faults */ >>> uint64_aligned_t cr2; >>> + /* >>> + * Only used if vcpuid == ~0 (wildcard for any VCPU). >>> + * In that case, injection data is set per-domain, and any VCPU >>> + * running a process with matching CR3 in user mode will inject >>> + * the trap. >>> + */ >>> + uint64_aligned_t cr3; >> >> The comment should say "Currently only used ...", and the code >> should then check this (returning the usual -EOPNOTSUPP). Or >> alternatively implement it right away (which may be the better >> route taking into consideration the first of the comments above). > > I will gladly change the comment, but I'm not sure for what CR3 value I > should return -EOPNOTSUPP. Should I enforce that cr3 == 0 for the > VCPU-specific case and for traps that are not page faults? Could you > please elaborate on what the alternative is? First of all you need a value (or separate flag) indicating that the CR3 check is to not be done. The current lack thereof shows (once again) that you continue to think just in terms of your use case, not in terms of a generic injection model. Once such a flag or special value is in place, the answer to the above is - I think - obvious: For other than #PF, it may be desirable (but not strictly necessary) to enforce the value to mean "don't care"; just do whatever currently is done for CR2. For #PF, the special value (or flag) used to suppress CR3 checking in the domain case should (for now) get enforced for the vCPU case. As to flag or value: Since e.g. ~0 is never valid, this would seem preferable over 0. And as long as there is one definitely invalid value, not introducing a separate qualifying flag would seem the preferred route to me. Jan