Message-ID: <2a889dcf-e20f-84cc-cc69-6cb381764047@redhat.com>
Date:   Wed, 15 Dec 2021 15:38:35 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [RFC PATCH] s390: vfio-ap: Register the vfio_ap module for the
 "ap" parent bus
Content-Language: en-US
To:     Cornelia Huck <cohuck@redhat.com>,
        Tony Krowiak <akrowiak@linux.ibm.com>,
        Harald Freudenberger <freude@linux.ibm.com>,
        linux-s390@vger.kernel.org, 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>,
        Christian Borntraeger <borntraeger@de.ibm.com>
References: <20211201141110.94636-1-thuth@redhat.com>
 <8512bb0a-a34a-09b0-65f3-781f3d092364@linux.ibm.com>
 <87k0g8scx1.fsf@redhat.com>
 <1eb9ca5c-b1bb-b768-64ee-e4a1b31bb171@linux.ibm.com>
 <6aaf6c60-a258-29e3-fcec-82c77d3945a4@redhat.com> <87tufaqbex.fsf@redhat.com>
From:   Thomas Huth <thuth@redhat.com>
In-Reply-To: <87tufaqbex.fsf@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 15/12/2021 13.51, Cornelia Huck wrote:
> On Wed, Dec 15 2021, Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 14/12/2021 22.55, Tony Krowiak wrote:
>>>
>>>
>>> On 12/13/21 11:11, Cornelia Huck wrote:
>>>> One possibility is simply blocking autoload of the module in userspace by
>>>> default, and only allow it to be loaded automatically when e.g. qemu-kvm
>>>> is installed on the system. This is obviously something that needs to be
>>>> decided by the distros.
>>>>
>>>> (kvm might actually be autoloaded already, so autoloading vfio-ap would
>>>> not really make it worse.)
>>>
>>> Of the vfio_ccw module is automatically loaded, then the kvm
>>> module will also get loaded. I startup up a RHEL8.3 system and
>>> sure enough, the vfio_ccw module is loaded along with the
>>> kvm, vfio and mdev modules. If this is true for all distros, then
>>> it wouldn't make much difference if the vfio_ap module is
>>> autoloaded too.
>>
>> I think I don't mind too much if we auto-load vfio-ap or not - but I think
>> we should make it consistent with vfio-ccw. So either auto-load both modules
>> (if the corresponding devices are available), or remove the
>> MODULE_DEVICE_TABLE() entries from both modules?
> 
> I think we really need to take a step back and think about the purpose
> of adding a MODULE_DEVICE_TABLE()... basically, it declares which types
> of devices on a certain bus a driver supports, in a way that can be
> consumed by userspace (after file2alias.c worked on it).
> 
> Userspace typically uses this to match devices it is notified about to
> drivers that could possibly drive those devices. In general, the
> assumption is that you will want to have the drivers for your devices
> loaded. In some cases (drivers only used in special cases, like here),
> it might be a better idea to autoload the drivers only under certain
> circumstances (e.g. if you know you're going to run KVM guests).
> 
> My main point, however, is that we're talking about policy here: whether
> a potentially useful driver should be loaded or not is a decision that
> should be made by userspace. Not providing a MODULE_DEVICE_TABLE does
> not look like the right solution, as it deprives userspace of the
> information to autoload the driver, if it actually wants to do so.

Thanks, Cornelia, that's a very valid point, I didn't see it from this 
perspective yet, but it sounds like the right way to handle this. So I think 
my patch should be applied, and if we then agree that the module should not 
be auto-loaded by default, it should be handled with the appropriate changes 
in userspace instead.

  Thomas