Patchwork [v3,1/6] Modify DIRTY_FLAG value and DIRTY_IDX introduce to use as indexes of bit-based phys_ram_dirty.

login
register
mail settings
Submitter Yoshiaki Tamura
Date April 19, 2010, 9:43 a.m.
Message ID <1271670198-12793-2-git-send-email-tamura.yoshiaki@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/50437/
State New
Headers show

Comments

Yoshiaki Tamura - April 19, 2010, 9:43 a.m.
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(-)
Avi Kivity - April 19, 2010, 10:15 a.m.
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.

> +
> +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.
Yoshiaki Tamura - April 19, 2010, 10:30 a.m.
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.

Patch

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];
+
+static inline int flag_to_idx(int flag)
+{
+    return ffs(flag) - 1;
+}
+
+static inline int idx_to_flag(int idx)
+{
+    return 1 << idx;
+}
 
 /* read dirty bit (return 0 or 1) */
 static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)