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

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 08/03/2016 07:57 PM, Julien Grall wrote:
> 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.
>

It will be considered in the next 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.
>

Will not be considered if we decide to drop the patch #03.

>> +
>> +    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.
>

Ok, thank you. I will move this part to altp2m_init.

>>
>>      /*
>>       * 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)
>>
>
>

Best regards,
~Sergej



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