From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v1 05/10] xen/pt: Remove XenPTReg->data field. Date: Fri, 17 Jul 2015 12:48:41 -0400 Message-ID: <20150717164841.GC19827__18665.1542237543$1437151799$gmane$org@l.oracle.com> References: <1435866681-18468-1-git-send-email-konrad.wilk@oracle.com> <1435866681-18468-6-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZG8oa-00077G-2K for xen-devel@lists.xenproject.org; Fri, 17 Jul 2015 16:48:48 +0000 Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: xen-devel@lists.xenproject.org, qemu-devel@nongnu.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On Fri, Jul 17, 2015 at 05:30:39PM +0100, Stefano Stabellini wrote: > On Thu, 2 Jul 2015, Konrad Rzeszutek Wilk wrote: > > We do not want to have two entries to cache the guest configuration > > registers: XenPTReg->data and dev.config. Instead we want to use > > only the dev.config. > > > > To do without much complications we rip out the ->data field > > and replace it with an pointer to the dev.config. This way we > > have the type-checking (uint8_t, uint16_t, etc) and as well > > and pre-computed location. > > > > Alternatively we could compute the offset in dev.config by > > using the XenPTRRegInfo and XenPTRegGroup every time but > > this way we have the pre-computed values. > > > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > hw/xen/xen_pt.h | 6 +++- > > hw/xen/xen_pt_config_init.c | 73 +++++++++++++++++++++++++++------------------ > > 2 files changed, 49 insertions(+), 30 deletions(-) > > > > diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h > > index 09358b1..586d055 100644 > > --- a/hw/xen/xen_pt.h > > +++ b/hw/xen/xen_pt.h > > @@ -134,7 +134,11 @@ struct XenPTRegInfo { > > struct XenPTReg { > > QLIST_ENTRY(XenPTReg) entries; > > XenPTRegInfo *reg; > > - uint32_t data; /* emulated value */ > > + union { > > + uint8_t *byte; > > + uint16_t *word; > > + uint32_t *dbword; > > + } ptr; /* pointer to dev.config. */ > > Nice (nasty?) trick. I would probably have just introduced a single > pointer, but this might turn out to be better as it avoids the risk of > merging bits that should be ignored. Right, and also adds nice type-casting checks. > > However it should be half-word (uint16_t) and word (uint32_t). OK.