Re: [Xen-devel] [PATCH 08/18] arm/altp2m: Add HVMOP_altp2m_destroy_p2m.

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 07/04/2016 06:32 PM, Julien Grall wrote:
> Hello Sergej,
>
> On 04/07/16 12:45, Sergej Proskurin wrote:
>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>> ---
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Cc: Julien Grall <julien.grall@xxxxxxx>
>> ---
>>   xen/arch/arm/hvm.c        |  2 +-
>>   xen/arch/arm/p2m.c        | 32 ++++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/p2m.h |  3 +++
>>   3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
>> index 005d7c6..f4ec5cf 100644
>> --- a/xen/arch/arm/hvm.c
>> +++ b/xen/arch/arm/hvm.c
>> @@ -145,7 +145,7 @@ static int
>> do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>>           break;
>>
>>       case HVMOP_altp2m_destroy_p2m:
>> -        rc = -EOPNOTSUPP;
>> +        rc = p2m_destroy_altp2m_by_id(d, a.u.view.view);
>>           break;
>>
>>       case HVMOP_altp2m_switch_p2m:
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 6c41b98..f82f1ea 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -2200,6 +2200,38 @@ void p2m_flush_altp2m(struct domain *d)
>>       altp2m_unlock(d);
>>   }
>>
>> +int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
>> +{
>> +    struct p2m_domain *p2m;
>> +    int rc = -EBUSY;
>> +
>> +    if ( !idx || idx >= MAX_ALTP2M )
>
> Can you please add a comment to explain why the altp2m at index 0
> cannot be destroyed.
>

This has been adopted from the x86 implementation. The altp2m[0] is
considered as hostp2m and is used as a safe harbor to which you can
switch as long as altp2m is active. After deactivating altp2m, the
system switches back to the original hostp2m view.That is, altp2m[0]
should only be destroyed/flushed/freed, when altp2m is deactivated.

>> +        return rc;
>> +
>> +    domain_pause_except_self(d);
>> +
>> +    altp2m_lock(d);
>> +
>> +    if ( d->arch.altp2m_vttbr[idx] != INVALID_MFN )
>> +    {
>> +        p2m = d->arch.altp2m_p2m[idx];
>> +
>> +        if ( !_atomic_read(p2m->active_vcpus) )
>> +        {
>> +            p2m_flush_table(p2m);
>> +            flush_tlb();
>
> Can you explain this call? flush_tlb is flushing TLBs for the current
> VMID only. However, d may not be equal to current when calling from
> HVM op.
>

You are correct. Please correct me if I am wrong, but I believe we even
do not need flushing at this point any more (see my response in patch
#06).Thank you.

>> +            d->arch.altp2m_vttbr[idx] = INVALID_MFN;
>> +            rc = 0;
>> +        }
>> +    }
>> +
>> +    altp2m_unlock(d);
>> +
>> +    domain_unpause_except_self(d);
>> +
>> +    return rc;
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>> index c51532a..255a282 100644
>> --- a/xen/include/asm-arm/p2m.h
>> +++ b/xen/include/asm-arm/p2m.h
>> @@ -140,6 +140,9 @@ int p2m_init_altp2m_by_id(struct domain *d,
>> unsigned int idx);
>>   /* Find an available alternate p2m and make it valid */
>>   int p2m_init_next_altp2m(struct domain *d, uint16_t *idx);
>>
>> +/* Make a specific alternate p2m invalid */
>> +int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx);
>> +
>>   #define p2m_is_foreign(_t)  ((_t) == p2m_map_foreign)
>>   #define p2m_is_ram(_t)      ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
>>
>>
>
> Regards,
>

~Sergej


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