From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58299) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QWSHd-0004DZ-Ik for qemu-devel@nongnu.org; Tue, 14 Jun 2011 07:59:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QWSHb-0005DU-La for qemu-devel@nongnu.org; Tue, 14 Jun 2011 07:59:49 -0400 Received: from goliath.siemens.de ([192.35.17.28]:26455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QWSHb-0005C9-4J for qemu-devel@nongnu.org; Tue, 14 Jun 2011 07:59:47 -0400 Message-ID: <4DF74D24.9000808@siemens.com> Date: Tue, 14 Jun 2011 13:59:32 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1307116614-11775-1-git-send-email-stefano.stabellini@eu.citrix.com> <4DF7403E.6000503@siemens.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] xen: avoid tracking the region 0xa0000 - 0xbffff List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: Anthony Perard , "xen-devel@lists.xensource.com" , Alexander Graf , "qemu-devel@nongnu.org" On 2011-06-14 13:50, Stefano Stabellini wrote: > On Tue, 14 Jun 2011, Jan Kiszka wrote: >> On 2011-06-14 12:54, Alexander Graf wrote: >>> >>> On 03.06.2011, at 17:56, wrote: >>> >>>> From: Stefano Stabellini >>>> >>>> Xen can only do dirty bit tracking for one memory region, so we should >>>> explicitly avoid trying to track the legacy VGA region between 0xa0000 >>>> and 0xbffff, rather than trying and failing. >>>> >>>> Signed-off-by: Stefano Stabellini >>>> --- >>>> xen-all.c | 4 ++++ >>>> 1 files changed, 4 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/xen-all.c b/xen-all.c >>>> index 9a5c3ec..1fdc2e8 100644 >>>> --- a/xen-all.c >>>> +++ b/xen-all.c >>>> @@ -218,6 +218,10 @@ static int xen_add_to_physmap(XenIOState *state, >>>> if (get_physmapping(state, start_addr, size)) { >>>> return 0; >>>> } >>>> + /* do not try to map legacy VGA memory */ >>>> + if (start_addr >= 0xa0000 && start_addr + size <= 0xbffff) { >>> >>> I don't quite like the hardcoded range here. What exactly is the issue? The fact that you can only map a single region? Then do a counter and fail when it's > 1. If you don't want to map the VGA region as memory slot, why not change the actual mapping code in the cirrus adapter? >> >> Err, please no "if (xen_enabled())" in that code. We just got rid of the >> kvm_enabled() mess. And it doesn't scale, it would be required in e1000 >> as well e.g. > > agreed [Actually, e1000 is not using dirty logging but coalesced MMIO.] > > >> BTW, if Xen is not able to track more than one dirty region, I think >> it's time to fix that limitation. At some point it may no longer be >> possible to work around it (who knows how the new memory API will look >> like in this regard). > > you are right, however it is not a simple fix and at present we don't > actually need to track more than one region... Well, you already miss dirty logged VGA/VBE memory access this way (everything that goes to legacy VGA mem, not the framebuffer BAR). Grub provides a really poor use experience in that mode. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux