All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harald Freudenberger <freude@linux.ibm.com>
To: Thomas Huth <thuth@redhat.com>,
	linux-s390@vger.kernel.org, Tony Krowiak <akrowiak@linux.ibm.com>,
	Halil Pasic <pasic@linux.ibm.com>,
	Jason Herne <jjherne@linux.ibm.com>
Cc: linux-kernel@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [RFC PATCH] s390: vfio-ap: Register the vfio_ap module for the "ap" parent bus
Date: Thu, 2 Dec 2021 09:33:43 +0100	[thread overview]
Message-ID: <15a39981-37ca-ed2d-3baf-c4cb74f8c343@linux.ibm.com> (raw)
In-Reply-To: <319a5ce7-873b-b8b9-1fa8-f2a69b418ac4@redhat.com>

On 02.12.21 08:13, Thomas Huth wrote:
> On 01/12/2021 18.10, Harald Freudenberger wrote:
>> On 01.12.21 15:11, Thomas Huth wrote:
>>> The crypto devices that we can use with the vfio_ap module are sitting
>>> on the "ap" bus, not on the "vfio_ap" bus that the module defines
>>> itself. With this change, the vfio_ap module now gets automatically
>>> loaded if a supported crypto adapter is available in the host.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   Note: Marked as "RFC" since I'm not 100% sure about it ...
>>>         please review carefully!
>>>
>>>   drivers/s390/crypto/vfio_ap_drv.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
>>> index 4d2556bc7fe5..5580e40608a4 100644
>>> --- a/drivers/s390/crypto/vfio_ap_drv.c
>>> +++ b/drivers/s390/crypto/vfio_ap_drv.c
>>> @@ -39,7 +39,7 @@ static struct ap_device_id ap_queue_ids[] = {
>>>       { /* end of sibling */ },
>>>   };
>>>   -MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
>>> +MODULE_DEVICE_TABLE(ap, ap_queue_ids);
>>>     /**
>>>    * vfio_ap_queue_dev_probe:
>> Hello Thomas, interesting.
>> Did you test this ? I mean did you build a kernel and have it run on a s390 with crypto cards available ?
>
> Yes, I've tested it. Without the patch, the vfio_ap module does not get loaded automatically if a crypto card is available. With the patch applied, the vfio_ap module correctly gets loaded automatically on my system (similar to the vfio_ccw module).
>
>> My strong feeling is that this will make the AP bus code stumble as the code silently assumes there are exact
>> two types of ap devices attached to the ap bus: ap cards and ap queues.
>
> This is only about getting the module loaded automatically once such a device is available ... AFAIK it does not grab any of the devices automatically, so there shouldn't be any problems?
>
>  Thomas
>
Yes, of course for the automatic module load works this way. But you understand that now
the vfio devices are childs of the ap bus and thus are siblings of the ap queue and ap card
devices. As I wrote the ap bus code is not prepared to deal with a 3th type of devices
dangling on the ap bus. So you should test what happens when there are real vfio ap devices
in use together with 'regular' ap card and queue devices.

However, I am still not sure if it is preferable to have the vfio ap module loaded automatically. The majority
of customers will never use vfio ap devices - this is specific to kvm hosts only. I think this has to be
decided by Tony and maybe some kvm architect. If there is an agreement, I will try to rework the
ap code to be able to deal with foreign devices attached to the ap bus.

So thanks for your investigations. Let's wait for Tony and see how we proceed.

Harald
Harald

  reply	other threads:[~2021-12-02  8:33 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-01 14:11 [RFC PATCH] s390: vfio-ap: Register the vfio_ap module for the "ap" parent bus Thomas Huth
2021-12-01 17:10 ` Harald Freudenberger
2021-12-02  7:13   ` Thomas Huth
2021-12-02  8:33     ` Harald Freudenberger [this message]
2021-12-03 19:26       ` Tony Krowiak
2021-12-08 13:46       ` Thomas Huth
2021-12-08 14:25         ` Cornelia Huck
2022-01-27 14:41           ` Tony Krowiak
2022-01-27 14:23       ` Tony Krowiak
2022-01-31 11:03         ` Harald Freudenberger
2021-12-13 15:44 ` Harald Freudenberger
2021-12-13 16:11   ` Cornelia Huck
2021-12-14 21:55     ` Tony Krowiak
2021-12-15 12:05       ` Thomas Huth
2021-12-15 12:51         ` Cornelia Huck
2021-12-15 14:38           ` Thomas Huth
2021-12-15 23:02           ` Halil Pasic
2021-12-16 10:39             ` Cornelia Huck
2021-12-16 11:25               ` Halil Pasic
2022-01-27 15:04             ` Tony Krowiak
2022-01-28  1:35               ` Halil Pasic
2022-01-27 14:48           ` Tony Krowiak
2022-01-27 14:46     ` Tony Krowiak
2021-12-14 21:28   ` Tony Krowiak
2022-01-27 10:33     ` Thomas Huth
2022-01-27 15:10       ` Tony Krowiak
2021-12-16  9:50   ` Harald Freudenberger
2021-12-16 10:44     ` Cornelia Huck
2022-01-27 14:45   ` Tony Krowiak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15a39981-37ca-ed2d-3baf-c4cb74f8c343@linux.ibm.com \
    --to=freude@linux.ibm.com \
    --cc=akrowiak@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=jjherne@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.