From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH RFC V4 4/5] xen, libxc: Request page fault injection via libxc Date: Tue, 05 Aug 2014 11:48:43 +0300 Message-ID: <53E09A6B.4080500@bitdefender.com> References: <1407151825-3843-1-git-send-email-rcojocaru@bitdefender.com> <1407151825-3843-4-git-send-email-rcojocaru@bitdefender.com> <53DFB4270200007800029147@mail.emea.novell.com> <53DFA022.9000100@bitdefender.com> <53DFC0D30200007800029228@mail.emea.novell.com> <53E0913B.2070309@bitdefender.com> <53E0B46A0200007800029561@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53E0B46A0200007800029561@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, xen-devel@lists.xen.org, jun.nakajima@intel.com, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 08/05/2014 11:39 AM, Jan Beulich wrote: >>>> On 05.08.14 at 10:09, wrote: >> On 08/04/2014 06:20 PM, Jan Beulich wrote: >>>>>> On 04.08.14 at 17:00, wrote: >>>> On 08/04/2014 05:26 PM, Jan Beulich wrote: >>>>>>>> On 04.08.14 at 13:30, wrote: >>>>>> + __vmread(VM_ENTRY_INTR_INFO, &ev); >>>>>> + >>>>>> + if ( (ev & INTR_INFO_VALID_MASK) && >>>>>> + hvm_event_needs_reinjection((ev >> 8) & 7, ev & 0xff) ) >>>>> >>>>> Are there no manifest constants for all these plain numbers? >>>> >>>> If there are, vmx_vmcs_save() in vmx.c (line 416) doesn't use them. I've >>>> copied that part verbatim. >>> >>> And that's precisely the problem: As long as there's exactly one use >>> site, the need for manifest constants is questionable (i.e. largely >>> cosmetic). As soon as there are multiple places, connecting them >>> together is largely impossible without naming these numbers - only >>> that way you have a reasonable chance to find the clone of the >>> original should the original be found to need tweaking. >> >> I'll gladly add #defines for those magic constants, but could you please >> recommend names for them and a header (or at least, category of headers) >> to put them in, in the interest of minimizing the number of RFC versions >> for this series? > > Did you even make an attempt to locate a proper place, or > to check whether such constants already exist? Simply looking > for INTR_INFO_VALID_MASK would have turned up > > #define INTR_INFO_VECTOR_MASK 0xff /* 7:0 */ > #define INTR_INFO_INTR_TYPE_MASK 0x700 /* 10:8 */ > > i.e. all you need is there, you just need to make use of them > (and please also - perhaps in an initial cleanup patch - for the > original you cloned from). And just to avoid a needless further > intermediate round: While there is no #define for the shift count > used, you also don't need one if you make use of MASK_EXTR(). I did, obviously, but you can only turn up so much when grepping for "7" in a large, mostly undocumented codebase. Thanks for your quick answer, I'll use those and modify the original code as well. Thanks, Razvan Cojocaru