From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1L2qhr-0005Ud-Im
	for qemu-devel@nongnu.org; Wed, 19 Nov 2008 12:19:11 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1L2qhq-0005UR-AC
	for qemu-devel@nongnu.org; Wed, 19 Nov 2008 12:19:11 -0500
Received: from [199.232.76.173] (port=38777 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1L2qhq-0005UO-4d
	for qemu-devel@nongnu.org; Wed, 19 Nov 2008 12:19:10 -0500
Received: from mail-qy0-f20.google.com ([209.85.221.20]:58595)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <glommer@gmail.com>) id 1L2qhp-00017X-PQ
	for qemu-devel@nongnu.org; Wed, 19 Nov 2008 12:19:10 -0500
Received: by qyk13 with SMTP id 13so31665qyk.10
	for <qemu-devel@nongnu.org>; Wed, 19 Nov 2008 09:19:08 -0800 (PST)
Message-ID: <5d6222a80811190919m22e55172xdd6ab4bf6e514c7f@mail.gmail.com>
Date: Wed, 19 Nov 2008 15:19:08 -0200
From: "Glauber Costa" <glommer@gmail.com>
Subject: Re: [Qemu-devel] [PATCH 6/6] kvm: vga optimization.
In-Reply-To: <49242F84.2010809@codemonkey.ws>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <1227108377-8442-1-git-send-email-glommer@redhat.com>
	<1227108377-8442-2-git-send-email-glommer@redhat.com>
	<1227108377-8442-3-git-send-email-glommer@redhat.com>
	<1227108377-8442-4-git-send-email-glommer@redhat.com>
	<1227108377-8442-5-git-send-email-glommer@redhat.com>
	<1227108377-8442-6-git-send-email-glommer@redhat.com>
	<1227108377-8442-7-git-send-email-glommer@redhat.com>
	<49242F84.2010809@codemonkey.ws>
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

>> +    if (!s->map_addr)
>> +        return;
>> +
>> +    if (cirrus_lfb_is_mapped(s)) {
>> +        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000,
>> +                                    (s->vram_offset +
>> s->cirrus_bank_base[0]) | IO_MEM_RAM);
>> +        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000,
>> +                                    (s->vram_offset +
>> s->cirrus_bank_base[1]) | IO_MEM_RAM);
>>
>
> Isn't necessary to reregister 0xa0000 too?

ENOFOLLOW. This is exactly what I'm doing.
>
>> +        if (kvm_enabled()) {
>> +            kvm_log_start(0xa0000, 0x8000);
>> +            kvm_log_start(0xa8000, 0x8000);
>> +        }
>>
>
> Why would you enable logging on a different region from what you've
> registered?  Shouldn't you enable logging on both regions?  If we're going
> to enable logging based on target_phys_addr_t instead of ram_addr_t (and I
> think we should), then we should enable it on all target_phys_addr_ts.

Again, I don't follow. We map 0xa0000 and 0xa8000 to some ram_addr_t,
and then enable logging in the very 0xa0000 and 0xa8000. What's the problem
with that? One late nitpick, it is that for consistency, I registered
0xa0000 + isa_mem_base,
(usually 0), and should use it in kvm_log_start.

>> +    }
>> +    else {
>>
>
> This is formatted incorrectly.
>
>>  +
>>  /*
>>  * graphic modes
>>  */
>>
>
> More extra whitespace.
>
> Regards,
>
> Anthony Liguori
>
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."