Re: [PATCH 02/16] powerpc/vas: Make VAS API powerpc platform independent

[Michael Ellerman <mpe@ellerman.id.au>]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 11/04/2021 à 02:31, Haren Myneni a écrit :
>> 
>> Using the same /dev/crypto/nx-gzip interface for both powerNV and
>> pseries. So this patcb moves VAS API to powerpc platform indepedent
>> directory. The actual functionality is not changed in this patch.
>
> This patch seems to do a lot more than moving VAS API to independent directory. A more detailed 
> description would help.
>
> And it is not something defined in the powerpc architecture I think, so it should
> remain in some common platform related directory.
>
>> 
>> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
>> ---
>>   arch/powerpc/Kconfig                          | 15 +++++
>>   arch/powerpc/include/asm/vas.h                | 22 ++++++-
>>   arch/powerpc/kernel/Makefile                  |  1 +
>>   .../{platforms/powernv => kernel}/vas-api.c   | 64 ++++++++++--------
>>   arch/powerpc/platforms/powernv/Kconfig        | 14 ----
>>   arch/powerpc/platforms/powernv/Makefile       |  2 +-
>>   arch/powerpc/platforms/powernv/vas-window.c   | 66 +++++++++++++++++++
>>   7 files changed, 140 insertions(+), 44 deletions(-)
>>   rename arch/powerpc/{platforms/powernv => kernel}/vas-api.c (83%)
>> 
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 386ae12d8523..7aa1fbf7c1dc 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -478,6 +478,21 @@ config PPC_UV
>>   
>>   	  If unsure, say "N".
>>   
>> +config PPC_VAS
>> +	bool "IBM Virtual Accelerator Switchboard (VAS)"
>> +	depends on PPC_POWERNV && PPC_64K_PAGES
>> +	default y
>> +	help
>> +	  This enables support for IBM Virtual Accelerator Switchboard (VAS).
>
> IIUC is a functionnality in a coprocessor of some IBM processors. Something similar in principle to 
> the communication coprocessors we find in Freescale processors.

It's not a coprocessor, it's a way you talk to coprocessors.

> It is not a generic functionnality part of the powerpc architecture, I don't think this belongs to 
> arch/powerpc/Kconfig

But you're right it's not part of the ISA.

> I think it should go in arch/powerpc/platform/Kconfig

The problem with that is it's shared between two existing platforms, ie.
powernv and pseries. We don't want to put it in one or the other.

In the past we have put code like that in arch/powerpc/sysdev, but I am
not a big fan of it, because it's just a bit of a dumping ground.

A while back I created arch/powerpc/platforms/4xx for 40x and 44x
related things, even though there's no actual 4xx platform. I don't
think that's caused any problems.

So I'm inclined to say we should make a arch/powerpc/platforms/book3s
and put VAS in there.

The naming is a bit fishy, because not all book3s CPUs do or will have
VAS. But we would expect any future CPU with VAS to be book3s.

In contrast if we named it platforms/ibm, we could potentially have a
future non-IBM CPU that contains VAS, which would then make the ibm name
confusing.

cheers