linux-kernel - Re: [PATCH] vfio: mlx5, pds: add IOMMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <725697d0-ffac-4660-b601-bde4a821efd3@oracle.com>
Date:   Mon, 23 Oct 2023 21:23:17 +0100
From:   Joao Martins <joao.m.martins@...cle.com>
To:     Arnd Bergmann <arnd@...db.de>, Jason Gunthorpe <jgg@...dia.com>
Cc:     Arnd Bergmann <arnd@...nel.org>, Kevin Tian <kevin.tian@...el.com>,
        Yishai Hadas <yishaih@...dia.com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Brett Creeley <brett.creeley@....com>,
        oushixiong <oushixiong@...inos.cn>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfio: mlx5, pds: add IOMMU_SUPPORT dependency



On 23/10/2023 19:46, Arnd Bergmann wrote:
> On Mon, Oct 23, 2023, at 19:50, Joao Martins wrote:
>> On 23/10/2023 14:12, Jason Gunthorpe wrote:
>>> On Mon, Oct 23, 2023 at 01:37:28PM +0100, Joao Martins wrote:
>>
>> To be specific what I meant to move is the IOMMUFD_DRIVER kconfig part, not the
>> whole iommufd Kconfig [in the patch introducing the problem] e.g.
>>
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 2b12b583ef4b..5cc869db1b79 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -7,6 +7,10 @@ config IOMMU_IOVA
>>  config IOMMU_API
>>         bool
>>
>> +config IOMMUFD_DRIVER
>> +       bool
>> +       default n
>> +
>>  menuconfig IOMMU_SUPPORT
>>         bool "IOMMU Hardware Support"
>>         depends on MMU
>> diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
>> index 1fa543204e89..99d4b075df49 100644
>> --- a/drivers/iommu/iommufd/Kconfig
>> +++ b/drivers/iommu/iommufd/Kconfig
>> @@ -11,10 +11,6 @@ config IOMMUFD
>>
>>           If you don't know what to do here, say N.
>>
>> -config IOMMUFD_DRIVER
>> -       bool
>> -       default n
>> -
>>  if IOMMUFD
>>  config IOMMUFD_VFIO_CONTAINER
>>         bool "IOMMUFD provides the VFIO container /dev/vfio/vfio"
>>
>> (...) or in alternative, do similar to this patch except that it's:
>>
>> 	select IOMMUFD_DRIVER if IOMMU_SUPPORT
>>
>> In the mlx5/pds vfio drivers.
> 
> If I understand it right, we have two providers (AMD and
> Intel iommu) and two consumers (mlx5 and pds) for this
> interface, so we probably don't want to use 'select' for
> both sides here.
> 
It's not quite one consumes the other.

IOMMU drivers use iova-bitmap, and are providers of the IOMMU support to IOMMUFD
usage.

The mlx5/pds (and VFIO too if either is enabled) consume iova-bitmap (thus
select IOMMUFD_DRIVER where the code is now being moved under) but they aren't
tied to the IOMMU support of it.

This is what we are trying to capture in the kconfig and thus structured it
this way.

> As with CONFIG_IOMMU_API, two two logical options are
> to either have a hidden symbol selected by the providers
> that the consumers depend on, or have a user-visible
> symbol and use 'depends on IOMMUFD_DRIVER' for both
> the providers and the consumers.
> 
> Either way, I think the problem with the warning goes
> away.
> 
>> Perhaps the merging of IOMMU_API with IOMMU_SUPPORT should
>> be best done separately?
> 
> Right, that part should be improved as well, but it's
> not causing other problems at the moment.
> 
>      Arnd