From: Baolu Lu <baolu.lu@linux.intel.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: baolu.lu@linux.intel.com, Kevin Tian <kevin.tian@intel.com>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>,
Nicolin Chen <nicolinc@nvidia.com>, Yi Liu <yi.l.liu@intel.com>,
Jacob Pan <jacob.jun.pan@linux.intel.com>,
Joel Granados <j.granados@samsung.com>,
iommu@lists.linux.dev, virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/8] iommufd: Add iommufd fault object
Date: Mon, 25 Mar 2024 13:01:32 +0800 [thread overview]
Message-ID: <90813c19-5fef-4c29-9387-6c9e2770a549@linux.intel.com> (raw)
In-Reply-To: <20240322170939.GJ66976@ziepe.ca>
On 2024/3/23 1:09, Jason Gunthorpe wrote:
> On Fri, Mar 15, 2024 at 09:46:06AM +0800, Baolu Lu wrote:
>> On 3/9/24 2:03 AM, Jason Gunthorpe wrote:
>>> On Mon, Jan 22, 2024 at 03:38:59PM +0800, Lu Baolu wrote:
>>>> --- /dev/null
>>>> +++ b/drivers/iommu/iommufd/fault.c
>>>> @@ -0,0 +1,255 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/* Copyright (C) 2024 Intel Corporation
>>>> + */
>>>> +#define pr_fmt(fmt) "iommufd: " fmt
>>>> +
>>>> +#include <linux/file.h>
>>>> +#include <linux/fs.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/mutex.h>
>>>> +#include <linux/iommufd.h>
>>>> +#include <linux/poll.h>
>>>> +#include <linux/anon_inodes.h>
>>>> +#include <uapi/linux/iommufd.h>
>>>> +
>>>> +#include "iommufd_private.h"
>>>> +
>>>> +static int device_add_fault(struct iopf_group *group)
>>>> +{
>>>> + struct iommufd_device *idev = group->cookie->private;
>>>> + void *curr;
>>>> +
>>>> + curr = xa_cmpxchg(&idev->faults, group->last_fault.fault.prm.grpid,
>>>> + NULL, group, GFP_KERNEL);
>>>> +
>>>> + return curr ? xa_err(curr) : 0;
>>>> +}
>>>> +
>>>> +static void device_remove_fault(struct iopf_group *group)
>>>> +{
>>>> + struct iommufd_device *idev = group->cookie->private;
>>>> +
>>>> + xa_store(&idev->faults, group->last_fault.fault.prm.grpid,
>>>> + NULL, GFP_KERNEL);
>>>
>>> xa_erase ?
>>
>> Yes. Sure.
>>
>>> Is grpid OK to use this way? Doesn't it come from the originating
>>> device?
>>
>> The group ID is generated by the hardware. Here, we use it as an index
>> in the fault array to ensure it can be quickly retrieved in the page
>> fault response path.
>
> I'm nervous about this, we are trusting HW outside the kernel to
> provide unique grp id's which are integral to how the kernel
> operates..
Agreed.
>
>>>> +static ssize_t iommufd_fault_fops_read(struct file *filep, char __user *buf,
>>>> + size_t count, loff_t *ppos)
>>>> +{
>>>> + size_t fault_size = sizeof(struct iommu_hwpt_pgfault);
>>>> + struct iommufd_fault *fault = filep->private_data;
>>>> + struct iommu_hwpt_pgfault data;
>>>> + struct iommufd_device *idev;
>>>> + struct iopf_group *group;
>>>> + struct iopf_fault *iopf;
>>>> + size_t done = 0;
>>>> + int rc;
>>>> +
>>>> + if (*ppos || count % fault_size)
>>>> + return -ESPIPE;
>>>> +
>>>> + mutex_lock(&fault->mutex);
>>>> + while (!list_empty(&fault->deliver) && count > done) {
>>>> + group = list_first_entry(&fault->deliver,
>>>> + struct iopf_group, node);
>>>> +
>>>> + if (list_count_nodes(&group->faults) * fault_size > count - done)
>>>> + break;
>>>> +
>>>> + idev = (struct iommufd_device *)group->cookie->private;
>>>> + list_for_each_entry(iopf, &group->faults, list) {
>>>> + iommufd_compose_fault_message(&iopf->fault, &data, idev);
>>>> + rc = copy_to_user(buf + done, &data, fault_size);
>>>> + if (rc)
>>>> + goto err_unlock;
>>>> + done += fault_size;
>>>> + }
>>>> +
>>>> + rc = device_add_fault(group);
>>>
>>> See I wonder if this should be some xa_alloc or something instead of
>>> trying to use the grpid?
>>
>> So this magic number will be passed to user space in the fault message.
>> And the user will then include this number in its response message. The
>> response message is valid only when the magic number matches. Do I get
>> you correctly?
>
> Yes, then it is simple xa_alloc() and xa_load() without any other
> searching and we don't have to rely on the grpid to be correctly
> formed by the PCI device.
>
> But I don't know about performance xa_alloc() is pretty fast but
> trusting the grpid would be faster..
>
> IMHO from a uapi perspective we should have a definate "cookie" that
> gets echo'd back. If the kernel uses xa_alloc or grpid to build that
> cookie it doesn't matter to the uAPI.
Okay, I will head in this direction.
Best regards,
baolu
next prev parent reply other threads:[~2024-03-25 5:01 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-22 7:38 [PATCH v3 0/8] IOMMUFD: Deliver IO page faults to user space Lu Baolu
2024-01-22 7:38 ` [PATCH v3 1/8] iommu: Add iopf domain attach/detach/replace interface Lu Baolu
2024-02-07 8:11 ` Tian, Kevin
2024-02-21 5:52 ` Baolu Lu
2024-02-21 6:49 ` Tian, Kevin
2024-02-21 7:21 ` Baolu Lu
2024-02-21 7:22 ` Tian, Kevin
2024-01-22 7:38 ` [PATCH v3 2/8] iommu/sva: Use iopf domain attach/detach interface Lu Baolu
2024-03-08 17:46 ` Jason Gunthorpe
2024-03-14 7:41 ` Baolu Lu
2024-03-22 16:59 ` Jason Gunthorpe
2024-03-25 3:52 ` Baolu Lu
2024-01-22 7:38 ` [PATCH v3 3/8] iommufd: Add fault and response message definitions Lu Baolu
2024-03-08 17:50 ` Jason Gunthorpe
2024-03-14 13:41 ` Baolu Lu
2024-03-22 17:04 ` Jason Gunthorpe
2024-03-25 3:57 ` Baolu Lu
2024-01-22 7:38 ` [PATCH v3 4/8] iommufd: Add iommufd fault object Lu Baolu
2024-03-08 18:03 ` Jason Gunthorpe
2024-03-15 1:46 ` Baolu Lu
2024-03-22 17:09 ` Jason Gunthorpe
2024-03-25 5:01 ` Baolu Lu [this message]
2024-03-20 16:18 ` Shameerali Kolothum Thodi
2024-03-22 17:22 ` Jason Gunthorpe
2024-03-25 3:26 ` Baolu Lu
2024-03-25 4:02 ` Baolu Lu
2024-01-22 7:39 ` [PATCH v3 5/8] iommufd: Associate fault object with iommufd_hw_pgtable Lu Baolu
2024-02-07 8:14 ` Tian, Kevin
2024-02-21 6:06 ` Baolu Lu
2024-03-02 2:36 ` Zhangfei Gao
2024-03-06 15:15 ` Zhangfei Gao
2024-03-06 16:01 ` Jason Gunthorpe
2024-03-07 1:54 ` Baolu Lu
2024-03-08 17:19 ` Jason Gunthorpe
2024-03-08 19:05 ` Jason Gunthorpe
2024-03-15 1:16 ` Baolu Lu
2024-03-22 17:06 ` Jason Gunthorpe
2024-03-25 4:59 ` Baolu Lu
2024-01-22 7:39 ` [PATCH v3 6/8] iommufd: IOPF-capable hw page table attach/detach/replace Lu Baolu
[not found] ` <CGME20240220135755eucas1p24e882cb5a735caed4bbfedb22a811442@eucas1p2.samsung.com>
2024-02-20 13:57 ` Joel Granados
2024-02-21 6:15 ` Baolu Lu
2024-01-22 7:39 ` [PATCH v3 7/8] iommufd/selftest: Add IOPF support for mock device Lu Baolu
2024-01-22 7:39 ` [PATCH v3 8/8] iommufd/selftest: Add coverage for IOPF test Lu Baolu
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=90813c19-5fef-4c29-9387-6c9e2770a549@linux.intel.com \
--to=baolu.lu@linux.intel.com \
--cc=iommu@lists.linux.dev \
--cc=j.granados@samsung.com \
--cc=jacob.jun.pan@linux.intel.com \
--cc=jean-philippe@linaro.org \
--cc=jgg@ziepe.ca \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=robin.murphy@arm.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=will@kernel.org \
--cc=yi.l.liu@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).