Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iommufd fault queue #24

Merged
merged 5 commits into from
Dec 10, 2024
Merged

iommufd fault queue #24

merged 5 commits into from
Dec 10, 2024

Conversation

birkelund
Copy link
Collaborator

No description provided.

@birkelund birkelund added the approved Approved for device testing label Sep 23, 2024
@birkelund birkelund changed the title Iommufd fault queue iommufd fault queue Sep 23, 2024

static struct opt_table opts[] = {
OPT_WITHOUT_ARG("-h|--help", opt_set_bool, &show_usage, "show usage"),
OPT_WITHOUT_ARG("-v|--verbose", opt_set_bool, &verbose, "verbose"),

OPT_WITH_ARG("-d|--device BDF", opt_set_charp, opt_show_charp, &bdf, "pci device"),
OPT_WITHOUT_ARG("-x|--reset", opt_set_bool, &reset, "reset"),
OPT_WITH_ARG("-t|--target DRIVER", opt_set_charp, opt_show_charp, &target, "bind to DRIVER"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a hit on the checkpatch script. You think you just need to line break before "bind to DRIVER"),

@@ -49,6 +50,7 @@ struct iommufd_device_info {

int dev_fd;
int dev_id;
int hwpt_id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need hwpt_id?

The hwpt_id is used immediately after it is produced to never be used again in
libvfn. It is relevant for the Linux Kernel in the following calls:
- IOMMU_HWPT_SET_DIRT_TRACKING
- IOMMU_HWPT_GET_DIRTY_BITMAP
- IOMMU_HWPT_INVALIDATE
Should we just not have it until we need it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we do not expose struct iommufd_device_info in the public headers, I see why we could get away with not storing it. We do not even need it for detaching.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just remove it/comment it out until the time that it is needed.


attach_cmd.pt_id = hwpt_alloc_cmd.out_hwpt_id;

if (ioctl(devfd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &attach_cmd)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about VFIO_DEVICE_DETACH_IOMMUFD_PT

At the moment the way to use the IOPF functionality in libvfn is by using a
VFIO_DEVICE_ATTACH_IOMMUFD_PT ioctl and then it is detached when the pci
device is closed. Couple of questions here:

  1. Do we see that the user will always have to close the PCI device to detach
    the hwpt? What happens if the user needs to connect to another hwpt or just
    needs another one?
  2. Does it make sense to think about providing a way to detach the hwpt without
    closing the pci device?
  3. Notice that this is more like a general comment regarding VFIO_DEVICE_ATTACH_IOMMUFD_PT than for this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to provide that functionality. How to do that I am not sure. This series suggests to add an iommufd specific header file vfn/iommu/iommufd.h since this functionality is tied to iommufd and not generic like the rest of the API in vfn/iommu.h. In lieu of that, detaching could (should) be provided as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on what we discussed offline, I would not implement a detach and leave it for when you have more information on how the library is being used.

birkelund and others added 5 commits December 10, 2024 07:59
Only check that the target device is an NVMe device when binding to the
nvme driver.

Signed-off-by: Klaus Jensen <[email protected]>
One might need to point the build to a linux kernel include directory
different from the one that is currently installed. To avoid having to
constantly install the linux kernel headers on the system, add a meson
build option that adds a user defined path to the include paths of the
project.

Signed-off-by: Joel Granados <[email protected]>
[k.jensen: refactor]
Signed-off-by: Klaus Jensen <[email protected]>
Add an internal list to keep track of devices that have been binded to
iommufd.

Signed-off-by: Klaus Jensen <[email protected]>
Add support for iommufd based fault queues.

Signed-off-by: Joel Granados <[email protected]>
[k.jensen: refactored]
Signed-off-by: Klaus Jensen <[email protected]>
Add example using the iommufd fault queue to handle device page faults.

This example is intended to be used with the QEMU pcie-ats-testdev
device. See QEMU for documentation.

Signed-off-by: Klaus Jensen <[email protected]>
@birkelund birkelund merged commit 7a9b7ce into main Dec 10, 2024
26 checks passed
@birkelund birkelund deleted the iommufd-fault-queue branch December 10, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for device testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants