Re: [Xen-devel] [PATCH v2 05/25] arm/altp2m: Rename and extend p2m_alloc_table.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

Title: s/altp2m/p2m/

On 01/08/16 18:10, Sergej Proskurin wrote:
> The initially named function "p2m_alloc_table" allocated pages solely
> required for the host's p2m. The new implementation leaves p2m
> allocation related parts inside of this function to generally initialize
> p2m/altp2m tables. This is done generically, as the host's p2m and
> altp2m tables are allocated similarly. Since this function will be used
> by the altp2m initialization routines, it is not made static. In
> addition, this commit provides the overlay function "p2m_table_init"
> that is used for the host's p2m allocation/initialization.
>
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
> v2: Removed altp2m table initialization from "p2m_table_init".
> ---
>  xen/arch/arm/p2m.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 17f3299..63fe3d9 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1277,11 +1277,11 @@ void guest_physmap_remove_page(struct domain *d,
>      p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
>  }
>
> -static int p2m_alloc_table(struct domain *d)
> +int p2m_alloc_table(struct p2m_domain *p2m)
The function p2m_alloc_table should not be exposed outside of the p2m. I 
explained it why in the previous patch.
>  {
> -    struct p2m_domain *p2m = &d->arch.p2m;
> -    struct page_info *page;
>      unsigned int i;
> +    struct page_info *page;
> +    struct vttbr *vttbr = &p2m->vttbr;
>
>      page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0);
>      if ( page == NULL )
> @@ -1293,11 +1293,28 @@ static int p2m_alloc_table(struct domain *d)
>
>      p2m->root = page;
>
> -    p2m->vttbr.vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) 
> << 48;
> +    /* Initialize the VTTBR associated with the allocated p2m table. */
> +    vttbr->vttbr = 0;
> +    vttbr->vmid = p2m->vmid & 0xff;
> +    vttbr->baddr = page_to_maddr(p2m->root);
This change does not belong to this patch. If we want to use VTTBR, it 
should be in patch #3.
> +
> +    return 0;
> +}
> +
> +static int p2m_table_init(struct domain *d)
> +{
> +    int rc;
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    rc = p2m_alloc_table(p2m);
> +    if ( rc != 0 )
> +        return -ENOMEM;
> +
> +    d->arch.altp2m_active = false;
Please avoid to spread the change for altp2m everywhere. The addition of 
altp2m in the p2m code should really be a single patch.
>      /*
>       * Make sure that all TLBs corresponding to the new VMID are flushed
> -     * before using it
> +     * before using it.
>       */
>      p2m_flush_tlb(p2m);
>
> @@ -1440,7 +1457,7 @@ static int p2m_init_hostp2m(struct domain *d)
>      if ( rc )
>          return rc;
>
> -    return p2m_alloc_table(d);
> +    return p2m_table_init(d);
>  }
>
>  int p2m_init(struct domain *d)
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel