From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1KxKE1-0004fR-Ii
	for qemu-devel@nongnu.org; Tue, 04 Nov 2008 06:37:33 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1KxKE0-0004eh-IG
	for qemu-devel@nongnu.org; Tue, 04 Nov 2008 06:37:32 -0500
Received: from [199.232.76.173] (port=50950 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1KxKE0-0004ed-C3
	for qemu-devel@nongnu.org; Tue, 04 Nov 2008 06:37:32 -0500
Received: from smtp.eu.citrix.com ([62.200.22.115]:51436)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <Stefano.Stabellini@eu.citrix.com>)
	id 1KxKDz-0005Sy-Ug
	for qemu-devel@nongnu.org; Tue, 04 Nov 2008 06:37:32 -0500
Message-ID: <491034BC.2050806@eu.citrix.com>
Date: Tue, 04 Nov 2008 11:40:44 +0000
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] vga optmization
References: <20081103173111.GC30410@poweredge.glommer>
	<fb249edb0811040131r2991e2bwc0ec416162dfcee5@mail.gmail.com>
In-Reply-To: <fb249edb0811040131r2991e2bwc0ec416162dfcee5@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
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
Cc: Glauber Costa <glommer@redhat.com>, aliguori@us.ibm.com

andrzej zaborowski wrote:

> Hi,
> 
> 2008/11/3 Glauber Costa <glommer@redhat.com>:
> [...]
>> diff --git a/cpu-all.h b/cpu-all.h
>> index cdd79bc..9118f4d 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -46,6 +46,8 @@
>>
>>  #ifdef BSWAP_NEEDED
>>
>> +#include "kvm.h"
>> +
>>  static inline uint16_t tswap16(uint16_t s)
>>  {
>>     return bswap16(s);
>> @@ -909,17 +911,10 @@ int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
>>  #define KQEMU_DIRTY_FLAG     0x04
>>  #define MIGRATION_DIRTY_FLAG 0x08
>>
>> -/* read dirty bit (return 0 or 1) */
>> -static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
>> -{
>> -    return phys_ram_dirty[addr >> TARGET_PAGE_BITS] == 0xff;
>> -}
>> +int cpu_physical_memory_get_dirty(ram_addr_t addr, int dirty_flags);
>> +int cpu_physical_memory_is_dirty(ram_addr_t addr);
>>
>> -static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
>> -                                                int dirty_flags)
>> -{
>> -    return phys_ram_dirty[addr >> TARGET_PAGE_BITS] & dirty_flags;
>> -}
>> +void qemu_physical_sync_dirty_bitmap(ram_addr_t start_addr);
> 
> This will prevent the functions from being inlined even if KVM is
> disabled (e.g. on other archs) and I think it could be easily
> retained.
> 


I agree on this.

> void qemu_physical_sync_dirty_bitmap(ram_addr_t start_addr)
> {
>     if (kvm_enabled())
>         kvm_physical_sync_dirty_bitmap(start_addr);
> }
> 


Why don't you make qemu_physical_sync_dirty_bitmap take also and end
address, and you merge the two bitmaps in this address range in this
function, so you don't have to change cpu_physical_memory_get_dirty at
all?

I am saying to do something like:

void qemu_physical_sync_dirty_bitmap(ram_addr_t start_addr, ram_addr_t end_addr)
{
	/* sync and merge the two bitmaps between start_addr and end_addr */
}

then leave cpu_physical_memory_get_dirty untouched.
I would prefer this approch, it also leaves more space to other
optimizations.