From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45755) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RXUow-0008UH-5H for qemu-devel@nongnu.org; Mon, 05 Dec 2011 04:26:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RXUou-0006Vl-Sd for qemu-devel@nongnu.org; Mon, 05 Dec 2011 04:26:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:19374) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RXUou-0006Vh-Ks for qemu-devel@nongnu.org; Mon, 05 Dec 2011 04:26:44 -0500 Message-ID: <4EDC8E50.9040803@redhat.com> Date: Mon, 05 Dec 2011 11:26:40 +0200 From: Avi Kivity MIME-Version: 1.0 References: <4EDB64DB.6080904@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] sub-page-sized mmio regions and address passed to read/write fns List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers On 12/04/2011 11:15 PM, Peter Maydell wrote: > On 4 December 2011 12:17, Avi Kivity wrote: > > On 12/02/2011 04:49 PM, Peter Maydell wrote: > >> However what I found is that the addresses passed to the read/write > >> functions aren't what I would expect. For instance if the board > >> maps the container at address 0x1e000000, then a read from 0x1e000100 > >> goes to the functions given by a9_gic_cpu_ops, as it should. However, > >> the offset parameter that the read function is passed is not 0x0 > >> (offset from the start of the a9mp-gic-cpu region) but 0x100 (offset > >> from the start of the page, I think). > >> > >> Is this expected behaviour? I certainly wasn't expecting it... > > > > A while ago this was the behaviour across the board. Then 8da3ff1809747 > > changed addresses to be relative, but apparently missed the subpage case. > > Having looked a bit more closely at the code I think this is what > the comment at the top of cpu_register_physical_memory_log() is > referring to: > > # Both start_addr and region_offset are rounded down to a page boundary > # before calculating this offset. This should not be a problem unless > # the low bits of start_addr and region_offset differ. > > In the case of a subregion at a non-page-aligned-address the > start_addr is not page aligned, but the region_offset is zero, > in the usual case, so we have differing low bits. Not an issue in the subpage code. As long as you extract the mmio index before adding region_offset, you're fine (as the mmio index resides in the low order bits). > >> I looked through the code that's getting called for reads, and > >> it looks to me like exec.c:subpage_readlen() is causing this. > >> We look up the subpage_t based on the address within the page, > >> but we don't then adjust the address we pass to io_mem_read > >> (except by region_offset, which I take from the comment at the > >> top of cpu_register_physical_memory_log() to be for something > >> else.) > > > I think you can use subpage_t's region_offset array for this (adding > > into it, of course, so the original value remains). > > Yes. I think the correction has to be calculated and applied in > cpu_register_physical_memory_log() -- for a region which starts > at a non-page-aligned address and extends over more than a page > the correcting offset needs to be applied for the whole region, > not just the first partial page. In that case we have to use subpages for full pages. But better to just assert() that this never happens for now. -- error compiling committee.c: too many arguments to function