From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Bligh Subject: Re: [qemu-upstream-4.2-testing test] 16779: regressions - FAIL Date: Mon, 04 Mar 2013 15:08:05 +0000 Message-ID: References: <20784.54292.926802.900837@mariner.uk.xensource.com> <348E65CAC53021DF1562094C@nimrod.local> Reply-To: Alex Bligh Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline 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: Anthony Perard , xen-devel@lists.xensource.com, Ian Jackson , Alex Bligh , Jan Beulich List-Id: xen-devel@lists.xenproject.org Stefano, > It could be any of the last 6 commits: I would start by bisecting the > problem further until you manage to pinpoint the precise backport that > caused it. My problem is trying to replicate it... > But like you say, it is probably the last one. > Consider that the changes introduced by the last commit are only useful > for live-migration, you can try to remove one by one to figure out > exactly what's breaking Windows install. > My guess would be this chunk: Me too. This was the bit that I couldn't actually work out what it was doing and why. In the original patch (below) it called memory_region_set_dirty, which doesn't exist in 4.2, with the parameter 'framebuffer', which also doesn't exist in 4.2. This only gets called if xc_hvm_track_dirty_vram returns an error other than ENODATA. I had presumed the intent was to mark the whole framebuffer as dirty if xc_hvm_track_dirty_vram failed this way. Does the loop start (and end) need to start from vram_offset (i.e. physmap->phys_offset), rather than start_addr? I suspect my confusion is that both start_addr and vram_offset are target_phys_addr_t but one is one side of a mapping and one the other. > > @@ -470,7 +470,21 @@ static int xen_sync_dirty_bitmap(XenIOState *state, > rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, > start_addr >> TARGET_PAGE_BITS, npages, > bitmap); > - if (rc) { > + if (rc < 0) { > + if (rc != -ENODATA) { > + ram_addr_t addr, end; > + > + xen_modified_memory(start_addr, size); > + > + end = TARGET_PAGE_ALIGN(start_addr + size); > + for (addr = start_addr & TARGET_PAGE_MASK; addr < end; addr > += TARGET_PAGE_SIZE) { + > cpu_physical_memory_set_dirty(addr); > + } > + > + DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx > + ", 0x" TARGET_FMT_plx "): %s\n", > + start_addr, start_addr + size, strerror(-rc)); > + } > return rc; > } > > in particular the loop around cpu_physical_memory_set_dirty > > Original patch: diff --git a/xen-all.c b/xen-all.c index b11542c..e6308be 100644 --- a/xen-all.c +++ b/xen-all.c @@ -507,7 +507,8 @@ static void xen_sync_dirty_bitmap(XenIOState *state, bitmap); if (rc < 0) { if (rc != -ENODATA) { - fprintf(stderr, "xen: track_dirty_vram failed (0x" TARGET_FMT_plx + memory_region_set_dirty(framebuffer, 0, size); + DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx ", 0x" TARGET_FMT_plx "): %s\n", start_addr, start_addr + size, strerror(-rc)); } -- Alex Bligh