| Message ID | 0-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com |
|---|---|
| Headers | show |
| Series | IOMMUFD Generic interface | expand |
On Mon, Nov 07, 2022 at 11:25:26PM -0800, Nicolin Chen wrote: > On Mon, Nov 07, 2022 at 08:49:09PM -0400, Jason Gunthorpe wrote: > > > diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c > > > @@ -489,6 +494,15 @@ static int pages_to_xarray(struct xarray *xa, unsigned long start_index, > > > > xas_lock(&xas); > > while (pages != end_pages) { > > + /* xarray does not participate in fault injection */ > > + if (pages == half_pages && iommufd_should_fail()) { > > + xas_set_err(&xas, -EINVAL); > > + xas_unlock(&xas); > > + /* aka xas_destroy() */ > > + xas_nomem(&xas, GFP_KERNEL); > > + goto err_clear; > > Coverity reports an "unchecked return value" at xas_nomem()... > > > +err_clear: > > if (xas_error(&xas)) { > > ...yet, I think we should be fine since we do xas_error here? Yes, in this flow xas_nomem() always returns false and there is no reason to check it. Just Coverity noise Jason
On Mon, Nov 07, 2022 at 09:48:32PM -0800, Nicolin Chen wrote: > On Mon, Nov 07, 2022 at 08:49:08PM -0400, Jason Gunthorpe wrote: > > > diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c > > > +TEST_F(iommufd, cmd_length) > > +{ > > +#define TEST_LENGTH(_struct, _ioctl) \ > > + { \ > > + struct { \ > > + struct _struct cmd; \ > > + uint8_t extra; \ > > + } cmd = { .cmd = { .size = sizeof(struct _struct) - 1 }, \ > > + .extra = UINT8_MAX }; \ > > + int old_errno; \ > > + int rc; \ > > + \ > > + EXPECT_ERRNO(EOPNOTSUPP, ioctl(self->fd, _ioctl, &cmd)); \ > > I guess it should be EINVAL corresponding to updated kernel code? > > > +TEST_F(iommufd, cmd_ex_fail) > > +{ > > + struct { > > + struct iommu_destroy cmd; > > + __u64 future; > > + } cmd = { .cmd = { .size = sizeof(cmd), .id = 0 } }; > > + > > + /* object id is invalid and command is longer */ > > + EXPECT_ERRNO(ENOENT, ioctl(self->fd, IOMMU_DESTROY, &cmd)); > > + /* future area is non-zero */ > > + cmd.future = 1; > > + EXPECT_ERRNO(E2BIG, ioctl(self->fd, IOMMU_DESTROY, &cmd)); > > + /* Original command "works" */ > > + cmd.cmd.size = sizeof(cmd.cmd); > > + EXPECT_ERRNO(ENOENT, ioctl(self->fd, IOMMU_DESTROY, &cmd)); > > + /* Short command fails */ > > + cmd.cmd.size = sizeof(cmd.cmd) - 1; > > + EXPECT_ERRNO(EOPNOTSUPP, ioctl(self->fd, IOMMU_DESTROY, &cmd)); > > Ditto Oops, yes, I fixed these > > > +TEST_HARNESS_MAIN > > diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c > > > +static void fail_nth_first(struct __test_metadata *_metadata, > > + struct fail_nth_state *nth_state) > > +{ > > + char buf[300]; > > + > > + snprintf(buf, sizeof(buf), "/proc/self/task/%u/fail-nth", gettid()); > > Not sure what's missing, I have a build error at gettid. Copying > a solution from tools/perf/jvmti/jvmti_agent.c file, can fix with: I think your glibc is probably old > ------------------------------ > diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c > index 99eaa9f32e0b..7704b3a754d3 100644 > --- a/tools/testing/selftests/iommu/iommufd_fail_nth.c > +++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c > @@ -19,6 +19,7 @@ > > #define __EXPORTED_HEADERS__ > #include <linux/vfio.h> > +#include <syscall.h> /* for gettid() */ > > #include "iommufd_utils.h" > > @@ -84,6 +85,13 @@ struct fail_nth_state { > unsigned int iteration; > }; > > +#ifndef HAVE_GETTID > +static inline pid_t gettid(void) > +{ > + return (pid_t)syscall(__NR_gettid); > +} > +#endif Ah, there is a lot of complicated makefile stuff to make this work, and it only works for perf/bpf not selftests It looks like there is no reason for this to need gettid, we don't use threads. So this can just be getpid and that is portable. Thanks, Jason
On Tue, Nov 08, 2022 at 08:27:13PM +0700, Bagas Sanjaya wrote: > I see kernel-doc warnings (missing short descriptions) when making > htmldocs, so I have applied the fixup: > > ---- >8 ---- > > From 5643b202ae9853c11434466c76aeaaa044e88b07 Mon Sep 17 00:00:00 2001 > From: Bagas Sanjaya <bagasdotme@gmail.com> > Date: Tue, 8 Nov 2022 20:14:25 +0700 > Subject: [PATCH] iommufd: Add missing ioctl short descriptions > > Checking kernel-doc comments in iommufd.h header with scripts/kernel-doc > produces missing short description warnings: > > include/uapi/linux/iommufd.h:80: warning: missing initial short description on line: > * struct iommu_iova_range > include/uapi/linux/iommufd.h:250: warning: missing initial short description on line: > * enum iommufd_option > include/uapi/linux/iommufd.h:267: warning: missing initial short description on line: > * enum iommufd_option_ops > include/uapi/linux/iommufd.h:277: warning: Cannot understand * @size: sizeof(struct iommu_option) > on line 277 - I thought it was a doc line > include/uapi/linux/iommufd.h:299: warning: missing initial short description on line: > * enum iommufd_vfio_ioas_op > > One of them is reported by Stephen Rothwell when merging iommufd tree for > linux-next. Thanks, I was about to try to figure out what that was. I squashed it in. Jason
> -----Original Message----- > From: Jason Gunthorpe [mailto:jgg@nvidia.com] > Sent: 08 November 2022 00:49 > To: bpf@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David > Woodhouse <dwmw2@infradead.org>; iommu@lists.linux.dev; Joerg Roedel > <joro@8bytes.org>; Kevin Tian <kevin.tian@intel.com>; > linux-doc@vger.kernel.org; linux-kselftest@vger.kernel.org; > llvm@lists.linux.dev; Nathan Chancellor <nathan@kernel.org>; Nick > Desaulniers <ndesaulniers@google.com>; Miguel Ojeda <ojeda@kernel.org>; > Robin Murphy <robin.murphy@arm.com>; Shuah Khan <shuah@kernel.org>; > Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Tom Rix > <trix@redhat.com>; Will Deacon <will@kernel.org> > Cc: Alex Williamson <alex.williamson@redhat.com>; Lu Baolu > <baolu.lu@linux.intel.com>; Chaitanya Kulkarni <chaitanyak@nvidia.com>; > Cornelia Huck <cohuck@redhat.com>; Daniel Jordan > <daniel.m.jordan@oracle.com>; David Gibson > <david@gibson.dropbear.id.au>; Eric Auger <eric.auger@redhat.com>; Eric > Farman <farman@linux.ibm.com>; Jason Wang <jasowang@redhat.com>; > Jean-Philippe Brucker <jean-philippe@linaro.org>; Joao Martins > <joao.m.martins@oracle.com>; kvm@vger.kernel.org; Matthew Rosato > <mjrosato@linux.ibm.com>; Michael S. Tsirkin <mst@redhat.com>; Nicolin > Chen <nicolinc@nvidia.com>; Niklas Schnelle <schnelle@linux.ibm.com>; > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; Yi > Liu <yi.l.liu@intel.com>; zhukeqian <zhukeqian1@huawei.com> > Subject: [PATCH v4 00/17] IOMMUFD Generic interface [...] > > - Userspace page tables aka 'nested translation' for ARM and Intel iommu > drivers: > https://github.com/nicolinc/iommufd/commits/iommufd_nesting Hi Eric/Yi/Nicolin, Could you please provide a latest Kernel/Qemu branch for the ARM nesting support? The above link points to Yi's git, but not sure which one is latest/stable to have a play. Thanks, Shameer
Hi Shameer, On 2022/11/11 23:51, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: Jason Gunthorpe [mailto:jgg@nvidia.com] >> Sent: 08 November 2022 00:49 >> To: bpf@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David >> Woodhouse <dwmw2@infradead.org>; iommu@lists.linux.dev; Joerg Roedel >> <joro@8bytes.org>; Kevin Tian <kevin.tian@intel.com>; >> linux-doc@vger.kernel.org; linux-kselftest@vger.kernel.org; >> llvm@lists.linux.dev; Nathan Chancellor <nathan@kernel.org>; Nick >> Desaulniers <ndesaulniers@google.com>; Miguel Ojeda <ojeda@kernel.org>; >> Robin Murphy <robin.murphy@arm.com>; Shuah Khan <shuah@kernel.org>; >> Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Tom Rix >> <trix@redhat.com>; Will Deacon <will@kernel.org> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Lu Baolu >> <baolu.lu@linux.intel.com>; Chaitanya Kulkarni <chaitanyak@nvidia.com>; >> Cornelia Huck <cohuck@redhat.com>; Daniel Jordan >> <daniel.m.jordan@oracle.com>; David Gibson >> <david@gibson.dropbear.id.au>; Eric Auger <eric.auger@redhat.com>; Eric >> Farman <farman@linux.ibm.com>; Jason Wang <jasowang@redhat.com>; >> Jean-Philippe Brucker <jean-philippe@linaro.org>; Joao Martins >> <joao.m.martins@oracle.com>; kvm@vger.kernel.org; Matthew Rosato >> <mjrosato@linux.ibm.com>; Michael S. Tsirkin <mst@redhat.com>; Nicolin >> Chen <nicolinc@nvidia.com>; Niklas Schnelle <schnelle@linux.ibm.com>; >> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; Yi >> Liu <yi.l.liu@intel.com>; zhukeqian <zhukeqian1@huawei.com> >> Subject: [PATCH v4 00/17] IOMMUFD Generic interface > [...] >> >> - Userspace page tables aka 'nested translation' for ARM and Intel iommu >> drivers: >> https://github.com/nicolinc/iommufd/commits/iommufd_nesting > > Hi Eric/Yi/Nicolin, > > Could you please provide a latest Kernel/Qemu branch for the ARM nesting support? > The above link points to Yi's git, but not sure which one is latest/stable to > have a play. Nicolin and I are working on the new version for nesting support. Below kernl branch is our latest progress so far. As the naming, it's still wip. We also need to workout a Qemu version, so still need some time before sharing with you. https://github.com/yiliu1765/iommufd/tree/wip/iommufd-v6.1-rc3-nesting
On Tue, Jan 10, 2023 at 03:16:23PM +0000, Joao Martins wrote: > On 10/01/2023 13:49, Jason Gunthorpe wrote: > > Can you also look at the dirty tracking stuff? I'd really like to see > > that done for the huawei vfio live migration driver > > > > He did look and provides comments based on his testing of the initial series > (IIUC from what we spoke at LPC). v2 should be simpler, though I am still > working it out the merging of unit tests into iommufd. > > My plan once I post the v2 was to handover the SMMUv3 part back to Shameerali > given the fact he has hardware that has the feature and can iterate more > meaningfully than me. Sounds good Thanks, Jason
> -----Original Message----- > From: Joao Martins [mailto:joao.m.martins@oracle.com] > Sent: 10 January 2023 15:16 > To: Jason Gunthorpe <jgg@nvidia.com>; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> > Cc: Yi Liu <yi.l.liu@intel.com>; bpf@vger.kernel.org; Jonathan Corbet > <corbet@lwn.net>; David Woodhouse <dwmw2@infradead.org>; > iommu@lists.linux.dev; Joerg Roedel <joro@8bytes.org>; Kevin Tian > <kevin.tian@intel.com>; linux-doc@vger.kernel.org; > linux-kselftest@vger.kernel.org; llvm@lists.linux.dev; Nathan Chancellor > <nathan@kernel.org>; Nick Desaulniers <ndesaulniers@google.com>; Miguel > Ojeda <ojeda@kernel.org>; Robin Murphy <robin.murphy@arm.com>; Shuah > Khan <shuah@kernel.org>; Suravee Suthikulpanit > <suravee.suthikulpanit@amd.com>; Tom Rix <trix@redhat.com>; Will > Deacon <will@kernel.org>; Alex Williamson <alex.williamson@redhat.com>; > Lu Baolu <baolu.lu@linux.intel.com>; Chaitanya Kulkarni > <chaitanyak@nvidia.com>; Cornelia Huck <cohuck@redhat.com>; Daniel > Jordan <daniel.m.jordan@oracle.com>; David Gibson > <david@gibson.dropbear.id.au>; Eric Auger <eric.auger@redhat.com>; Eric > Farman <farman@linux.ibm.com>; Jason Wang <jasowang@redhat.com>; > Jean-Philippe Brucker <jean-philippe@linaro.org>; kvm@vger.kernel.org; > Matthew Rosato <mjrosato@linux.ibm.com>; Michael S. Tsirkin > <mst@redhat.com>; Nicolin Chen <nicolinc@nvidia.com>; Niklas Schnelle > <schnelle@linux.ibm.com>; zhukeqian <zhukeqian1@huawei.com> > Subject: Re: [PATCH v4 00/17] IOMMUFD Generic interface > > On 10/01/2023 13:49, Jason Gunthorpe wrote: > > Can you also look at the dirty tracking stuff? I'd really like to see > > that done for the huawei vfio live migration driver > > > > He did look and provides comments based on his testing of the initial series > (IIUC from what we spoke at LPC). v2 should be simpler, though I am still > working it out the merging of unit tests into iommufd. > > My plan once I post the v2 was to handover the SMMUv3 part back to > Shameerali > given the fact he has hardware that has the feature and can iterate more > meaningfully than me. No problem. Happy to help. Thanks, Shameer