[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]

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<address@hidden>
> > ---
> > 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.