Re: [PATCH v1 01/20] s390/ap: Move response_type struct into ap_msg struct

[Holger Dengler <dengler@linux.ibm.com>]

On 25/02/2025 09:56, Harald Freudenberger wrote:
> On 2025-02-24 16:23, Holger Dengler wrote:
>> On 23/02/2025 10:54, Harald Freudenberger wrote:
>>> Move the very small response_type struct into struct ap_msg.
>>> So there is no need to kmalloc this tiny struct with each
>>> ap message preparation.
>>
>> I understand the intention for this patch, but in my opinion the
>> layering concept between ap and zcrypt is violated by defining the
>> response-type as part of the ap message struct. But I don't have any
>> better solution, so for the moment you may leave it as is.
>>
>>>
>>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>>> ---
>>>  drivers/s390/crypto/ap_bus.h           |  12 ++-
>>>  drivers/s390/crypto/zcrypt_msgtype50.c |  22 +++---
>>>  drivers/s390/crypto/zcrypt_msgtype6.c  | 101 ++++++++++---------------
>>>  3 files changed, 59 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
>>> index f4622ee4d894..a5d8f805625f 100644
>>> --- a/drivers/s390/crypto/ap_bus.h
>>> +++ b/drivers/s390/crypto/ap_bus.h
>>>  struct ap_message {
>>>      struct list_head list;        /* Request queueing. */
>>>      unsigned long psmid;        /* Message id. */
>>> @@ -222,7 +231,7 @@ struct ap_message {
>>>      size_t bufsize;            /* allocated msg buffer size */
>>>      u16 flags;            /* Flags, see AP_MSG_FLAG_xxx */
>>>      int rc;                /* Return code for this message */
>>> -    void *private;            /* ap driver private pointer. */
>>> +    struct ap_response_type response;
>>
>> I don't like this change. The completion and the type are both
>> message-type related. That means, this change pulls messate-type
>> related data definitions into the ap-layer. On the other hand, I have
>> currently no idea how this can be solved.
>>
> 
> Well, the "private" data could be opaque allocated in ap_init_apmsg without
> any knowledge about the data - just the size. And the msg type 50 and 6
> implementations could just check for the right size and then overlay the
> private data bytes with their own struct.

The only "problem" is, that the lower layer (ap in this case) has no knowledge, how much memory is required for the private data.

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com