Re: [Qemu-devel] [PATCH 01/19] Add support for multi-level phys map.

[Aurelien Jarno]

On Mon, Mar 30, 2009 at 04:36:16PM +0200, Tristan Gingold wrote:
> As Alpha physical addresses are 44 bits, l1_phys_map can't be anymore 2 
> levels.
> Use a more generic multi-level approach and explain why we don't need to
> extend l1_map.
> 
> Signed-off-by: Tristan Gingold <address@hidden>
> ---
>  exec.c |   50 ++++++++++++++++++++++++++------------------------
>  1 files changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index df22c79..4b2ebc2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -147,15 +147,14 @@ typedef struct PhysPageDesc {
>  } PhysPageDesc;
>  
>  #define L2_BITS 10
> -#if defined(CONFIG_USER_ONLY) && defined(TARGET_VIRT_ADDR_SPACE_BITS)
> -/* XXX: this is a temporary hack for alpha target.
> - *      In the future, this is to be replaced by a multi-level table
> - *      to actually be able to handle the complete 64 bits address space.
> - */
> -#define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
> +
> +#define L1_BITS_ ((TARGET_PHYS_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % L2_BITS)
> +#if L1_BITS_ < 4 /* avoid ridiculous small l1 */
> +#define L1_BITS (L1_BITS_ + L2_BITS)
>  #else
> -#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
> +#define L1_BITS L1_BITS_
>  #endif
> +#define L1_SHIFT (TARGET_PHYS_ADDR_SPACE_BITS - TARGET_PAGE_BITS - L1_BITS)
>  
>  #define L1_SIZE (1 << L1_BITS)
>  #define L2_SIZE (1 << L2_BITS)
> @@ -165,7 +164,9 @@ unsigned long qemu_host_page_bits;
>  unsigned long qemu_host_page_size;
>  unsigned long qemu_host_page_mask;
>  
> -/* XXX: for system emulation, it could just be an array */
> +/* XXX: for system emulation, it could just be an array.  As this is 
> currently
> +   a two level map this limits the size of RAM memory that can contains
> +   target code.  In practice this is large enough (>= 4GB) */
>  static PageDesc *l1_map[L1_SIZE];
>  static PhysPageDesc **l1_phys_map;
>  
> @@ -338,25 +339,26 @@ static PhysPageDesc 
> *phys_page_find_alloc(target_phys_addr_t index, int alloc)
>  {
>      void **lp, **p;
>      PhysPageDesc *pd;
> +    int i;
>  
> +    /* Level 1.  */
>      p = (void **)l1_phys_map;
> -#if TARGET_PHYS_ADDR_SPACE_BITS > 32
> +    lp = p + ((index >> L1_SHIFT) & (L1_SIZE - 1));
>  
> -#if TARGET_PHYS_ADDR_SPACE_BITS > (32 + L1_BITS)
> -#error unsupported TARGET_PHYS_ADDR_SPACE_BITS
> -#endif
> -    lp = p + ((index >> (L1_BITS + L2_BITS)) & (L1_SIZE - 1));
> -    p = *lp;
> -    if (!p) {
> -        /* allocate if not found */
> -        if (!alloc)
> -            return NULL;
> -        p = qemu_vmalloc(sizeof(void *) * L1_SIZE);
> -        memset(p, 0, sizeof(void *) * L1_SIZE);
> -        *lp = p;
> +    /* Level 2..n-1 */
> +    for (i = (L1_SHIFT / L2_BITS) - 1; i > 0; i--) {
> +        p = *lp;
> +        if (!p) {
> +            /* allocate if not found */
> +            if (!alloc)
> +                return NULL;
> +            p = qemu_vmalloc(sizeof(void *) * L2_SIZE);
> +            memset(p, 0, sizeof(void *) * L2_SIZE);
> +            *lp = p;
> +        }
> +        lp = p + ((index >> (i * L2_BITS)) & (L2_SIZE - 1));
>      }
> -#endif
> -    lp = p + ((index >> L2_BITS) & (L1_SIZE - 1));
> +
>      pd = *lp;
>      if (!pd) {
>          int i;

While this part of the patch looks ok, it seems that some other parts of
exec.c needs to be modified to accommodate the new multi-level map. Is
some parts of the patch missing?

Also I think that we should remove L2_* into LN_* to show it is actually
a multilevel map.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net