Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio_pci driver for mlx5 devices

From: Yishai Hadas <yishaih@nvidia.com>
To: Cornelia Huck <cohuck@redhat.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	"Alex Williamson" <alex.williamson@redhat.com>
Cc: <bhelgaas@google.com>, <saeedm@nvidia.com>,
	<linux-pci@vger.kernel.org>, <kvm@vger.kernel.org>,
	<netdev@vger.kernel.org>, <kuba@kernel.org>, <leonro@nvidia.com>,
	<kwankhede@nvidia.com>, <mgurtovoy@nvidia.com>,
	<maorg@nvidia.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio_pci driver for mlx5 devices
Date: Fri, 29 Oct 2021 10:48:30 +0300	[thread overview]
Message-ID: <76130eb7-6252-85a7-33d9-c7f00f5a3506@nvidia.com> (raw)
In-Reply-To: <87wnlwb9o3.fsf@redhat.com>

On 10/29/2021 9:57 AM, Cornelia Huck wrote:
> On Thu, Oct 28 2021, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
>> On Thu, Oct 28, 2021 at 09:30:35AM -0600, Alex Williamson wrote:
>>> On Wed, 27 Oct 2021 16:23:45 -0300
>>> Jason Gunthorpe <jgg@nvidia.com> wrote:
>>>
>>>> On Wed, Oct 27, 2021 at 01:05:20PM -0600, Alex Williamson wrote:
>>>>
>>>>>> As far as the actual issue, if you hadn't just discovered it now
>>>>>> nobody would have known we have this gap - much like how the very
>>>>>> similar reset issue was present in VFIO for so many years until you
>>>>>> plugged it.
>>>>> But the fact that we did discover it is hugely important.  We've
>>>>> identified that the potential use case is significantly limited and
>>>>> that userspace doesn't have a good mechanism to determine when to
>>>>> expose that limitation to the user.
>>>> Huh?
>>>>
>>>> We've identified that, depending on device behavior, the kernel may
>>>> need to revoke MMIO access to protect itself from hostile userspace
>>>> triggering TLP Errors or something.
>>>>
>>>> Well behaved userspace must already stop touching the MMIO on the
>>>> device when !RUNNING - I see no compelling argument against that
>>>> position.
>>> Not touching MMIO is not specified in our uAPI protocol,
>> To be frank, not much is specified in the uAPI comment, certainly not
>> a detailed meaning of RUNNING.
> Yes! And I think that means we need to improve that comment before the
> first in-tree driver to use it is merged, just to make sure we all agree
> on the protocol, and future drivers can rely on that understanding as
> well.
>

This can done by a follow-up patch as part of the the RC cycles, once we 
agree on the exact comment.

Alternatively,

We can come with V6 on Sunday if we can agree on the comment here soon.

In any case, we don't expect for now code changes in mlx5 as of that.

Frankly, I don't believe that this should block the series from being 
merged.

Yishai