Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
Hi Luca, On 22/08/2022 07:56, Luca Fancellu wrote: On 5 Aug 2022, at 18:35, Julien Grall wrote: Hi Luca, On 05/08/2022 14:08, 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 --- Changes in v2: - rephrased comment to not list caller functions (Julien) --- xen/arch/arm/domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 2cd481979cf1..9db8a37a089c 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -885,9 +885,8 @@ 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 may be supplied by an external entity, so + * we need to validate it NIT: Missing a full stop. This can be added on commit. Acked-by: Julien Grall Hi Julien, Any plan to commit this one? Not an important change, just asking so that I can remove it from my watch list. Sorry for the delay. It is now pushed. Cheers, -- Julien Grall
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
> On 5 Aug 2022, at 18:35, Julien Grall wrote: > > Hi Luca, > > On 05/08/2022 14:08, 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 >> --- >> Changes in v2: >> - rephrased comment to not list caller functions (Julien) >> --- >> xen/arch/arm/domain.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c >> index 2cd481979cf1..9db8a37a089c 100644 >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -885,9 +885,8 @@ 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 may be supplied by an external >> entity, so >> + * we need to validate it > > NIT: Missing a full stop. This can be added on commit. > > Acked-by: Julien Grall Hi Julien, Any plan to commit this one? Not an important change, just asking so that I can remove it from my watch list. Cheers, Luca > > Cheers, > > -- > Julien Grall
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
Hi Luca, On 05/08/2022 14:08, 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 --- Changes in v2: - rephrased comment to not list caller functions (Julien) --- xen/arch/arm/domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 2cd481979cf1..9db8a37a089c 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -885,9 +885,8 @@ 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 may be supplied by an external entity, so + * we need to validate it NIT: Missing a full stop. This can be added on commit. Acked-by: Julien Grall Cheers, -- Julien Grall
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
> On 5 Aug 2022, at 14:08, 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 Hi All, Sorry I forgot to put v2 in the tag. Cheers, Luca > --- > Changes in v2: > - rephrased comment to not list caller functions (Julien) > --- > xen/arch/arm/domain.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 2cd481979cf1..9db8a37a089c 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -885,9 +885,8 @@ 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 may be supplied by an external entity, > so > + * we need to validate it > */ > int arch_set_info_guest( > struct vcpu *v, vcpu_guest_context_u c) > -- > 2.17.1 > >
[PATCH] arm/domain: fix comment for arch_set_info_guest
The function arch_set_info_guest is not reached anymore through VCPUOP_initialise on arm, update the comment. Signed-off-by: Luca Fancellu --- Changes in v2: - rephrased comment to not list caller functions (Julien) --- xen/arch/arm/domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 2cd481979cf1..9db8a37a089c 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -885,9 +885,8 @@ 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 may be supplied by an external entity, so + * we need to validate it */ 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
> On 28 Jul 2022, at 19:07, Julien Grall 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 >> --- >> 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
Re: [PATCH] arm/domain: fix comment for arch_set_info_guest
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 --- 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
[PATCH] arm/domain: fix comment for arch_set_info_guest
The function arch_set_info_guest is not reached anymore through VCPUOP_initialise on arm, update the comment. Signed-off-by: Luca Fancellu --- 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
