From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v1 04/10] xen/pt: Use xen_host_pci_get_[byte, word, long] instead of xen_host_pci_get_long Date: Fri, 17 Jul 2015 16:59:38 +0100 Message-ID: References: <1435866681-18468-1-git-send-email-konrad.wilk@oracle.com> <1435866681-18468-5-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.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZG847-0002VB-0W for xen-devel@lists.xenproject.org; Fri, 17 Jul 2015 16:00:47 +0000 In-Reply-To: <1435866681-18468-5-git-send-email-konrad.wilk@oracle.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: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, qemu-devel@nongnu.org, JBeulich@suse.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Thu, 2 Jul 2015, Konrad Rzeszutek Wilk wrote: > Otherwise we get: > > xen_pt_config_reg_init: Offset 0x0004 mismatch! Emulated=0x0000, host=0x2300017, syncing to 0x2300014. > xen_pt_config_reg_init: Error: Offset 0x0004:0x2300014 expands past register size(2)! > > which is not surprising. We read the value as an 32-bit (from host), > then operate it as a 16-bit - and the remainder is left unchanged. > > We end up writting the value as 16-bit (so 0014) to dev.config ^ writing > (as we use proper xen_set_host_[byte,word,long] so we don't spill > to other registers) but in XenPTReg->data it is as 32-bit (0x2300014)! > > It is harmless as the read/write functions end up using an size mask > and never modify the bits past 16-bit (reg->size is 2). > > This patch fixes the warnings by reading the value using the > proper size. > > Note that the check for size is still left in-case the developer > sets bits past the reg->size in the ->init routines. > > Signed-off-by: Konrad Rzeszutek Wilk > --- > hw/xen/xen_pt_config_init.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > index 09309ba..e597993 100644 > --- a/hw/xen/xen_pt_config_init.c > +++ b/hw/xen/xen_pt_config_init.c > @@ -1876,7 +1876,12 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s, > offset = reg_grp->base_offset + reg->offset; > size_mask = 0xFFFFFFFF >> ((4 - reg->size) << 3); > > - rc = xen_host_pci_get_long(&s->real_device, offset, &val); > + switch (reg->size) { > + case 1: rc = xen_host_pci_get_byte(&s->real_device, offset, (uint8_t *)&val);break; > + case 2: rc = xen_host_pci_get_word(&s->real_device, offset, (uint16_t *)&val);break; > + case 4: rc = xen_host_pci_get_long(&s->real_device, offset, &val);break; > + default: assert(1); > + } > if (rc) { > /* Serious issues when we cannot read the host values! */ > g_free(reg_entry); Please merge this patch with patch #2/10. It is best to place the break statements on new lines, to follow QEMU's code style.