From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6fEk-0004ax-2H for qemu-devel@nongnu.org; Wed, 09 Dec 2015 08:56:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6fEf-0002uG-2f for qemu-devel@nongnu.org; Wed, 09 Dec 2015 08:56:53 -0500 Received: from smtp.citrix.com ([66.165.176.89]:44968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6fEe-0002sy-UL for qemu-devel@nongnu.org; Wed, 09 Dec 2015 08:56:49 -0500 References: <1449141675.4424.125.camel@citrix.com> <1449141788-15084-1-git-send-email-ian.campbell@citrix.com> <1449141788-15084-5-git-send-email-ian.campbell@citrix.com> <1449668506.16124.239.camel@citrix.com> From: Andrew Cooper Message-ID: <5668331C.3040200@citrix.com> Date: Wed, 9 Dec 2015 13:56:44 +0000 MIME-Version: 1.0 In-Reply-To: <1449668506.16124.239.camel@citrix.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Xen-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Campbell , stefano.stabellini@eu.citrix.com Cc: ian.jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, xen-devel@lists.xen.org On 09/12/15 13:41, Ian Campbell wrote: > On Thu, 2015-12-03 at 11:23 +0000, Ian Campbell wrote: >> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c >> index 5e324ef..c96d974 100644 >> --- a/hw/display/xenfb.c >> +++ b/hw/display/xenfb.c >> @@ -104,9 +104,8 @@ static int common_bind(struct common *c) >> if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1) >> return -1; >> >> - c->page = xc_map_foreign_range(xen_xc, c->xendev.dom, >> - XC_PAGE_SIZE, >> - PROT_READ | PROT_WRITE, mfn); >> + c->page = xc_map_foreign_pages(xen_xc, c->xendev.dom, >> + PROT_READ | PROT_WRITE, &mfn, 1); > This doesn't build for i386 userspace, since mfn is a uint64_t but > xc_map_foreign_pages() wants a xen_pfn_t * (where xen_pfn_t == unsigned > long on x86). > > Until now that was just a truncation which was already checked for with: > > uint64_t mfn; > > if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1) > return -1; > assert(mfn == (xen_pfn_t)mfn); > > I think in principal passing "(xen_pfn_t *)&mfn" would ok (since it is a > singleton array in this case), but I was thinking of going a bit further > and: It is never ok to convert a pointer like this. In 32bit (little endian) userspace, it will leave the upper half of mfn uninitialised on the stack. ~Andrew