* Re: [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
From: Jason Gunthorpe @ 2019-06-10 13:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Yishai Hadas, Doug Ledford, linux-rdma, linux-kernel,
kernel-janitors
In-Reply-To: <20190608092231.GA28890@mwanda>
On Sat, Jun 08, 2019 at 12:22:31PM +0300, Dan Carpenter wrote:
> The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
> larger than the number of bits in an int then that's undefined behavior.
> It turns out this doesn't really cause an issue at runtime but it's
> still nice to clean it up.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/infiniband/hw/mlx4/qp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> index 5221c0794d1d..9f6eb23e8044 100644
> --- a/drivers/infiniband/hw/mlx4/qp.c
> +++ b/drivers/infiniband/hw/mlx4/qp.c
> @@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
> struct mlx4_ib_create_qp *ucmd)
> {
> /* Sanity check SQ size before proceeding */
> - if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
> + if (ucmd->log_sq_bb_count > 31 ||
> + (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
Surely this should use check_shl_overflow() ?
Jason
^ permalink raw reply
* Re: [PATCH for-rc 3/3] IB/hfi1: Correct tid qp rcd to match verbs context
From: Jason Gunthorpe @ 2019-06-10 13:25 UTC (permalink / raw)
To: Marciniszyn, Mike
Cc: Leon Romanovsky, Dalessandro, Dennis, dledford@redhat.com,
linux-rdma@vger.kernel.org, stable@vger.kernel.org, Wan, Kaike
In-Reply-To: <32E1700B9017364D9B60AED9960492BC70DA2848@fmsmsx120.amr.corp.intel.com>
On Mon, Jun 10, 2019 at 01:03:54PM +0000, Marciniszyn, Mike wrote:
> > > diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c
> > b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > > index 6fb9303..d77276d 100644
> > > +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > > @@ -312,9 +312,8 @@ static struct hfi1_ctxtdata *qp_to_rcd(struct
> > rvt_dev_info *rdi,
> > > if (qp->ibqp.qp_num == 0)
> > > ctxt = 0;
> > > else
> > > - ctxt = ((qp->ibqp.qp_num >> dd->qos_shift) %
> > > - (dd->n_krcv_queues - 1)) + 1;
> > > -
> > > + ctxt = hfi1_get_qp_map(dd,
> > > + (u8)(qp->ibqp.qp_num >> dd-
> > >qos_shift));
> >
> > It is one time use functions, why don't you handle this (u8) casting
> > inside of hfi1_get_qp_map()?
> >
>
> I assume the suggestion is to remove the u8 cast at the call site?
>
> The function return value already is a u8 and there is a cast of the 64 bit CSR read result.
Why do you need an explicit cast at all?
Jason
^ permalink raw reply
* Re: [PATCH for-rc 3/3] IB/hfi1: Correct tid qp rcd to match verbs context
From: Leon Romanovsky @ 2019-06-10 13:10 UTC (permalink / raw)
To: Marciniszyn, Mike
Cc: Dalessandro, Dennis, jgg@ziepe.ca, dledford@redhat.com,
linux-rdma@vger.kernel.org, stable@vger.kernel.org, Wan, Kaike
In-Reply-To: <32E1700B9017364D9B60AED9960492BC70DA2848@fmsmsx120.amr.corp.intel.com>
On Mon, Jun 10, 2019 at 01:03:54PM +0000, Marciniszyn, Mike wrote:
> > > diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c
> > b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > > index 6fb9303..d77276d 100644
> > > --- a/drivers/infiniband/hw/hfi1/tid_rdma.c
> > > +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > > @@ -312,9 +312,8 @@ static struct hfi1_ctxtdata *qp_to_rcd(struct
> > rvt_dev_info *rdi,
> > > if (qp->ibqp.qp_num == 0)
> > > ctxt = 0;
> > > else
> > > - ctxt = ((qp->ibqp.qp_num >> dd->qos_shift) %
> > > - (dd->n_krcv_queues - 1)) + 1;
> > > -
> > > + ctxt = hfi1_get_qp_map(dd,
> > > + (u8)(qp->ibqp.qp_num >> dd-
> > >qos_shift));
> >
> > It is one time use functions, why don't you handle this (u8) casting
> > inside of hfi1_get_qp_map()?
> >
>
> I assume the suggestion is to remove the u8 cast at the call site?
Yes, sorry for not being clear.
>
> The function return value already is a u8 and there is a cast of the 64 bit CSR read result.
>
> Mike
^ permalink raw reply
* Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Jason Gunthorpe @ 2019-06-10 13:09 UTC (permalink / raw)
To: Ralph Campbell
Cc: Andrea Arcangeli, linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <e460ddf5-9ed3-7f3b-98ce-526c12fdb8b1-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Fri, Jun 07, 2019 at 03:39:06PM -0700, Ralph Campbell wrote:
> > > + range->hmm = hmm;
> > > + kref_get(&hmm->kref);
> > > /* Initialize range to track CPU page table updates. */
> > > mutex_lock(&hmm->lock);
> > >
>
> I forgot to add that I think you can delete the duplicate
> "range->hmm = hmm;"
> here between the mutex_lock/unlock.
Done, thanks
Jason
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* RE: [PATCH for-rc 3/3] IB/hfi1: Correct tid qp rcd to match verbs context
From: Marciniszyn, Mike @ 2019-06-10 13:03 UTC (permalink / raw)
To: Leon Romanovsky, Dalessandro, Dennis
Cc: jgg@ziepe.ca, dledford@redhat.com, linux-rdma@vger.kernel.org,
stable@vger.kernel.org, Wan, Kaike
In-Reply-To: <20190608081533.GO5261@mtr-leonro.mtl.com>
> > diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c
> b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > index 6fb9303..d77276d 100644
> > --- a/drivers/infiniband/hw/hfi1/tid_rdma.c
> > +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
> > @@ -312,9 +312,8 @@ static struct hfi1_ctxtdata *qp_to_rcd(struct
> rvt_dev_info *rdi,
> > if (qp->ibqp.qp_num == 0)
> > ctxt = 0;
> > else
> > - ctxt = ((qp->ibqp.qp_num >> dd->qos_shift) %
> > - (dd->n_krcv_queues - 1)) + 1;
> > -
> > + ctxt = hfi1_get_qp_map(dd,
> > + (u8)(qp->ibqp.qp_num >> dd-
> >qos_shift));
>
> It is one time use functions, why don't you handle this (u8) casting
> inside of hfi1_get_qp_map()?
>
I assume the suggestion is to remove the u8 cast at the call site?
The function return value already is a u8 and there is a cast of the 64 bit CSR read result.
Mike
^ permalink raw reply
* Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
From: John Hubbard @ 2019-06-10 0:16 UTC (permalink / raw)
To: Jason Gunthorpe, Christoph Hellwig
Cc: Andrea Arcangeli, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Felix.Kuehling-5C7GfCeVMHo@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Jerome Glisse,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
In-Reply-To: <20190608114133.GA14873-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On 6/8/19 4:41 AM, Jason Gunthorpe wrote:
> On Sat, Jun 08, 2019 at 02:10:08AM -0700, Christoph Hellwig wrote:
>> On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
>>> HMM defines its own struct hmm_update which is passed to the
>>> sync_cpu_device_pagetables() callback function. This is
>>> sufficient when the only action is to invalidate. However,
>>> a device may want to know the reason for the invalidation and
>>> be able to see the new permissions on a range, update device access
>>> rights or range statistics. Since sync_cpu_device_pagetables()
>>> can be called from try_to_unmap(), the mmap_sem may not be held
>>> and find_vma() is not safe to be called.
>>> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
>>> to allow the full invalidation information to be used.
>>>
>>> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
>>>
>>> I'm sending this out now since we are updating many of the HMM APIs
>>> and I think it will be useful.
>>
>> This is the right thing to do. But the really right thing is to just
>> kill the hmm_mirror API entirely and move to mmu_notifiers. At least
>> for noveau this already is way simpler, although right now it defeats
>> Jasons patch to avoid allocating the struct hmm in the fault path.
>> But as said before that can be avoided by just killing struct hmm,
>> which for many reasons is the right thing to do anyway.
>>
>> I've got a series here, which is a bit broken (epecially the last
>> patch can't work as-is), but should explain where I'm trying to head:
>>
>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-mirror-simplification
>
> At least the current hmm approach does rely on the collision retry
> locking scheme in struct hmm/struct hmm_range for the pagefault side
> to work right.
>
> So, before we can apply patch one in this series we need to fix
> hmm_vma_fault() and all its varients. Otherwise the driver will be
> broken.
>
> I'm hoping to first define what this locking should be (see other
> emails to Ralph) then, ideally, see if we can extend mmu notifiers to
> get it directly withouth hmm stuff.
>
> Then we apply your patch one and the hmm ops wrapper dies.
>
This all makes sense, and thanks for all this work to simplify and clarify
HMM. It's going to make it a lot easier to work with, when the dust settles.
thanks,
--
John Hubbard
NVIDIA
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
From: Ira Weiny @ 2019-06-09 19:46 UTC (permalink / raw)
To: Ralph Campbell
Cc: Andrea Arcangeli, linux-rdma, John Hubbard, Felix.Kuehling,
dri-devel, linux-mm, Jerome Glisse, Jason Gunthorpe, amd-gfx
In-Reply-To: <20190608001452.7922-1-rcampbell@nvidia.com>
On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
> HMM defines its own struct hmm_update which is passed to the
> sync_cpu_device_pagetables() callback function. This is
> sufficient when the only action is to invalidate. However,
> a device may want to know the reason for the invalidation and
> be able to see the new permissions on a range, update device access
> rights or range statistics. Since sync_cpu_device_pagetables()
> can be called from try_to_unmap(), the mmap_sem may not be held
> and find_vma() is not safe to be called.
> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
> to allow the full invalidation information to be used.
>
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
I don't disagree with Christoph or Jason but since I've been trying to sort out
where hmm does and does not fit any chance to remove a custom structure is a
good simplification IMO. So...
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> ---
>
> I'm sending this out now since we are updating many of the HMM APIs
> and I think it will be useful.
>
>
> drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++--
> include/linux/hmm.h | 27 ++-------------------------
> mm/hmm.c | 13 ++++---------
> 3 files changed, 8 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 8c92374afcf2..c34b98fafe2f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -252,13 +252,13 @@ nouveau_svmm_invalidate(struct nouveau_svmm *svmm, u64 start, u64 limit)
>
> static int
> nouveau_svmm_sync_cpu_device_pagetables(struct hmm_mirror *mirror,
> - const struct hmm_update *update)
> + const struct mmu_notifier_range *update)
> {
> struct nouveau_svmm *svmm = container_of(mirror, typeof(*svmm), mirror);
> unsigned long start = update->start;
> unsigned long limit = update->end;
>
> - if (!update->blockable)
> + if (!mmu_notifier_range_blockable(update))
> return -EAGAIN;
>
> SVMM_DBG(svmm, "invalidate %016lx-%016lx", start, limit);
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 0fa8ea34ccef..07a2d38fde34 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -377,29 +377,6 @@ static inline uint64_t hmm_pfn_from_pfn(const struct hmm_range *range,
>
> struct hmm_mirror;
>
> -/*
> - * enum hmm_update_event - type of update
> - * @HMM_UPDATE_INVALIDATE: invalidate range (no indication as to why)
> - */
> -enum hmm_update_event {
> - HMM_UPDATE_INVALIDATE,
> -};
> -
> -/*
> - * struct hmm_update - HMM update information for callback
> - *
> - * @start: virtual start address of the range to update
> - * @end: virtual end address of the range to update
> - * @event: event triggering the update (what is happening)
> - * @blockable: can the callback block/sleep ?
> - */
> -struct hmm_update {
> - unsigned long start;
> - unsigned long end;
> - enum hmm_update_event event;
> - bool blockable;
> -};
> -
> /*
> * struct hmm_mirror_ops - HMM mirror device operations callback
> *
> @@ -420,7 +397,7 @@ struct hmm_mirror_ops {
> /* sync_cpu_device_pagetables() - synchronize page tables
> *
> * @mirror: pointer to struct hmm_mirror
> - * @update: update information (see struct hmm_update)
> + * @update: update information (see struct mmu_notifier_range)
> * Return: -EAGAIN if update.blockable false and callback need to
> * block, 0 otherwise.
> *
> @@ -434,7 +411,7 @@ struct hmm_mirror_ops {
> * synchronous call.
> */
> int (*sync_cpu_device_pagetables)(struct hmm_mirror *mirror,
> - const struct hmm_update *update);
> + const struct mmu_notifier_range *update);
> };
>
> /*
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9aad3550f2bb..b49a43712554 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -164,7 +164,6 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> - struct hmm_update update;
> struct hmm_range *range;
> unsigned long flags;
> int ret = 0;
> @@ -173,15 +172,10 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> if (!kref_get_unless_zero(&hmm->kref))
> return 0;
>
> - update.start = nrange->start;
> - update.end = nrange->end;
> - update.event = HMM_UPDATE_INVALIDATE;
> - update.blockable = mmu_notifier_range_blockable(nrange);
> -
> spin_lock_irqsave(&hmm->ranges_lock, flags);
> hmm->notifiers++;
> list_for_each_entry(range, &hmm->ranges, list) {
> - if (update.end < range->start || update.start >= range->end)
> + if (nrange->end < range->start || nrange->start >= range->end)
> continue;
>
> range->valid = false;
> @@ -198,9 +192,10 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> list_for_each_entry(mirror, &hmm->mirrors, list) {
> int rc;
>
> - rc = mirror->ops->sync_cpu_device_pagetables(mirror, &update);
> + rc = mirror->ops->sync_cpu_device_pagetables(mirror, nrange);
> if (rc) {
> - if (WARN_ON(update.blockable || rc != -EAGAIN))
> + if (WARN_ON(mmu_notifier_range_blockable(nrange) ||
> + rc != -EAGAIN))
> continue;
> ret = -EAGAIN;
> break;
> --
> 2.20.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v3 13/33] docs: infiniband: convert docs to ReST and rename to *.rst
From: Mauro Carvalho Chehab @ 2019-06-09 2:27 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Doug Ledford, Jason Gunthorpe, linux-rdma
In-Reply-To: <cover.1560045490.git.mchehab+samsung@kernel.org>
The InfiniBand docs are plain text with no markups.
So, all we needed to do were to add the title markups and
some markup sequences in order to properly parse tables,
lists and literal blocks.
At its new index.rst, let's add a :orphan: while this is not linked to
the main index.rst file, in order to avoid build warnings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
.../{core_locking.txt => core_locking.rst} | 64 ++++++-----
Documentation/infiniband/index.rst | 23 ++++
.../infiniband/{ipoib.txt => ipoib.rst} | 24 ++--
.../infiniband/{opa_vnic.txt => opa_vnic.rst} | 108 +++++++++---------
.../infiniband/{sysfs.txt => sysfs.rst} | 4 +-
.../{tag_matching.txt => tag_matching.rst} | 5 +
.../infiniband/{user_mad.txt => user_mad.rst} | 33 ++++--
.../{user_verbs.txt => user_verbs.rst} | 12 +-
drivers/infiniband/core/user_mad.c | 2 +-
drivers/infiniband/ulp/ipoib/Kconfig | 2 +-
10 files changed, 174 insertions(+), 103 deletions(-)
rename Documentation/infiniband/{core_locking.txt => core_locking.rst} (78%)
create mode 100644 Documentation/infiniband/index.rst
rename Documentation/infiniband/{ipoib.txt => ipoib.rst} (90%)
rename Documentation/infiniband/{opa_vnic.txt => opa_vnic.rst} (63%)
rename Documentation/infiniband/{sysfs.txt => sysfs.rst} (69%)
rename Documentation/infiniband/{tag_matching.txt => tag_matching.rst} (98%)
rename Documentation/infiniband/{user_mad.txt => user_mad.rst} (90%)
rename Documentation/infiniband/{user_verbs.txt => user_verbs.rst} (93%)
diff --git a/Documentation/infiniband/core_locking.txt b/Documentation/infiniband/core_locking.rst
similarity index 78%
rename from Documentation/infiniband/core_locking.txt
rename to Documentation/infiniband/core_locking.rst
index 4b1f36b6ada0..f34669beb4fe 100644
--- a/Documentation/infiniband/core_locking.txt
+++ b/Documentation/infiniband/core_locking.rst
@@ -1,4 +1,6 @@
-INFINIBAND MIDLAYER LOCKING
+===========================
+InfiniBand Midlayer Locking
+===========================
This guide is an attempt to make explicit the locking assumptions
made by the InfiniBand midlayer. It describes the requirements on
@@ -6,45 +8,47 @@ INFINIBAND MIDLAYER LOCKING
protocols that use the midlayer.
Sleeping and interrupt context
+==============================
With the following exceptions, a low-level driver implementation of
all of the methods in struct ib_device may sleep. The exceptions
are any methods from the list:
- create_ah
- modify_ah
- query_ah
- destroy_ah
- post_send
- post_recv
- poll_cq
- req_notify_cq
- map_phys_fmr
+ - create_ah
+ - modify_ah
+ - query_ah
+ - destroy_ah
+ - post_send
+ - post_recv
+ - poll_cq
+ - req_notify_cq
+ - map_phys_fmr
which may not sleep and must be callable from any context.
The corresponding functions exported to upper level protocol
consumers:
- ib_create_ah
- ib_modify_ah
- ib_query_ah
- ib_destroy_ah
- ib_post_send
- ib_post_recv
- ib_req_notify_cq
- ib_map_phys_fmr
+ - ib_create_ah
+ - ib_modify_ah
+ - ib_query_ah
+ - ib_destroy_ah
+ - ib_post_send
+ - ib_post_recv
+ - ib_req_notify_cq
+ - ib_map_phys_fmr
are therefore safe to call from any context.
In addition, the function
- ib_dispatch_event
+ - ib_dispatch_event
used by low-level drivers to dispatch asynchronous events through
the midlayer is also safe to call from any context.
Reentrancy
+----------
All of the methods in struct ib_device exported by a low-level
driver must be fully reentrant. The low-level driver is required to
@@ -62,6 +66,7 @@ Reentrancy
information between different calls of ib_poll_cq() is not defined.
Callbacks
+---------
A low-level driver must not perform a callback directly from the
same callchain as an ib_device method call. For example, it is not
@@ -74,18 +79,18 @@ Callbacks
completion event handlers for the same CQ are not called
simultaneously. The driver must guarantee that only one CQ event
handler for a given CQ is running at a time. In other words, the
- following situation is not allowed:
+ following situation is not allowed::
- CPU1 CPU2
+ CPU1 CPU2
- low-level driver ->
- consumer CQ event callback:
- /* ... */
- ib_req_notify_cq(cq, ...);
- low-level driver ->
- /* ... */ consumer CQ event callback:
- /* ... */
- return from CQ event handler
+ low-level driver ->
+ consumer CQ event callback:
+ /* ... */
+ ib_req_notify_cq(cq, ...);
+ low-level driver ->
+ /* ... */ consumer CQ event callback:
+ /* ... */
+ return from CQ event handler
The context in which completion event and asynchronous event
callbacks run is not defined. Depending on the low-level driver, it
@@ -93,6 +98,7 @@ Callbacks
Upper level protocol consumers may not sleep in a callback.
Hot-plug
+--------
A low-level driver announces that a device is ready for use by
consumers when it calls ib_register_device(), all initialization
diff --git a/Documentation/infiniband/index.rst b/Documentation/infiniband/index.rst
new file mode 100644
index 000000000000..22eea64de722
--- /dev/null
+++ b/Documentation/infiniband/index.rst
@@ -0,0 +1,23 @@
+:orphan:
+
+==========
+InfiniBand
+==========
+
+.. toctree::
+ :maxdepth: 1
+
+ core_locking
+ ipoib
+ opa_vnic
+ sysfs
+ tag_matching
+ user_mad
+ user_verbs
+
+.. only:: subproject and html
+
+ Indices
+ =======
+
+ * :ref:`genindex`
diff --git a/Documentation/infiniband/ipoib.txt b/Documentation/infiniband/ipoib.rst
similarity index 90%
rename from Documentation/infiniband/ipoib.txt
rename to Documentation/infiniband/ipoib.rst
index 47c1dd9818f2..0dd36154c0c9 100644
--- a/Documentation/infiniband/ipoib.txt
+++ b/Documentation/infiniband/ipoib.rst
@@ -1,4 +1,6 @@
-IP OVER INFINIBAND
+==================
+IP over InfiniBand
+==================
The ib_ipoib driver is an implementation of the IP over InfiniBand
protocol as specified by RFC 4391 and 4392, issued by the IETF ipoib
@@ -8,16 +10,17 @@ IP OVER INFINIBAND
masqueraded to the kernel as ethernet interfaces).
Partitions and P_Keys
+=====================
When the IPoIB driver is loaded, it creates one interface for each
port using the P_Key at index 0. To create an interface with a
different P_Key, write the desired P_Key into the main interface's
- /sys/class/net/<intf name>/create_child file. For example:
+ /sys/class/net/<intf name>/create_child file. For example::
echo 0x8001 > /sys/class/net/ib0/create_child
This will create an interface named ib0.8001 with P_Key 0x8001. To
- remove a subinterface, use the "delete_child" file:
+ remove a subinterface, use the "delete_child" file::
echo 0x8001 > /sys/class/net/ib0/delete_child
@@ -28,6 +31,7 @@ Partitions and P_Keys
rtnl_link_ops, where children created using either way behave the same.
Datagram vs Connected modes
+===========================
The IPoIB driver supports two modes of operation: datagram and
connected. The mode is set and read through an interface's
@@ -51,6 +55,7 @@ Datagram vs Connected modes
networking stack to use the smaller UD MTU for these neighbours.
Stateless offloads
+==================
If the IB HW supports IPoIB stateless offloads, IPoIB advertises
TCP/IP checksum and/or Large Send (LSO) offloading capability to the
@@ -60,9 +65,10 @@ Stateless offloads
on/off using ethtool calls. Currently LRO is supported only for
checksum offload capable devices.
- Stateless offloads are supported only in datagram mode.
+ Stateless offloads are supported only in datagram mode.
Interrupt moderation
+====================
If the underlying IB device supports CQ event moderation, one can
use ethtool to set interrupt mitigation parameters and thus reduce
@@ -71,6 +77,7 @@ Interrupt moderation
moderation is supported.
Debugging Information
+=====================
By compiling the IPoIB driver with CONFIG_INFINIBAND_IPOIB_DEBUG set
to 'y', tracing messages are compiled into the driver. They are
@@ -79,7 +86,7 @@ Debugging Information
runtime through files in /sys/module/ib_ipoib/.
CONFIG_INFINIBAND_IPOIB_DEBUG also enables files in the debugfs
- virtual filesystem. By mounting this filesystem, for example with
+ virtual filesystem. By mounting this filesystem, for example with::
mount -t debugfs none /sys/kernel/debug
@@ -96,10 +103,13 @@ Debugging Information
performance, because it adds tests to the fast path.
References
+==========
Transmission of IP over InfiniBand (IPoIB) (RFC 4391)
- http://ietf.org/rfc/rfc4391.txt
+ http://ietf.org/rfc/rfc4391.txt
+
IP over InfiniBand (IPoIB) Architecture (RFC 4392)
- http://ietf.org/rfc/rfc4392.txt
+ http://ietf.org/rfc/rfc4392.txt
+
IP over InfiniBand: Connected Mode (RFC 4755)
http://ietf.org/rfc/rfc4755.txt
diff --git a/Documentation/infiniband/opa_vnic.txt b/Documentation/infiniband/opa_vnic.rst
similarity index 63%
rename from Documentation/infiniband/opa_vnic.txt
rename to Documentation/infiniband/opa_vnic.rst
index 282e17be798a..2f888d9ffec0 100644
--- a/Documentation/infiniband/opa_vnic.txt
+++ b/Documentation/infiniband/opa_vnic.rst
@@ -1,3 +1,7 @@
+=================================================================
+Intel Omni-Path (OPA) Virtual Network Interface Controller (VNIC)
+=================================================================
+
Intel Omni-Path (OPA) Virtual Network Interface Controller (VNIC) feature
supports Ethernet functionality over Omni-Path fabric by encapsulating
the Ethernet packets between HFI nodes.
@@ -17,70 +21,72 @@ an independent Ethernet network. The configuration is performed by an
Ethernet Manager (EM) which is part of the trusted Fabric Manager (FM)
application. HFI nodes can have multiple VNICs each connected to a
different virtual Ethernet switch. The below diagram presents a case
-of two virtual Ethernet switches with two HFI nodes.
+of two virtual Ethernet switches with two HFI nodes::
- +-------------------+
- | Subnet/ |
- | Ethernet |
- | Manager |
- +-------------------+
- / /
- / /
- / /
- / /
-+-----------------------------+ +------------------------------+
-| Virtual Ethernet Switch | | Virtual Ethernet Switch |
-| +---------+ +---------+ | | +---------+ +---------+ |
-| | VPORT | | VPORT | | | | VPORT | | VPORT | |
-+--+---------+----+---------+-+ +-+---------+----+---------+---+
- | \ / |
- | \ / |
- | \/ |
- | / \ |
- | / \ |
- +-----------+------------+ +-----------+------------+
- | VNIC | VNIC | | VNIC | VNIC |
- +-----------+------------+ +-----------+------------+
- | HFI | | HFI |
- +------------------------+ +------------------------+
+ +-------------------+
+ | Subnet/ |
+ | Ethernet |
+ | Manager |
+ +-------------------+
+ / /
+ / /
+ / /
+ / /
+ +-----------------------------+ +------------------------------+
+ | Virtual Ethernet Switch | | Virtual Ethernet Switch |
+ | +---------+ +---------+ | | +---------+ +---------+ |
+ | | VPORT | | VPORT | | | | VPORT | | VPORT | |
+ +--+---------+----+---------+-+ +-+---------+----+---------+---+
+ | \ / |
+ | \ / |
+ | \/ |
+ | / \ |
+ | / \ |
+ +-----------+------------+ +-----------+------------+
+ | VNIC | VNIC | | VNIC | VNIC |
+ +-----------+------------+ +-----------+------------+
+ | HFI | | HFI |
+ +------------------------+ +------------------------+
The Omni-Path encapsulated Ethernet packet format is as described below.
-Bits Field
-------------------------------------
+==================== ================================
+Bits Field
+==================== ================================
Quad Word 0:
-0-19 SLID (lower 20 bits)
-20-30 Length (in Quad Words)
-31 BECN bit
-32-51 DLID (lower 20 bits)
-52-56 SC (Service Class)
-57-59 RC (Routing Control)
-60 FECN bit
-61-62 L2 (=10, 16B format)
-63 LT (=1, Link Transfer Head Flit)
+0-19 SLID (lower 20 bits)
+20-30 Length (in Quad Words)
+31 BECN bit
+32-51 DLID (lower 20 bits)
+52-56 SC (Service Class)
+57-59 RC (Routing Control)
+60 FECN bit
+61-62 L2 (=10, 16B format)
+63 LT (=1, Link Transfer Head Flit)
Quad Word 1:
-0-7 L4 type (=0x78 ETHERNET)
-8-11 SLID[23:20]
-12-15 DLID[23:20]
-16-31 PKEY
-32-47 Entropy
-48-63 Reserved
+0-7 L4 type (=0x78 ETHERNET)
+8-11 SLID[23:20]
+12-15 DLID[23:20]
+16-31 PKEY
+32-47 Entropy
+48-63 Reserved
Quad Word 2:
-0-15 Reserved
-16-31 L4 header
-32-63 Ethernet Packet
+0-15 Reserved
+16-31 L4 header
+32-63 Ethernet Packet
Quad Words 3 to N-1:
-0-63 Ethernet packet (pad extended)
+0-63 Ethernet packet (pad extended)
Quad Word N (last):
-0-23 Ethernet packet (pad extended)
-24-55 ICRC
-56-61 Tail
-62-63 LT (=01, Link Transfer Tail Flit)
+0-23 Ethernet packet (pad extended)
+24-55 ICRC
+56-61 Tail
+62-63 LT (=01, Link Transfer Tail Flit)
+==================== ================================
Ethernet packet is padded on the transmit side to ensure that the VNIC OPA
packet is quad word aligned. The 'Tail' field contains the number of bytes
@@ -123,7 +129,7 @@ operation. It also handles the encapsulation of Ethernet packets with an
Omni-Path header in the transmit path. For each VNIC interface, the
information required for encapsulation is configured by the EM via VEMA MAD
interface. It also passes any control information to the HW dependent driver
-by invoking the RDMA netdev control operations.
+by invoking the RDMA netdev control operations::
+-------------------+ +----------------------+
| | | Linux |
diff --git a/Documentation/infiniband/sysfs.txt b/Documentation/infiniband/sysfs.rst
similarity index 69%
rename from Documentation/infiniband/sysfs.txt
rename to Documentation/infiniband/sysfs.rst
index 9fab5062f84b..f0abd6fa48f4 100644
--- a/Documentation/infiniband/sysfs.txt
+++ b/Documentation/infiniband/sysfs.rst
@@ -1,4 +1,6 @@
-SYSFS FILES
+===========
+Sysfs files
+===========
The sysfs interface has moved to
Documentation/ABI/stable/sysfs-class-infiniband.
diff --git a/Documentation/infiniband/tag_matching.txt b/Documentation/infiniband/tag_matching.rst
similarity index 98%
rename from Documentation/infiniband/tag_matching.txt
rename to Documentation/infiniband/tag_matching.rst
index d2a3bf819226..ef56ea585f92 100644
--- a/Documentation/infiniband/tag_matching.txt
+++ b/Documentation/infiniband/tag_matching.rst
@@ -1,12 +1,16 @@
+==================
Tag matching logic
+==================
The MPI standard defines a set of rules, known as tag-matching, for matching
source send operations to destination receives. The following parameters must
match the following source and destination parameters:
+
* Communicator
* User tag - wild card may be specified by the receiver
* Source rank – wild car may be specified by the receiver
* Destination rank – wild
+
The ordering rules require that when more than one pair of send and receive
message envelopes may match, the pair that includes the earliest posted-send
and the earliest posted-receive is the pair that must be used to satisfy the
@@ -35,6 +39,7 @@ the header to initiate an RDMA READ operation directly to the matching buffer.
A fin message needs to be received in order for the buffer to be reused.
Tag matching implementation
+===========================
There are two types of matching objects used, the posted receive list and the
unexpected message list. The application posts receive buffers through calls
diff --git a/Documentation/infiniband/user_mad.txt b/Documentation/infiniband/user_mad.rst
similarity index 90%
rename from Documentation/infiniband/user_mad.txt
rename to Documentation/infiniband/user_mad.rst
index 7aca13a54a3a..d88abfc0e370 100644
--- a/Documentation/infiniband/user_mad.txt
+++ b/Documentation/infiniband/user_mad.rst
@@ -1,6 +1,9 @@
-USERSPACE MAD ACCESS
+====================
+Userspace MAD access
+====================
Device files
+============
Each port of each InfiniBand device has a "umad" device and an
"issm" device attached. For example, a two-port HCA will have two
@@ -8,12 +11,13 @@ Device files
device of each type (for switch port 0).
Creating MAD agents
+===================
A MAD agent can be created by filling in a struct ib_user_mad_reg_req
and then calling the IB_USER_MAD_REGISTER_AGENT ioctl on a file
descriptor for the appropriate device file. If the registration
request succeeds, a 32-bit id will be returned in the structure.
- For example:
+ For example::
struct ib_user_mad_reg_req req = { /* ... */ };
ret = ioctl(fd, IB_USER_MAD_REGISTER_AGENT, (char *) &req);
@@ -26,12 +30,14 @@ Creating MAD agents
ioctl. Also, all agents registered through a file descriptor will
be unregistered when the descriptor is closed.
- 2014 -- a new registration ioctl is now provided which allows additional
+ 2014
+ a new registration ioctl is now provided which allows additional
fields to be provided during registration.
Users of this registration call are implicitly setting the use of
pkey_index (see below).
Receiving MADs
+==============
MADs are received using read(). The receive side now supports
RMPP. The buffer passed to read() must be at least one
@@ -41,7 +47,8 @@ Receiving MADs
MAD (RMPP), the errno is set to ENOSPC and the length of the
buffer needed is set in mad.length.
- Example for normal MAD (non RMPP) reads:
+ Example for normal MAD (non RMPP) reads::
+
struct ib_user_mad *mad;
mad = malloc(sizeof *mad + 256);
ret = read(fd, mad, sizeof *mad + 256);
@@ -50,7 +57,8 @@ Receiving MADs
free(mad);
}
- Example for RMPP reads:
+ Example for RMPP reads::
+
struct ib_user_mad *mad;
mad = malloc(sizeof *mad + 256);
ret = read(fd, mad, sizeof *mad + 256);
@@ -76,11 +84,12 @@ Receiving MADs
poll()/select() may be used to wait until a MAD can be read.
Sending MADs
+============
MADs are sent using write(). The agent ID for sending should be
filled into the id field of the MAD, the destination LID should be
filled into the lid field, and so on. The send side does support
- RMPP so arbitrary length MAD can be sent. For example:
+ RMPP so arbitrary length MAD can be sent. For example::
struct ib_user_mad *mad;
@@ -97,6 +106,7 @@ Sending MADs
perror("write");
Transaction IDs
+===============
Users of the umad devices can use the lower 32 bits of the
transaction ID field (that is, the least significant half of the
@@ -105,6 +115,7 @@ Transaction IDs
the kernel and will be overwritten before a MAD is sent.
P_Key Index Handling
+====================
The old ib_umad interface did not allow setting the P_Key index for
MADs that are sent and did not provide a way for obtaining the P_Key
@@ -119,6 +130,7 @@ P_Key Index Handling
default, and the IB_USER_MAD_ENABLE_PKEY ioctl will be removed.
Setting IsSM Capability Bit
+===========================
To set the IsSM capability bit for a port, simply open the
corresponding issm device file. If the IsSM bit is already set,
@@ -129,25 +141,26 @@ Setting IsSM Capability Bit
the issm file.
/dev files
+==========
To create the appropriate character device files automatically with
- udev, a rule like
+ udev, a rule like::
KERNEL=="umad*", NAME="infiniband/%k"
KERNEL=="issm*", NAME="infiniband/%k"
- can be used. This will create device nodes named
+ can be used. This will create device nodes named::
/dev/infiniband/umad0
/dev/infiniband/issm0
for the first port, and so on. The InfiniBand device and port
- associated with these devices can be determined from the files
+ associated with these devices can be determined from the files::
/sys/class/infiniband_mad/umad0/ibdev
/sys/class/infiniband_mad/umad0/port
- and
+ and::
/sys/class/infiniband_mad/issm0/ibdev
/sys/class/infiniband_mad/issm0/port
diff --git a/Documentation/infiniband/user_verbs.txt b/Documentation/infiniband/user_verbs.rst
similarity index 93%
rename from Documentation/infiniband/user_verbs.txt
rename to Documentation/infiniband/user_verbs.rst
index 47ebf2f80b2b..8ddc4b1cfef2 100644
--- a/Documentation/infiniband/user_verbs.txt
+++ b/Documentation/infiniband/user_verbs.rst
@@ -1,4 +1,6 @@
-USERSPACE VERBS ACCESS
+======================
+Userspace verbs access
+======================
The ib_uverbs module, built by enabling CONFIG_INFINIBAND_USER_VERBS,
enables direct userspace access to IB hardware via "verbs," as
@@ -13,6 +15,7 @@ USERSPACE VERBS ACCESS
libmthca userspace driver be installed.
User-kernel communication
+=========================
Userspace communicates with the kernel for slow path, resource
management operations via the /dev/infiniband/uverbsN character
@@ -28,6 +31,7 @@ User-kernel communication
system call.
Resource management
+===================
Since creation and destruction of all IB resources is done by
commands passed through a file descriptor, the kernel can keep track
@@ -41,6 +45,7 @@ Resource management
prevent one process from touching another process's resources.
Memory pinning
+==============
Direct userspace I/O requires that memory regions that are potential
I/O targets be kept resident at the same physical address. The
@@ -54,13 +59,14 @@ Memory pinning
number of pages pinned by a process.
/dev files
+==========
To create the appropriate character device files automatically with
- udev, a rule like
+ udev, a rule like::
KERNEL=="uverbs*", NAME="infiniband/%k"
- can be used. This will create device nodes named
+ can be used. This will create device nodes named::
/dev/infiniband/uverbs0
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 671f07ba1fad..5ecd370e018b 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -744,7 +744,7 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, void __user *arg,
"process %s did not enable P_Key index support.\n",
current->comm);
dev_warn(&file->port->dev,
- " Documentation/infiniband/user_mad.txt has info on the new ABI.\n");
+ " Documentation/infiniband/user_mad.rst has info on the new ABI.\n");
}
}
diff --git a/drivers/infiniband/ulp/ipoib/Kconfig b/drivers/infiniband/ulp/ipoib/Kconfig
index 4760ce465d89..7af68604af77 100644
--- a/drivers/infiniband/ulp/ipoib/Kconfig
+++ b/drivers/infiniband/ulp/ipoib/Kconfig
@@ -7,7 +7,7 @@ config INFINIBAND_IPOIB
transports IP packets over InfiniBand so you can use your IB
device as a fancy NIC.
- See Documentation/infiniband/ipoib.txt for more information
+ See Documentation/infiniband/ipoib.rst for more information
config INFINIBAND_IPOIB_CM
bool "IP-over-InfiniBand Connected Mode support"
--
2.21.0
^ permalink raw reply related
* Re: [PATCH RFC 00/10] RDMA/FS DAX truncate proposal
From: Ira Weiny @ 2019-06-09 1:29 UTC (permalink / raw)
To: Dave Chinner
Cc: Jason Gunthorpe, Theodore Ts'o,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard, Jeff Layton,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
linux-xfs-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
Jérôme Glisse, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
Jan Kara, linux-ext4-u79uwXL29TY76Z2rM5mHXA, Andrew Morton
In-Reply-To: <20190608001036.GF14308-pA1nmv6sEBkOM8BvhN4Z8vybgvtCy99p@public.gmane.org>
On Sat, Jun 08, 2019 at 10:10:36AM +1000, Dave Chinner wrote:
> On Fri, Jun 07, 2019 at 11:25:35AM -0700, Ira Weiny wrote:
> > On Fri, Jun 07, 2019 at 01:04:26PM +0200, Jan Kara wrote:
> > > On Thu 06-06-19 15:03:30, Ira Weiny wrote:
> > > > On Thu, Jun 06, 2019 at 12:42:03PM +0200, Jan Kara wrote:
> > > > > On Wed 05-06-19 18:45:33, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > > > > > From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > >
> > > > > So I'd like to actually mandate that you *must* hold the file lease until
> > > > > you unpin all pages in the given range (not just that you have an option to
> > > > > hold a lease). And I believe the kernel should actually enforce this. That
> > > > > way we maintain a sane state that if someone uses a physical location of
> > > > > logical file offset on disk, he has a layout lease. Also once this is done,
> > > > > sysadmin has a reasonably easy way to discover run-away RDMA application
> > > > > and kill it if he wishes so.
> > > >
> > > > Fair enough.
> > > >
> > > > I was kind of heading that direction but had not thought this far forward. I
> > > > was exploring how to have a lease remain on the file even after a "lease
> > > > break". But that is incompatible with the current semantics of a "layout"
> > > > lease (as currently defined in the kernel). [In the end I wanted to get an RFC
> > > > out to see what people think of this idea so I did not look at keeping the
> > > > lease.]
> > > >
> > > > Also hitch is that currently a lease is forcefully broken after
> > > > <sysfs>/lease-break-time. To do what you suggest I think we would need a new
> > > > lease type with the semantics you describe.
> > >
> > > I'd do what Dave suggested - add flag to mark lease as unbreakable by
> > > truncate and teach file locking core to handle that. There actually is
> > > support for locks that are not broken after given timeout so there
> > > shouldn't be too many changes need.
> > >
> > > > Previously I had thought this would be a good idea (for other reasons). But
> > > > what does everyone think about using a "longterm lease" similar to [1] which
> > > > has the semantics you proppose? In [1] I was not sure "longterm" was a good
> > > > name but with your proposal I think it makes more sense.
> > >
> > > As I wrote elsewhere in this thread I think FL_LAYOUT name still makes
> > > sense and I'd add there FL_UNBREAKABLE to mark unusal behavior with
> > > truncate.
> >
> > Ok I want to make sure I understand what you and Dave are suggesting.
> >
> > Are you suggesting that we have something like this from user space?
> >
> > fcntl(fd, F_SETLEASE, F_LAYOUT | F_UNBREAKABLE);
>
> Rather than "unbreakable", perhaps a clearer description of the
> policy it entails is "exclusive"?
>
> i.e. what we are talking about here is an exclusive lease that
> prevents other processes from changing the layout. i.e. the
> mechanism used to guarantee a lease is exclusive is that the layout
> becomes "unbreakable" at the filesystem level, but the policy we are
> actually presenting to uses is "exclusive access"...
That sounds good.
Ira
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
From: Jason Gunthorpe @ 2019-06-08 11:50 UTC (permalink / raw)
To: Ralph Campbell
Cc: Andrea Arcangeli, linux-rdma@vger.kernel.org, John Hubbard,
Felix.Kuehling@amd.com, dri-devel@lists.freedesktop.org,
linux-mm@kvack.org, Jerome Glisse, amd-gfx@lists.freedesktop.org
In-Reply-To: <20190608001452.7922-1-rcampbell@nvidia.com>
On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
> HMM defines its own struct hmm_update which is passed to the
> sync_cpu_device_pagetables() callback function. This is
> sufficient when the only action is to invalidate. However,
> a device may want to know the reason for the invalidation and
> be able to see the new permissions on a range, update device access
> rights or range statistics. Since sync_cpu_device_pagetables()
> can be called from try_to_unmap(), the mmap_sem may not be held
> and find_vma() is not safe to be called.
> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
> to allow the full invalidation information to be used.
>
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
> ---
>
> I'm sending this out now since we are updating many of the HMM APIs
> and I think it will be useful.
I agree with CH that struct hmm_update seems particularly pointless
and we really should just use mmu_notifier_range directly.
We need to find out from the DRM folks if we can merge this kind of
stuff through hmm.git and then resolve any conflicts that might arise
in DRM tree or in nouveau tree?
But I would like to see this patch go in this cycle, thanks
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
From: Jason Gunthorpe @ 2019-06-08 11:41 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrea Arcangeli, Ralph Campbell, linux-rdma@vger.kernel.org,
John Hubbard, Felix.Kuehling@amd.com,
dri-devel@lists.freedesktop.org, linux-mm@kvack.org,
Jerome Glisse, amd-gfx@lists.freedesktop.org
In-Reply-To: <20190608091008.GC32185@infradead.org>
On Sat, Jun 08, 2019 at 02:10:08AM -0700, Christoph Hellwig wrote:
> On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
> > HMM defines its own struct hmm_update which is passed to the
> > sync_cpu_device_pagetables() callback function. This is
> > sufficient when the only action is to invalidate. However,
> > a device may want to know the reason for the invalidation and
> > be able to see the new permissions on a range, update device access
> > rights or range statistics. Since sync_cpu_device_pagetables()
> > can be called from try_to_unmap(), the mmap_sem may not be held
> > and find_vma() is not safe to be called.
> > Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
> > to allow the full invalidation information to be used.
> >
> > Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
> >
> > I'm sending this out now since we are updating many of the HMM APIs
> > and I think it will be useful.
>
> This is the right thing to do. But the really right thing is to just
> kill the hmm_mirror API entirely and move to mmu_notifiers. At least
> for noveau this already is way simpler, although right now it defeats
> Jasons patch to avoid allocating the struct hmm in the fault path.
> But as said before that can be avoided by just killing struct hmm,
> which for many reasons is the right thing to do anyway.
>
> I've got a series here, which is a bit broken (epecially the last
> patch can't work as-is), but should explain where I'm trying to head:
>
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-mirror-simplification
At least the current hmm approach does rely on the collision retry
locking scheme in struct hmm/struct hmm_range for the pagefault side
to work right.
So, before we can apply patch one in this series we need to fix
hmm_vma_fault() and all its varients. Otherwise the driver will be
broken.
I'm hoping to first define what this locking should be (see other
emails to Ralph) then, ideally, see if we can extend mmu notifiers to
get it directly withouth hmm stuff.
Then we apply your patch one and the hmm ops wrapper dies.
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Jason Gunthorpe @ 2019-06-08 11:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrea Arcangeli, Ralph Campbell, linux-rdma, John Hubbard,
Felix.Kuehling, dri-devel, linux-mm, Jerome Glisse, amd-gfx
In-Reply-To: <20190608084948.GA32185@infradead.org>
On Sat, Jun 08, 2019 at 01:49:48AM -0700, Christoph Hellwig wrote:
> I still think sruct hmm should die. We already have a structure used
> for additional information for drivers having crazly tight integration
> into the VM, and it is called struct mmu_notifier_mm. We really need
> to reuse that intead of duplicating it badly.
Probably. But at least in ODP we needed something very similar to
'struct hmm' to make our mmu notifier implementation work.
The mmu notifier api really lends itself to having a per-mm structure
in the driver to hold the 'struct mmu_notifier'..
I think I see other drivers are doing things like assuming that there
is only one mm in their world (despite being FD based, so this is not
really guarenteed)
So, my first attempt would be an api something like:
priv = mmu_notififer_attach_mm(ops, current->mm, sizeof(my_priv))
mmu_notifier_detach_mm(priv);
ops->invalidate_start(struct mmu_notififer *mn):
struct p *priv = mmu_notifier_priv(mn);
Such that
- There is only one priv per mm
- All the srcu stuff is handled inside mmu notifier
- It is reference counted, so ops can be attached multiple times to
the same mm
Then odp's per_mm, and struct hmm (if we keep it at all) is simply a
'priv' in the above.
I was thinking of looking at this stuff next, once this series is
done.
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
From: Dan Carpenter @ 2019-06-08 9:22 UTC (permalink / raw)
To: Yishai Hadas
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
kernel-janitors
The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
larger than the number of bits in an int then that's undefined behavior.
It turns out this doesn't really cause an issue at runtime but it's
still nice to clean it up.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/infiniband/hw/mlx4/qp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 5221c0794d1d..9f6eb23e8044 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
struct mlx4_ib_create_qp *ucmd)
{
/* Sanity check SQ size before proceeding */
- if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
+ if (ucmd->log_sq_bb_count > 31 ||
+ (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
ucmd->log_sq_stride >
ilog2(roundup_pow_of_two(dev->dev->caps.max_sq_desc_sz)) ||
ucmd->log_sq_stride < MLX4_IB_MIN_SQ_STRIDE)
--
2.20.1
^ permalink raw reply related
* Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
From: Christoph Hellwig @ 2019-06-08 9:10 UTC (permalink / raw)
To: Ralph Campbell
Cc: Andrea Arcangeli, linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190608001452.7922-1-rcampbell-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
> HMM defines its own struct hmm_update which is passed to the
> sync_cpu_device_pagetables() callback function. This is
> sufficient when the only action is to invalidate. However,
> a device may want to know the reason for the invalidation and
> be able to see the new permissions on a range, update device access
> rights or range statistics. Since sync_cpu_device_pagetables()
> can be called from try_to_unmap(), the mmap_sem may not be held
> and find_vma() is not safe to be called.
> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
> to allow the full invalidation information to be used.
>
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
> ---
>
> I'm sending this out now since we are updating many of the HMM APIs
> and I think it will be useful.
This is the right thing to do. But the really right thing is to just
kill the hmm_mirror API entirely and move to mmu_notifiers. At least
for noveau this already is way simpler, although right now it defeats
Jasons patch to avoid allocating the struct hmm in the fault path.
But as said before that can be avoided by just killing struct hmm,
which for many reasons is the right thing to do anyway.
I've got a series here, which is a bit broken (epecially the last
patch can't work as-is), but should explain where I'm trying to head:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-mirror-simplification
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Christoph Hellwig @ 2019-06-08 8:54 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190606184438.31646-3-jgg-uk2M96/98Pc@public.gmane.org>
FYI, I very much disagree with the direction this is moving.
struct hmm_mirror literally is a trivial duplication of the
mmu_notifiers. All these drivers should just use the mmu_notifiers
directly for the mirroring part instead of building a thing wrapper
that adds nothing but helping to manage the lifetime of struct hmm,
which shouldn't exist to start with.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Christoph Hellwig @ 2019-06-08 8:49 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190606184438.31646-2-jgg-uk2M96/98Pc@public.gmane.org>
I still think sruct hmm should die. We already have a structure used
for additional information for drivers having crazly tight integration
into the VM, and it is called struct mmu_notifier_mm. We really need
to reuse that intead of duplicating it badly.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH mlx5-next 1/3] net/mlx5: Expose eswitch encap mode
From: Leon Romanovsky @ 2019-06-08 8:25 UTC (permalink / raw)
To: Petr Vorel
Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list, Maor Gottlieb,
Mark Bloch, Saeed Mahameed, linux-netdev
In-Reply-To: <20190606115635.GA30976@dell5510>
On Thu, Jun 06, 2019 at 01:56:36PM +0200, Petr Vorel wrote:
> Hi,
>
> > From: Maor Gottlieb <maorg@mellanox.com>
>
> > Add API to get the current Eswitch encap mode.
> > It will be used in downstream patches to check if
> > flow table can be created with encap support or not.
>
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thanks Petr for review, I'll add your tags and resend.
>
> Kind regards,
> Petr
^ permalink raw reply
* Re: [PATCH mlx5-next 1/3] net/mlx5: Expose eswitch encap mode
From: Leon Romanovsky @ 2019-06-08 8:25 UTC (permalink / raw)
To: Parav Pandit
Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list, Maor Gottlieb,
Mark Bloch, Saeed Mahameed, linux-netdev
In-Reply-To: <VI1PR0501MB227150ADAFACF4A5DEC26693D1170@VI1PR0501MB2271.eurprd05.prod.outlook.com>
On Thu, Jun 06, 2019 at 01:08:46PM +0000, Parav Pandit wrote:
>
>
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> > owner@vger.kernel.org> On Behalf Of Leon Romanovsky
> > Sent: Thursday, June 6, 2019 4:36 PM
> > To: Doug Ledford <dledford@redhat.com>; Jason Gunthorpe
> > <jgg@mellanox.com>
> > Cc: Leon Romanovsky <leonro@mellanox.com>; RDMA mailing list <linux-
> > rdma@vger.kernel.org>; Maor Gottlieb <maorg@mellanox.com>; Mark Bloch
> > <markb@mellanox.com>; Saeed Mahameed <saeedm@mellanox.com>;
> > linux-netdev <netdev@vger.kernel.org>
> > Subject: [PATCH mlx5-next 1/3] net/mlx5: Expose eswitch encap mode
> >
> > From: Maor Gottlieb <maorg@mellanox.com>
> >
> > Add API to get the current Eswitch encap mode.
> > It will be used in downstream patches to check if flow table can be created
> > with encap support or not.
> >
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 10 ++++++++++
> > include/linux/mlx5/eswitch.h | 10 ++++++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > index 9ea0ccfe5ef5..1da7f9569ee8 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> > @@ -2452,6 +2452,16 @@ u8 mlx5_eswitch_mode(struct mlx5_eswitch
> > *esw) } EXPORT_SYMBOL_GPL(mlx5_eswitch_mode);
> >
> > +u16 mlx5_eswitch_get_encap_mode(struct mlx5_core_dev *dev) {
>
> Encap mode as well defined devlink definition.
> So instead of u16, it should return enum devlink_eswitch_encap_mode.
>
> Since this is only reading the mode, it is better to define struct mlx5_core_dev* as const struct mlx5_core_dev *.
Thanks Parav, I'll change and resend, anyway second patch uses wrong types too.
^ permalink raw reply
* Re: [PATCH for-rc 3/3] IB/hfi1: Correct tid qp rcd to match verbs context
From: Leon Romanovsky @ 2019-06-08 8:15 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: jgg, dledford, linux-rdma, Mike Marciniszyn, stable, Kaike Wan
In-Reply-To: <20190607122538.158478.62945.stgit@awfm-01.aw.intel.com>
On Fri, Jun 07, 2019 at 08:25:38AM -0400, Dennis Dalessandro wrote:
> From: Mike Marciniszyn <mike.marciniszyn@intel.com>
>
> The qp priv rcd pointer doesn't match the context being
> used for verbs causing issues when 9B and kdeth packets
> are processed by different receive contexts and hence
> different CPUs.
>
> When running on different CPUs the following panic can
> occur:
> [476262.398106] WARNING: CPU: 3 PID: 2584 at lib/list_debug.c:59 __list_del_entry+0xa1/0xd0
> [476262.398109] list_del corruption. prev->next should be ffff9a7ac31f7a30, but was ffff9a7c3bc89230
> [476262.398266] CPU: 3 PID: 2584 Comm: z_wr_iss Kdump: loaded Tainted: P OE ------------ 3.10.0-862.2.3.el7_lustre.x86_64 #1
> [476262.398272] Call Trace:
> [476262.398277] <IRQ> [<ffffffffb7b0d78e>] dump_stack+0x19/0x1b
> [476262.398314] [<ffffffffb74916d8>] __warn+0xd8/0x100
> [476262.398317] [<ffffffffb749175f>] warn_slowpath_fmt+0x5f/0x80
> [476262.398320] [<ffffffffb7768671>] __list_del_entry+0xa1/0xd0
> [476262.398402] [<ffffffffc0c7a945>] process_rcv_qp_work+0xb5/0x160 [hfi1]
> [476262.398424] [<ffffffffc0c7bc2b>] handle_receive_interrupt_nodma_rtail+0x20b/0x2b0 [hfi1]
> [476262.398438] [<ffffffffc0c70683>] receive_context_interrupt+0x23/0x40 [hfi1]
> [476262.398447] [<ffffffffb7540a94>] __handle_irq_event_percpu+0x44/0x1c0
> [476262.398450] [<ffffffffb7540c42>] handle_irq_event_percpu+0x32/0x80
> [476262.398454] [<ffffffffb7540ccc>] handle_irq_event+0x3c/0x60
> [476262.398460] [<ffffffffb7543a1f>] handle_edge_irq+0x7f/0x150
> [476262.398469] [<ffffffffb742d504>] handle_irq+0xe4/0x1a0
> [476262.398475] [<ffffffffb7b23f7d>] do_IRQ+0x4d/0xf0
> [476262.398481] [<ffffffffb7b16362>] common_interrupt+0x162/0x162
> [476262.398482] <EOI> [<ffffffffb775a326>] ? memcpy+0x6/0x110
> [476262.398645] [<ffffffffc109210d>] ? abd_copy_from_buf_off_cb+0x1d/0x30 [zfs]
> [476262.398678] [<ffffffffc10920f0>] ? abd_copy_to_buf_off_cb+0x30/0x30 [zfs]
> [476262.398696] [<ffffffffc1093257>] abd_iterate_func+0x97/0x120 [zfs]
> [476262.398710] [<ffffffffc10934d9>] abd_copy_from_buf_off+0x39/0x60 [zfs]
> [476262.398726] [<ffffffffc109b828>] arc_write_ready+0x178/0x300 [zfs]
> [476262.398732] [<ffffffffb7b11032>] ? mutex_lock+0x12/0x2f
> [476262.398734] [<ffffffffb7b11032>] ? mutex_lock+0x12/0x2f
> [476262.398837] [<ffffffffc1164d05>] zio_ready+0x65/0x3d0 [zfs]
> [476262.398884] [<ffffffffc04d725e>] ? tsd_get_by_thread+0x2e/0x50 [spl]
> [476262.398893] [<ffffffffc04d1318>] ? taskq_member+0x18/0x30 [spl]
> [476262.398968] [<ffffffffc115ef22>] zio_execute+0xa2/0x100 [zfs]
> [476262.398982] [<ffffffffc04d1d2c>] taskq_thread+0x2ac/0x4f0 [spl]
> [476262.399001] [<ffffffffb74cee80>] ? wake_up_state+0x20/0x20
> [476262.399043] [<ffffffffc115ee80>] ? zio_taskq_member.isra.7.constprop.10+0x80/0x80 [zfs]
> [476262.399055] [<ffffffffc04d1a80>] ? taskq_thread_spawn+0x60/0x60 [spl]
> [476262.399067] [<ffffffffb74bae31>] kthread+0xd1/0xe0
> [476262.399072] [<ffffffffb74bad60>] ? insert_kthread_work+0x40/0x40
> [476262.399082] [<ffffffffb7b1f5f7>] ret_from_fork_nospec_begin+0x21/0x21
> [476262.399087] [<ffffffffb74bad60>] ? insert_kthread_work+0x40/0x40
>
> Fix by reading the map entry in the same manner as the
> hardware so that the kdeth and verbs contexts match.
>
> Fixes: 5190f052a365 ("IB/hfi1: Allow the driver to initialize QP priv struct")
> Cc: <stable@vger.kernel.org>
> Reviewed-by: Kaike Wan <kaike.wan@intel.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
> drivers/infiniband/hw/hfi1/chip.c | 13 +++++++++++++
> drivers/infiniband/hw/hfi1/chip.h | 1 +
> drivers/infiniband/hw/hfi1/tid_rdma.c | 5 ++---
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
> index 4221a99e..674f62a 100644
> --- a/drivers/infiniband/hw/hfi1/chip.c
> +++ b/drivers/infiniband/hw/hfi1/chip.c
> @@ -14032,6 +14032,19 @@ static void init_kdeth_qp(struct hfi1_devdata *dd)
> }
>
> /**
> + * hfi1_get_qp_map
> + * @dd: device data
> + * @idx: index to read
> + */
> +u8 hfi1_get_qp_map(struct hfi1_devdata *dd, u8 idx)
> +{
> + u64 reg = read_csr(dd, RCV_QP_MAP_TABLE + (idx / 8) * 8);
> +
> + reg >>= (idx % 8) * 8;
> + return (u8)reg;
> +}
> +
> +/**
> * init_qpmap_table
> * @dd - device data
> * @first_ctxt - first context
> diff --git a/drivers/infiniband/hw/hfi1/chip.h b/drivers/infiniband/hw/hfi1/chip.h
> index 4e6c355..b76cf81 100644
> --- a/drivers/infiniband/hw/hfi1/chip.h
> +++ b/drivers/infiniband/hw/hfi1/chip.h
> @@ -1445,6 +1445,7 @@ int hfi1_set_ctxt_pkey(struct hfi1_devdata *dd, struct hfi1_ctxtdata *ctxt,
> void remap_intr(struct hfi1_devdata *dd, int isrc, int msix_intr);
> void remap_sdma_interrupts(struct hfi1_devdata *dd, int engine, int msix_intr);
> void reset_interrupts(struct hfi1_devdata *dd);
> +u8 hfi1_get_qp_map(struct hfi1_devdata *dd, u8 idx);
>
> /*
> * Interrupt source table.
> diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c b/drivers/infiniband/hw/hfi1/tid_rdma.c
> index 6fb9303..d77276d 100644
> --- a/drivers/infiniband/hw/hfi1/tid_rdma.c
> +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
> @@ -312,9 +312,8 @@ static struct hfi1_ctxtdata *qp_to_rcd(struct rvt_dev_info *rdi,
> if (qp->ibqp.qp_num == 0)
> ctxt = 0;
> else
> - ctxt = ((qp->ibqp.qp_num >> dd->qos_shift) %
> - (dd->n_krcv_queues - 1)) + 1;
> -
> + ctxt = hfi1_get_qp_map(dd,
> + (u8)(qp->ibqp.qp_num >> dd->qos_shift));
It is one time use functions, why don't you handle this (u8) casting
inside of hfi1_get_qp_map()?
Thanks
> return dd->rcd[ctxt];
> }
>
>
^ permalink raw reply
* Re: [PATCH 10/13] megaraid_sas: set virt_boundary_mask in the scsi host
From: Christoph Hellwig @ 2019-06-08 8:14 UTC (permalink / raw)
To: Kashyap Desai
Cc: Christoph Hellwig, Jens Axboe, Sebastian Ott, Sagi Grimberg,
Max Gurtovoy, Bart Van Assche, Ulf Hansson, Alan Stern,
Oliver Neukum, linux-block, linux-rdma, linux-mmc, linux-nvme,
linux-scsi, PDL,MEGARAIDLINUX, PDL-MPT-FUSIONLINUX, linux-hyperv,
linux-usb, usb-storage, linux-kernel
In-Reply-To: <cd713506efb9579d1f69a719d831c28d@mail.gmail.com>
On Thu, Jun 06, 2019 at 09:07:27PM +0530, Kashyap Desai wrote:
> Hi Christoph, Changes for <megaraid_sas> and <mpt3sas> looks good. We want
> to confirm few sanity before ACK. BTW, what benefit we will see moving
> virt_boundry setting to SCSI mid layer ? Is it just modular approach OR any
> functional fix ?
The big difference is that virt_boundary now also changes the
max_segment_size, and this ensures that this limit is also communicated
to the DMA mapping layer.
^ permalink raw reply
* Re: properly communicate queue limits to the DMA layer
From: Jens Axboe @ 2019-06-08 8:10 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Sebastian Ott, Sagi Grimberg, Max Gurtovoy,
Bart Van Assche, Ulf Hansson, Alan Stern, Oliver Neukum,
linux-block, linux-rdma, linux-mmc, linux-nvme, linux-scsi,
megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv, linux-usb,
usb-storage, linux-kernel
In-Reply-To: <yq1o939i9qh.fsf@oracle.com>
On 6/7/19 11:30 AM, Martin K. Petersen wrote:
>
> Jens,
>
>>> The SCSI bits will need a bit more review, and possibly tweaking
>>> fo megaraid and mpt3sas. But they are really independent of the
>>> other patches, so maybe skip them for now and leave them for Martin
>>> to deal with.
>>
>> I dropped the SCSI bits.
>
> I'll monitor and merge them.
Great, thanks Martin.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v16 14/16] tee, arm64: untag user pointers in tee_shm_register
From: Kees Cook @ 2019-06-08 4:05 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <dc3f3092abbc0d48e51b2e2a2ca8f4c4f69fa0f4.1559580831.git.andreyknvl@google.com>
On Mon, Jun 03, 2019 at 06:55:16PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided
> user pointers for vma lookups (via __check_mem_type()), which can only by
> done with untagged pointers.
>
> Untag user pointers in this function.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
"tee: shm: untag user pointers in tee_shm_register"
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/tee/tee_shm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 49fd7312e2aa..96945f4cefb8 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -263,6 +263,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
> shm->teedev = teedev;
> shm->ctx = ctx;
> shm->id = -1;
> + addr = untagged_addr(addr);
> start = rounddown(addr, PAGE_SIZE);
> shm->offset = addr - start;
> shm->size = length;
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v16 09/16] fs, arm64: untag user pointers in fs/userfaultfd.c
From: Kees Cook @ 2019-06-08 4:03 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <7d6fef00d7daf647b5069101da8cf5a202da75b0.1559580831.git.andreyknvl@google.com>
On Mon, Jun 03, 2019 at 06:55:11PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> userfaultfd code use provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in validate_range().
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
"userfaultfd: untag user pointers"
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> fs/userfaultfd.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 3b30301c90ec..24d68c3b5ee2 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1263,21 +1263,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
> }
>
> static __always_inline int validate_range(struct mm_struct *mm,
> - __u64 start, __u64 len)
> + __u64 *start, __u64 len)
> {
> __u64 task_size = mm->task_size;
>
> - if (start & ~PAGE_MASK)
> + *start = untagged_addr(*start);
> +
> + if (*start & ~PAGE_MASK)
> return -EINVAL;
> if (len & ~PAGE_MASK)
> return -EINVAL;
> if (!len)
> return -EINVAL;
> - if (start < mmap_min_addr)
> + if (*start < mmap_min_addr)
> return -EINVAL;
> - if (start >= task_size)
> + if (*start >= task_size)
> return -EINVAL;
> - if (len > task_size - start)
> + if (len > task_size - *start)
> return -EINVAL;
> return 0;
> }
> @@ -1327,7 +1329,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> goto out;
> }
>
> - ret = validate_range(mm, uffdio_register.range.start,
> + ret = validate_range(mm, &uffdio_register.range.start,
> uffdio_register.range.len);
> if (ret)
> goto out;
> @@ -1516,7 +1518,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
> goto out;
>
> - ret = validate_range(mm, uffdio_unregister.start,
> + ret = validate_range(mm, &uffdio_unregister.start,
> uffdio_unregister.len);
> if (ret)
> goto out;
> @@ -1667,7 +1669,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
> if (ret)
> goto out;
>
> @@ -1707,7 +1709,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> sizeof(uffdio_copy)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
> if (ret)
> goto out;
> /*
> @@ -1763,7 +1765,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
> sizeof(uffdio_zeropage)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
> uffdio_zeropage.range.len);
> if (ret)
> goto out;
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v16 08/16] fs, arm64: untag user pointers in copy_mount_options
From: Kees Cook @ 2019-06-08 4:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <51f44a12c4e81c9edea8dcd268f820f5d1fad87c.1559580831.git.andreyknvl@google.com>
On Mon, Jun 03, 2019 at 06:55:10PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> In copy_mount_options a user address is being subtracted from TASK_SIZE.
> If the address is lower than TASK_SIZE, the size is calculated to not
> allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
> However if the address is tagged, then the size will be calculated
> incorrectly.
>
> Untag the address before subtracting.
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
One thing I just noticed in the commit titles... "arm64" is in the
prefix, but these are arch-indep areas. Should the ", arm64" be left
out?
I would expect, instead:
fs/namespace: untag user pointers in copy_mount_options
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> fs/namespace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index b26778bdc236..2e85712a19ed 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2993,7 +2993,7 @@ void *copy_mount_options(const void __user * data)
> * the remainder of the page.
> */
> /* copy_from_user cannot cross TASK_SIZE ! */
> - size = TASK_SIZE - (unsigned long)data;
> + size = TASK_SIZE - (unsigned long)untagged_addr(data);
> if (size > PAGE_SIZE)
> size = PAGE_SIZE;
>
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v16 07/16] mm, arm64: untag user pointers in get_vaddr_frames
From: Kees Cook @ 2019-06-08 4:00 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <da1d0e0f6d69c15a12987379e372182f416cbc02.1559580831.git.andreyknvl@google.com>
On Mon, Jun 03, 2019 at 06:55:09PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> get_vaddr_frames uses provided user pointers for vma lookups, which can
> only by done with untagged pointers. Instead of locating and changing
> all callers of this function, perform untagging in it.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> mm/frame_vector.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index c64dca6e27c2..c431ca81dad5 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -46,6 +46,8 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
> nr_frames = vec->nr_allocated;
>
> + start = untagged_addr(start);
> +
> down_read(&mm->mmap_sem);
> locked = 1;
> vma = find_vma_intersection(mm, start, start + 1);
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox