[PATCH] arm/domain: fix comment for arch_set_info_guest

Luca Fancellu posted 1 patch 1 year, 8 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220725144634.8086-1-luca.fancellu@arm.com
There is a newer version of this series
xen/arch/arm/domain.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] arm/domain: fix comment for arch_set_info_guest
Posted by Luca Fancellu 1 year, 8 months ago
The function arch_set_info_guest is not reached anymore through
VCPUOP_initialise on arm, update the comment.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
 xen/arch/arm/domain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2f8eaab7b56b..6451cd013c1a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -882,9 +882,9 @@ static int is_guest_pv64_psr(uint64_t psr)
 #endif
 
 /*
- * Initialise VCPU state. The context can be supplied by either the
- * toolstack (XEN_DOMCTL_setvcpucontext) or the guest
- * (VCPUOP_initialise) and therefore must be properly validated.
+ * Initialise VCPU state. The context can be supplied by the toolstack
+ * (XEN_DOMCTL_setvcpucontext) and therefore must be properly validated,
+ * or by PSCI call (PSCI_cpu_on) handled by vpsci module.
  */
 int arch_set_info_guest(
     struct vcpu *v, vcpu_guest_context_u c)
-- 
2.17.1
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
Posted by Julien Grall 1 year, 8 months ago
Hi Luca,

On 25/07/2022 15:46, Luca Fancellu wrote:
> The function arch_set_info_guest is not reached anymore through
> VCPUOP_initialise on arm, update the comment.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
>   xen/arch/arm/domain.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 2f8eaab7b56b..6451cd013c1a 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -882,9 +882,9 @@ static int is_guest_pv64_psr(uint64_t psr)
>   #endif
>   
>   /*
> - * Initialise VCPU state. The context can be supplied by either the
> - * toolstack (XEN_DOMCTL_setvcpucontext) or the guest
> - * (VCPUOP_initialise) and therefore must be properly validated.
> + * Initialise VCPU state. The context can be supplied by the toolstack
> + * (XEN_DOMCTL_setvcpucontext) and therefore must be properly validated,
> + * or by PSCI call (PSCI_cpu_on) handled by vpsci module.
>    */

I would prefer if the comment doesn't mention who are the callers. So 
there are no need to modify the comment the next time we add/remove a 
caller. How about something like:

"Initialise vCPU state. The context may be supplied by an external 
entity, so we need to validate it"

Cheers,

-- 
Julien Grall
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
Posted by Luca Fancellu 1 year, 8 months ago

> On 28 Jul 2022, at 19:07, Julien Grall <julien@xen.org> wrote:
> 
> Hi Luca,
> 
> On 25/07/2022 15:46, Luca Fancellu wrote:
>> The function arch_set_info_guest is not reached anymore through
>> VCPUOP_initialise on arm, update the comment.
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> xen/arch/arm/domain.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 2f8eaab7b56b..6451cd013c1a 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -882,9 +882,9 @@ static int is_guest_pv64_psr(uint64_t psr)
>> #endif
>> /*
>> - * Initialise VCPU state. The context can be supplied by either the
>> - * toolstack (XEN_DOMCTL_setvcpucontext) or the guest
>> - * (VCPUOP_initialise) and therefore must be properly validated.
>> + * Initialise VCPU state. The context can be supplied by the toolstack
>> + * (XEN_DOMCTL_setvcpucontext) and therefore must be properly validated,
>> + * or by PSCI call (PSCI_cpu_on) handled by vpsci module.
>> */
> 
> I would prefer if the comment doesn't mention who are the callers. So there are no need to modify the comment the next time we add/remove a caller. How about something like:
> 
> "Initialise vCPU state. The context may be supplied by an external entity, so we need to validate it"

Sounds good! I’ll update and push it soon!

> 
> Cheers,
> 
> -- 
> Julien Grall