[Qemu-devel] Re: [PATCH v3 1/6] Modify DIRTY_FLAG value and DIRTY_IDX introduce to use as indexes of bit-based phys_ram_dirty.

[Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>]

Avi Kivity wrote:
> On 04/19/2010 12:43 PM, Yoshiaki Tamura wrote:
>> It uses ffs() to convert DIRTY_FLAG to DIRTY_IDX.
>>
>> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>> ---
>> cpu-all.h | 30 ++++++++++++++++++++++++++----
>> 1 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/cpu-all.h b/cpu-all.h
>> index f8bfa66..8c2d678 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -37,6 +37,9 @@
>>
>> #include "softfloat.h"
>>
>> +/* to use ffs in flag_to_idx() */
>> +#include<strings.h>
>> +
>> #if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
>> #define BSWAP_NEEDED
>> #endif
>> @@ -853,7 +856,6 @@ target_phys_addr_t
>> cpu_get_phys_page_debug(CPUState *env, target_ulong addr);
>> /* memory API */
>>
>> extern int phys_ram_fd;
>> -extern uint8_t *phys_ram_dirty;
>> extern ram_addr_t ram_size;
>> extern ram_addr_t last_ram_offset;
>>
>> @@ -878,9 +880,29 @@ extern int mem_prealloc;
>> /* Set if TLB entry is an IO callback. */
>> #define TLB_MMIO (1<< 5)
>>
>> -#define VGA_DIRTY_FLAG 0x01
>> -#define CODE_DIRTY_FLAG 0x02
>> -#define MIGRATION_DIRTY_FLAG 0x08
>> +/* Use DIRTY_IDX as indexes of bit-based phys_ram_dirty. */
>> +#define MASTER_DIRTY_IDX 0
>> +#define VGA_DIRTY_IDX 1
>> +#define CODE_DIRTY_IDX 2
>> +#define MIGRATION_DIRTY_IDX 3
>> +#define NUM_DIRTY_IDX 4
>> +
>> +#define MASTER_DIRTY_FLAG (1<< MASTER_DIRTY_IDX)
>> +#define VGA_DIRTY_FLAG (1<< VGA_DIRTY_IDX)
>> +#define CODE_DIRTY_FLAG (1<< CODE_DIRTY_IDX)
>> +#define MIGRATION_DIRTY_FLAG (1<< MIGRATION_DIRTY_IDX)
>> +
>> +extern unsigned long *phys_ram_dirty[NUM_DIRTY_IDX];
>
> Please modify the bitmap definition in the patch where you start using
> it; this patch won't compile by itself, breaking bisection.

Sorry about that.
I would merge this with 2/6.

>
>> +
>> +static inline int flag_to_idx(int flag)
>> +{
>> + return ffs(flag) - 1;
>> +}
>> +
>> +static inline int idx_to_flag(int idx)
>> +{
>> + return 1<< idx;
>> +}
>>
>
> These should not be in global scope - it is not clear they refer to
> dirty bitmaps. Please rename or move to local scope.
>
> Alternatively, convert the callers to use *_DIRTY_IDX.

OK.  Let me rename it to, dirty_flag_to_idx() and dirty_idx_to_flag() respectively.