Re: [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

From: Sergej Proskurin <proskurin@sec.in.tum.de>
To: Julien Grall <julien.grall@arm.com>, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.
Date: Mon, 4 Jul 2016 15:51:42 +0200	[thread overview]
Message-ID: <2733d404-bb63-bcc7-a6b9-1eaa293b677b@sec.in.tum.de> (raw)
In-Reply-To: <577A6673.10209@arm.com>

Hello Julien,

On 07/04/2016 03:36 PM, Julien Grall wrote:
> Hello Sergej,
> 
> On 04/07/16 12:45, Sergej Proskurin wrote:
>> This commit moves the altp2m-related code from x86 to ARM.
> 
> Looking at the code in the follow-up patches, I have the impression that
> the code is very similar (if not exactly) to the x86 code. If so, we
> should move the HVMOP for altp2m in common code rather than duplicating
> the code.
> 

You are correct: A big part of the code is very similar to the x86
implementation. We are already started thinking about which parts need
to be pulled out into a common place. Thank you.

>> Functions
>> that are no yet supported notify the caller or print a BUG message
>> stating their absence.
>>
>> Also, the struct arch_domain is extended with the altp2m_active
>> attribute, represeting the current altp2m activity configuration of the
> 
> s/represeting/representing/
> 

Ok.

>> domain.
>>
>> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
>> ---
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/hvm.c           | 82
>> ++++++++++++++++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/altp2m.h | 22 ++++++++++--
>>   xen/include/asm-arm/domain.h |  3 ++
>>   3 files changed, 105 insertions(+), 2 deletions(-)
> 
> [..]
> 
>> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
>> index a87747a..16ae9d6 100644
>> --- a/xen/include/asm-arm/altp2m.h
>> +++ b/xen/include/asm-arm/altp2m.h
>> @@ -2,6 +2,7 @@
>>    * Alternate p2m
>>    *
>>    * Copyright (c) 2014, Intel Corporation.
>> + * Copyright (c) 2016, Sergej Proskurin <proskurin@sec.in.tum.de>.
>>    *
>>    * This program is free software; you can redistribute it and/or
>> modify it
>>    * under the terms and conditions of the GNU General Public License,
>> @@ -24,8 +25,7 @@
>>   /* Alternate p2m on/off per domain */
>>   static inline bool_t altp2m_active(const struct domain *d)
>>   {
>> -    /* Not implemented on ARM. */
>> -    return 0;
>> +    return d->arch.altp2m_active;
>>   }
>>
>>   /* Alternate p2m VCPU */
>> @@ -36,4 +36,22 @@ static inline uint16_t altp2m_vcpu_idx(const struct
>> vcpu *v)
>>       return 0;
>>   }
>>
>> +static inline void altp2m_vcpu_initialise(struct vcpu *v)
>> +{
>> +    /* Not implemented on ARM, should not be reached. */
>> +    BUG();
>> +}
>> +
>> +static inline void altp2m_vcpu_destroy(struct vcpu *v)
>> +{
>> +    /* Not implemented on ARM, should not be reached. */
>> +    BUG();
>> +}
>> +
>> +static inline void altp2m_vcpu_reset(struct vcpu *v)
>> +{
>> +    /* Not implemented on ARM, should not be reached. */
>> +    BUG();
>> +}
> 
> Those 3 helpers are not used by anyone in the code so far and you
> replace them by another implementation in patch #5.
> 
> So I would prefer if you introduce the helpers and implementation only
> when they will be used.
> 

Will do. Thank you.

>>   #endif /* __ASM_ARM_ALTP2M_H */
>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index 979f7de..2039f16 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -127,6 +127,9 @@ struct arch_domain
>>       paddr_t efi_acpi_gpa;
>>       paddr_t efi_acpi_len;
>>   #endif
>> +
>> +    /* altp2m: allow multiple copies of host p2m */
>> +    bool_t altp2m_active;
>>   }  __cacheline_aligned;
>>
>>   struct arch_vcpu
>>
> 
> Regards,
> 

Thank you.

Best regards,
Sergej

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