linux-kernel - Re: [PATCH v4 10/17] iommufd: Support IOMMU_HWPT

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bd6c6a0f-3b7e-ca7c-468f-d8fe7fb382fb@intel.com>
Date:   Mon, 16 Oct 2023 15:03:04 +0800
From:   Yi Liu <yi.l.liu@...el.com>
To:     Nicolin Chen <nicolinc@...dia.com>,
        Jason Gunthorpe <jgg@...dia.com>
CC:     <joro@...tes.org>, <alex.williamson@...hat.com>,
        <kevin.tian@...el.com>, <robin.murphy@....com>,
        <baolu.lu@...ux.intel.com>, <cohuck@...hat.com>,
        <eric.auger@...hat.com>, <kvm@...r.kernel.org>,
        <mjrosato@...ux.ibm.com>, <chao.p.peng@...ux.intel.com>,
        <yi.y.sun@...ux.intel.com>, <peterx@...hat.com>,
        <jasowang@...hat.com>, <shameerali.kolothum.thodi@...wei.com>,
        <lulu@...hat.com>, <suravee.suthikulpanit@....com>,
        <iommu@...ts.linux.dev>, <linux-kernel@...r.kernel.org>,
        <linux-kselftest@...r.kernel.org>, <zhenzhong.duan@...el.com>,
        <joao.m.martins@...cle.com>
Subject: Re: [PATCH v4 10/17] iommufd: Support IOMMU_HWPT_ALLOC allocation
 with user data

On 2023/10/14 08:51, Nicolin Chen wrote:
> On Fri, Oct 13, 2023 at 09:07:09PM -0300, Jason Gunthorpe wrote:
>> On Fri, Oct 13, 2023 at 01:58:59PM -0700, Nicolin Chen wrote:
>>> On Fri, Oct 13, 2023 at 12:19:23PM -0300, Jason Gunthorpe wrote:
>>>> On Thu, Sep 21, 2023 at 12:51:31AM -0700, Yi Liu wrote:
>>>>> IOMMU_HWPT_ALLOC already supports iommu_domain allocation for usersapce.
>>>>> But it can only allocate a hw_pagetable that associates to a given IOAS,
>>>>> i.e. only a kernel-managed hw_pagetable of IOMMU_HWPT_TYPE_DEFAULT type.
>>>>>
>>>>> IOMMU drivers can now support user-managed hw_pagetables, for two-stage
>>>>> translation use cases, that require user data input from the user space.
>>>>>
>>>>> Extend the IOMMU_HWPT_ALLOC ioctl to accept non-default hwpt_type with a
>>>>> type specified user data. Also, update the @pt_id to accept hwpt_id too
>>>>> besides an ioas_id. Then, pass them to the downstream alloc_fn().
>>>>>
>>>>> Co-developed-by: Nicolin Chen <nicolinc@...dia.com>
>>>>> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
>>>>> Signed-off-by: Yi Liu <yi.l.liu@...el.com>
>>>>> ---
>>>>>   drivers/iommu/iommufd/hw_pagetable.c | 19 ++++++++++++++++++-
>>>>>   include/uapi/linux/iommufd.h         | 23 +++++++++++++++++++++--
>>>>>   2 files changed, 39 insertions(+), 3 deletions(-)
>>>>
>>>> Can we also come with a small vt-d patch that does implement an op for
>>>> this? Or is it too big?
>>>>
>>>> It would be nice if we could wrap IOMMU_HWPT_ALLOC into one
>>>> self-contained series and another series for invalidate.
>>>
>>> We now only use IOMMU_HWPT_ALLOC for nested domain allocations,
>>> which won't be supported until the cache_invalidate_user ops is
>>> preset?
>>>
>>> /* e.g. the following piece is in iommufd_user_managed_hwpt_alloc */
>>>
>>> +       /* Driver is buggy by missing cache_invalidate_user in domain_ops */
>>> +       if (WARN_ON_ONCE(!hwpt->domain->ops->cache_invalidate_user)) {
>>> +               rc = -EINVAL;
>>> +               goto out_abort;
>>> +       }
>>>
>>
>> Hm. That hunk could migrate to the invalidate series.
>>
>> I'm just leeary of doing the invalidation too considering how
>> complicated it is
> 
> OK. Let's see how Yi/Kevin/Baolu reply about the feasibility
> with the VT-d driver. Then Yi and I can accordingly separate
> the allocation part into a smaller series.

Current nesting series actually extends HWPT_ALLOC ioctl to accept user
data for allocating domain with vendor specific data. Nested translation
happens to be the usage of it. But nesting requires invalidation. If we
want to do further split, then this new series would be just "extending
HWPT_ALLOC to accept vendor specific data from userspace". But it will
lack of a user if nesting is separated. Is this acceptable? @Jason

BTW. Do we still have unsolved issue on the invalidation path?

-- 
Regards,
Yi Liu