Re: [PATCH v2 09/15] xen/arm: psci: Detect SMCCC version

[Julien Grall <julien.grall@arm.com>]

On 12/02/18 14:43, Volodymyr Babchuk wrote:
> Hi Julien,
> 
> On 09.02.18 19:09, Julien Grall wrote:
>>
>>
>> On 02/09/2018 05:04 PM, Volodymyr Babchuk wrote:
>>> Julien,
>>>
>>>
>>> On 08.02.18 21:21, Julien Grall wrote:
>>>> PSCI 1.0 and later allows the SMCCC version to be (indirectly) probed
>>>> via PSCI_FEATURES. If the PSCI_FEATURES does not exist (PSCI 0.2 or
>>>> earlier) and the function return an error, then we considered SMCCC 1.0
>>>> is implemented.
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>>> ---
>>>>      Changes in v2:
>>>>          - Patch added
>>>> ---
>>>>   xen/arch/arm/psci.c         | 34 +++++++++++++++++++++++++++++++++-
>>>>   xen/include/asm-arm/smccc.h |  5 ++++-
>>>>   2 files changed, 37 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
>>>
>>> I find it strange to determine SMCCC version in PSCI code. psci.c is 
>>> not the first place, where I will look for SMCCC version discovery.
>>> I think it is better to add smccc.c, where such functions can reside.
>>
>> SMCCC version discovery is based on PSCI, hence it is in the PSCI 
>> code. I can't see a good reason to create a file with 3 lines at the 
>> moment.
> 
> SMCCC version discovery is a Arm Architecture Service function. PSCI 
> used to discover if this function is supported at all. Dubious 
> architectural solution from my point of view. But it is already done...
> 
> We had similar discussions about introducing new files earlier, so you 
> know  my point. I would like to see clean codebase where one can 
> navigate without grep/cscope. I see no point, why function that calls 
> Arm architecture service to identify SMCCC version should reside in PSCI 
> code.

Good luck for navigating without grep/cscope in a such a big code base.

> 
> Besides, that file will have more than 3 lines at the moment. Your 
> current psci_init_smccc is longer right now :)

You got my point with "3 lines"... It is one function in one file. There 
limited reason to create a file for that, more that I clearly don't want 
to expose psci_features() outside of psci.c for now.

We can revisit this in the future if we need to discover SMCCC in a 
different way.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel