From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH V7 for-4.5 4/4] xen: Handle resumed instruction based on previous mem_event reply Date: Thu, 11 Sep 2014 18:21:41 +0300 Message-ID: <5411BE05.5050007@bitdefender.com> References: <1410441334-10603-1-git-send-email-rcojocaru@bitdefender.com> <1410441334-10603-5-git-send-email-rcojocaru@bitdefender.com> <5411C15C0200007800033EDB@mail.emea.novell.com> <5411AB7F.8070004@bitdefender.com> <5411D5B20200007800034024@mail.emea.novell.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 1XS6Bm-00014h-Ct for xen-devel@lists.xenproject.org; Thu, 11 Sep 2014 15:21:38 +0000 Received: from smtp03.buh.bitdefender.org (smtp.bitdefender.biz [10.17.80.77]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 2B03680096 for ; Thu, 11 Sep 2014 18:21:35 +0300 (EEST) In-Reply-To: <5411D5B20200007800034024@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, 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 09/11/2014 06:02 PM, Jan Beulich wrote: >>>> On 11.09.14 at 16:02, wrote: >> On 09/11/2014 04:35 PM, Jan Beulich wrote: >>>>>> On 11.09.14 at 15:15, wrote: >>>> + if ( violation ) >>>> + v->arch.mem_event.emulate_flags = rsp.flags; >>> >>> ... a second time here (rather making this one simply a conditional >>> expression)? >> >> I'll assign to v->arch.mem_event.emulate_flags directly in the switch. > > I doubt that's going to result in better code. You're right, I thought I'd simplify by getting rid of "violation" but the code looks worse now. I'll remove the first assignment and modify the second. >>> And I further wonder whether all the MEM_EVENT_FLAG_* values are >>> really potentially useful in v->arch.mem_event.emulate_flags - right >>> now it rather looks like this field could be a simple bool_t (likely with >>> a different name), which would at once make the >>> hvm_mem_event_emulate_one() a little better readable. >> >> The value is checked here: >> >> + hvm_mem_event_emulate_one((v->arch.mem_event.emulate_flags & >> + MEM_EVENT_FLAG_EMULATE_NOWRITE) != 0, >> + TRAP_invalid_op, >> HVM_DELIVER_NO_ERROR_CODE); >> >> where it matters if MEM_EVENT_FLAG_EMULATE_NOWRITE is set. > > Right, and this would reduce by quite a bit if you could just pass the > boolean variable. Sorry, I don't quite follow that. Currently, v->arch.mem_event.emulate_flags serves both as a way to know if emulation is required (0 if no emulation is required, and != 0 if emulation is required), and as a way to know which kind of emulation is required ("regular" emulation, or emulation with writes disabled). Making it into a bool_t would make it possible to know that emulation is required, but not which kind of emulation: + if ( v->arch.mem_event.emulate_flags ) + { + hvm_mem_event_emulate_one((v->arch.mem_event.emulate_flags & + MEM_EVENT_FLAG_EMULATE_NOWRITE) != 0, + TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE); + + v->arch.mem_event.emulate_flags = 0; + return 1; + } So even in a scenario where we don't care about MEM_EVENT_FLAG_SKIP, we still care about two different types response flags: MEM_EVENT_FLAG_EMULATE for "regular" emulation and MEM_EVENT_FLAG_EMULATE_NOWRITE that emulates with the dummy write handlers (or-ed together for an "emulate no write" response). Thanks, Razvan Cojocaru