* Re: [PATCH][next] IB/qib: Use struct_size() helper
From: Dennis Dalessandro @ 2019-05-30 18:29 UTC (permalink / raw)
To: Gustavo A. R. Silva, Mike Marciniszyn, Doug Ledford,
Jason Gunthorpe
Cc: linux-rdma, linux-kernel
In-Reply-To: <20190529151326.GA24109@embeddedor>
On 5/29/2019 11:13 AM, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
>
> So, replace the following form:
>
> sizeof(*pkt) + sizeof(pkt->addr[0])*n
>
> with:
>
> struct_size(pkt, addr, n)
>
> Also, notice that variable size is unnecessary, hence it is removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/infiniband/hw/qib/qib_user_sdma.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qib/qib_user_sdma.c b/drivers/infiniband/hw/qib/qib_user_sdma.c
> index 0c204776263f..97649f64e09e 100644
> --- a/drivers/infiniband/hw/qib/qib_user_sdma.c
> +++ b/drivers/infiniband/hw/qib/qib_user_sdma.c
> @@ -904,10 +904,11 @@ static int qib_user_sdma_queue_pkts(const struct qib_devdata *dd,
> }
>
> if (frag_size) {
> - int pktsize, tidsmsize, n;
> + int tidsmsize, n;
> + size_t pktsize;
>
> n = npages*((2*PAGE_SIZE/frag_size)+1);
> - pktsize = sizeof(*pkt) + sizeof(pkt->addr[0])*n;
> + pktsize = struct_size(pkt, addr, n);
>
> /*
> * Determine if this is tid-sdma or just sdma.
>
Again, same minor objection but correct patch none the less.
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
^ permalink raw reply
* Re: [PATCH][next] IB/rdmavt: Use struct_size() helper
From: Jason Gunthorpe @ 2019-05-30 18:29 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: Gustavo A. R. Silva, Mike Marciniszyn, Doug Ledford, linux-rdma,
linux-kernel
In-Reply-To: <09563aec-f6d0-f27f-4f67-1e21cebd997c@intel.com>
On Thu, May 30, 2019 at 02:26:22PM -0400, Dennis Dalessandro wrote:
> On 5/29/2019 11:12 AM, Gustavo A. R. Silva wrote:
> > Make use of the struct_size() helper instead of an open-coded version
> > in order to avoid any potential type mistakes, in particular in the
> > context in which this code is being used.
> >
> > So, replace the following form:
> >
> > sizeof(struct rvt_sge) * init_attr->cap.max_send_sge + sizeof(struct rvt_swqe)
> >
> > with:
> >
> > struct_size(swq, sg_list, init_attr->cap.max_send_sge)
> >
> > and so on...
> >
> > Also, notice that variable size is unnecessary, hence it is removed.
> >
> > This code was detected with the help of Coccinelle.
> >
> > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> > drivers/infiniband/sw/rdmavt/qp.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> > index 31a2e65e4906..a60f5faea198 100644
> > +++ b/drivers/infiniband/sw/rdmavt/qp.c
> > @@ -988,9 +988,7 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> > case IB_QPT_UC:
> > case IB_QPT_RC:
> > case IB_QPT_UD:
> > - sz = sizeof(struct rvt_sge) *
> > - init_attr->cap.max_send_sge +
> > - sizeof(struct rvt_swqe);
> > + sz = struct_size(swq, sg_list, init_attr->cap.max_send_sge);
> > swq = vzalloc_node(array_size(sz, sqsize), rdi->dparms.node);
> > if (!swq)
> > return ERR_PTR(-ENOMEM);
> >
>
> Looks correct, I don't think this makes the code easier to read though. The
> macro name "struct_size" is misleading to me. Maybe that's just me, and in
> any case...
It is struct_size because it computes the full size of a struct ending in a
variable length array with N items.
The previous code was wonky because it assumes that struct rvt_swqe is
the underlying type of sg_list, while the new version gets the type
information automatically.
Jason
^ permalink raw reply
* Re: [PATCH][next] IB/rdmavt: Use struct_size() helper
From: Dennis Dalessandro @ 2019-05-30 18:26 UTC (permalink / raw)
To: Gustavo A. R. Silva, Mike Marciniszyn, Doug Ledford,
Jason Gunthorpe
Cc: linux-rdma, linux-kernel
In-Reply-To: <20190529151248.GA24080@embeddedor>
On 5/29/2019 11:12 AM, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
>
> So, replace the following form:
>
> sizeof(struct rvt_sge) * init_attr->cap.max_send_sge + sizeof(struct rvt_swqe)
>
> with:
>
> struct_size(swq, sg_list, init_attr->cap.max_send_sge)
>
> and so on...
>
> Also, notice that variable size is unnecessary, hence it is removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/infiniband/sw/rdmavt/qp.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> index 31a2e65e4906..a60f5faea198 100644
> --- a/drivers/infiniband/sw/rdmavt/qp.c
> +++ b/drivers/infiniband/sw/rdmavt/qp.c
> @@ -988,9 +988,7 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> case IB_QPT_UC:
> case IB_QPT_RC:
> case IB_QPT_UD:
> - sz = sizeof(struct rvt_sge) *
> - init_attr->cap.max_send_sge +
> - sizeof(struct rvt_swqe);
> + sz = struct_size(swq, sg_list, init_attr->cap.max_send_sge);
> swq = vzalloc_node(array_size(sz, sqsize), rdi->dparms.node);
> if (!swq)
> return ERR_PTR(-ENOMEM);
>
Looks correct, I don't think this makes the code easier to read though.
The macro name "struct_size" is misleading to me. Maybe that's just me,
and in any case...
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
^ permalink raw reply
* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Catalin Marinas @ 2019-05-30 17:15 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Kees Cook, Evgenii Stepanov, Linux ARM,
Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher
In-Reply-To: <CAAeHK+yAUsZWhp6xPAbWewX5Nbw+-G3svUyPmhXu5MVeEDKYvA@mail.gmail.com>
On Tue, May 28, 2019 at 04:14:45PM +0200, Andrey Konovalov wrote:
> Thanks for a lot of valuable input! I've read through all the replies
> and got somewhat lost. What are the changes I need to do to this
> series?
>
> 1. Should I move untagging for memory syscalls back to the generic
> code so other arches would make use of it as well, or should I keep
> the arm64 specific memory syscalls wrappers and address the comments
> on that patch?
Keep them generic again but make sure we get agreement with Khalid on
the actual ABI implications for sparc.
> 2. Should I make untagging opt-in and controlled by a command line argument?
Opt-in, yes, but per task rather than kernel command line option.
prctl() is a possibility of opting in.
> 3. Should I "add Documentation/core-api/user-addresses.rst to describe
> proper care and handling of user space pointers with untagged_addr(),
> with examples based on all the cases seen so far in this series"?
> Which examples specifically should it cover?
I think we can leave 3 for now as not too urgent. What I'd like is for
Vincenzo's TBI user ABI document to go into a more common place since we
can expand it to cover both sparc and arm64. We'd need an arm64-specific
doc as well for things like prctl() and later MTE that sparc may support
differently.
--
Catalin
^ permalink raw reply
* Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
From: Catalin Marinas @ 2019-05-30 16:57 UTC (permalink / raw)
To: Khalid Aziz
Cc: Andrew Murray, Andrey Konovalov, Mark Rutland, kvm, Szabolcs Nagy,
Will Deacon, dri-devel, linux-mm, linux-kselftest, Felix Kuehling,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov,
linux-media, Kevin Brodsky, Kees Cook,
Ruben Ayrapetyan <Ruben.Ayrapetyan@
In-Reply-To: <f79336b5-46b4-39c0-b754-23366207e32d@oracle.com>
On Thu, May 30, 2019 at 10:05:55AM -0600, Khalid Aziz wrote:
> On 5/30/19 9:11 AM, Catalin Marinas wrote:
> > So if a database program is doing an anonymous mmap(PROT_TBI) of 100GB,
> > IIUC for sparc the faulted-in pages will have random colours (on 64-byte
> > granularity). Ignoring the information leak from prior uses of such
> > pages, it would be the responsibility of the db program to issue the
> > stxa. On arm64, since we also want to do this via malloc(), any large
> > allocation would require all pages to be faulted in so that malloc() can
> > set the write colour before being handed over to the user. That's what
> > we want to avoid and the user is free to repaint the memory as it likes.
>
> On sparc, any newly allocated page is cleared along with any old tags on
> it. Since clearing tag happens automatically when page is cleared on
> sparc, clear_user_page() will need to execute additional stxa
> instructions to set a new tag. It is doable. In a way it is done already
> if page is being pre-colored with tag 0 always ;)
Ah, good to know. On arm64 we'd have to use different instructions,
although the same loop.
> Where would the pre-defined tag be stored - as part of address stored
> in vm_start or a new field in vm_area_struct?
I think we can discuss the details when we post the actual MTE patches.
In our internal hack we overloaded the VM_HIGH_ARCH_* flags and selected
CONFIG_ARCH_USES_HIGH_VMA_FLAGS (used for pkeys on x86).
For the time being, I'd rather restrict tagged addresses passed to
mmap() until we agreed that they have any meaning. If we allowed them
now but get ignored (though probably no-one would be doing this), I feel
it's slightly harder to change the semantics afterwards.
Thanks.
--
Catalin
^ permalink raw reply
* Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
From: Khalid Aziz @ 2019-05-30 16:05 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, kvm, Christian Koenig, Szabolcs Nagy, Will Deacon,
dri-devel, linux-mm, Lee Smith, linux-kselftest,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, linux-arm-kernel, Dave Martin, Evgeniy Stepanov,
linux-media, Kees Cook, Ruben Ayrapetyan, Andrey Konovalov,
Kevin Brodsky, Alex Williamson, Mauro Carvalho Chehab
In-Reply-To: <20190530151105.GA35418@arrakis.emea.arm.com>
On 5/30/19 9:11 AM, Catalin Marinas wrote:
> On Wed, May 29, 2019 at 01:16:37PM -0600, Khalid Aziz wrote:
>> mmap() can return the same tagged address but I am uneasy about kernel
>> pre-coloring the pages. Database can mmap 100's of GB of memory. That is
>> lot of work being offloaded to the kernel to pre-color the page even if
>> done in batches as pages are faulted in.
>
> For anonymous mmap() for example, the kernel would have to zero the
> faulted in pages anyway. We can handle the colouring at the same time in
> clear_user_page() (as I said below, we have to clear the colour anyway
> from previous uses, so it's simply extending this to support something
> other than tag/colour 0 by default with no additional overhead).
>
On sparc M7, clear_user_page() ends up in M7clear_user_page defined in
arch/sparc/lib/M7memset.S. M7 code use Block Init Store (BIS) to clear
the page. BIS on M7 clears the memory tags as well and no separate
instructions are needed to clear the tags. As a result when kernel
clears a page before returning it to user, the page is not only zeroed
out, its tags are also cleared to 0.
>>> Since we already need such loop in the kernel, we might as well allow
>>> user space to require a certain colour. This comes in handy for large
>>> malloc() and another advantage is that the C library won't be stuck
>>> trying to paint the whole range (think GB).
>>
>> If kernel is going to pre-color all pages in a vma, we will need to
>> store the default tag in the vma. It will add more time to page fault
>> handling code. On sparc M7, kernel will need to execute additional 128
>> stxa instructions to set the tags on a normal page.
>
> As I said, since the user can retrieve an old colour using ldxa, the
> kernel should perform this operation anyway on any newly allocated page
> (unless you clear the existing colour on page freeing).>
Tags are not cleared on sparc on freeing. They get cleared when the page
is allocated again.
>>>> We can try to store tags for an entire region in vma but that is
>>>> expensive, plus on sparc tags are set in userspace with no
>>>> participation from kernel and now we need a way for userspace to
>>>> communicate the tags to kernel.
>>>
>>> We can't support finer granularity through the mmap() syscall and, as
>>> you said, the vma is not the right thing to store the individual tags.
>>> With the above extension to mmap(), we'd have to store a colour per vma
>>> and prevent merging if different colours (we could as well use the
>>> pkeys mechanism we already have in the kernel but use a colour per vma
>>> instead of a key).
>>
>> Since tags can change on any part of mmap region on sparc at any time
>> without kernel being involved, I am not sure I see much reason for
>> kernel to enforce any tag related restrictions.
>
> It's not enforcing a tag, more like the default colour for a faulted in
> page. Anyway, if sparc is going with default 0/untagged, that's fine as
> well. We may add this mmap() option to arm64 only.
>
>>>> From sparc point of view, making kernel responsible for assigning tags
>>>> to a page on page fault is full of pitfalls.
>>>
>>> This could be just some arm64-specific but if you plan to deploy it more
>>> generically for sparc (at the C library level), you may find this
>>> useful.
>>
>> Common semantics from app developer point of view will be very useful to
>> maintain. If arm64 says mmap with MAP_FIXED and a tagged address will
>> return a pre-colored page, I would rather have it be the same on any
>> architecture. Is there a use case that justifies kernel doing this extra
>> work?
>
> So if a database program is doing an anonymous mmap(PROT_TBI) of 100GB,
> IIUC for sparc the faulted-in pages will have random colours (on 64-byte
> granularity). Ignoring the information leak from prior uses of such
> pages, it would be the responsibility of the db program to issue the
> stxa. On arm64, since we also want to do this via malloc(), any large
> allocation would require all pages to be faulted in so that malloc() can
> set the write colour before being handed over to the user. That's what
> we want to avoid and the user is free to repaint the memory as it likes.
>
On sparc, any newly allocated page is cleared along with any old tags on
it. Since clearing tag happens automatically when page is cleared on
sparc, clear_user_page() will need to execute additional stxa
instructions to set a new tag. It is doable. In a way it is done already
if page is being pre-colored with tag 0 always ;) Where would the
pre-defined tag be stored - as part of address stored in vm_start or a
new field in vm_area_struct?
--
Khalid
^ permalink raw reply
* Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
From: Catalin Marinas @ 2019-05-30 15:11 UTC (permalink / raw)
To: Khalid Aziz
Cc: Andrew Murray, Andrey Konovalov, Mark Rutland, kvm, Szabolcs Nagy,
Will Deacon, dri-devel, linux-mm, linux-kselftest, Felix Kuehling,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov,
linux-media, Kevin Brodsky, Kees Cook,
Ruben Ayrapetyan <Ruben.Ayrapetyan@
In-Reply-To: <b2753e81-7b57-481f-0095-3c6fecb1a74c@oracle.com>
On Wed, May 29, 2019 at 01:16:37PM -0600, Khalid Aziz wrote:
> On 5/29/19 8:20 AM, Catalin Marinas wrote:
> > On Tue, May 28, 2019 at 05:33:04PM -0600, Khalid Aziz wrote:
> >> Steps 1 and 2 are accomplished by userspace by calling mprotect() with
> >> PROT_ADI. Tags are set by storing tags in a loop, for example:
> >>
> >> version = 10;
> >> tmp_addr = shmaddr;
> >> end = shmaddr + BUFFER_SIZE;
> >> while (tmp_addr < end) {
> >> asm volatile(
> >> "stxa %1, [%0]0x90\n\t"
> >> :
> >> : "r" (tmp_addr), "r" (version));
> >> tmp_addr += adi_blksz;
> >> }
> >
> > On arm64, a sequence similar to the above would live in the libc. So a
> > malloc() call will tag the memory and return the tagged address to thePre-coloring could easily be done by
> > user.
> >
> > We were not planning for a PROT_ADI/MTE but rather have MTE enabled for
> > all user memory ranges. We may revisit this before we upstream the MTE
> > support (probably some marginal benefit for the hardware not fetching
> > the tags from memory if we don't need to, e.g. code sections).
> >
> > Given that we already have the TBI feature and with MTE enabled the top
> > byte is no longer ignored, we are planning for an explicit opt-in by the
> > user via prctl() to enable MTE.
>
> OK. I had initially proposed enabling ADI for a process using prctl().
> Feedback I got was prctl was not a desirable interface and I ended up
> making mprotect() with PROT_ADI enable ADI on the process instead. Just
> something to keep in mind.
Thanks for the feedback. We'll keep this in mind when adding MTE
support. In the way we plan to deploy this, it would be a libc decision
to invoke the mmap() with the right flag.
This could actually simplify the automatic page faulting below brk(),
basically no tagged/coloured memory allowed implicitly. It needs
feedback from the bionic/glibc folk.
> >> With these semantics, giving mmap() or shamat() a tagged address is
> >> meaningless since no tags have been stored at the addresses mmap() will
> >> allocate and one can not store tags before memory range has been
> >> allocated. If we choose to allow tagged addresses to come into mmap()
> >> and shmat(), sparc code can strip the tags unconditionally and that may
> >> help simplify ABI and/or code.
> >
> > We could say that with TBI (pre-MTE support), the top byte is actually
> > ignored on mmap(). Now, if you pass a MAP_FIXED with a tagged address,
> > should the user expect the same tagged address back or stripping the tag
> > is acceptable? If we want to keep the current mmap() semantics, I'd say
> > the same tag is returned. However, with MTE this also implies that the
> > memory was coloured.
>
> Is assigning a tag aprivileged operationon ARM64? I am thinking not
> since you mentioned libc could do it in a loop for malloc'd memory.
Indeed it's not, the user can do it.
> mmap() can return the same tagged address but I am uneasy about kernel
> pre-coloring the pages. Database can mmap 100's of GB of memory. That is
> lot of work being offloaded to the kernel to pre-color the page even if
> done in batches as pages are faulted in.
For anonymous mmap() for example, the kernel would have to zero the
faulted in pages anyway. We can handle the colouring at the same time in
clear_user_page() (as I said below, we have to clear the colour anyway
from previous uses, so it's simply extending this to support something
other than tag/colour 0 by default with no additional overhead).
> > Since the user can probe the pre-existing colour in a faulted-in page
> > (either with some 'ldxa' instruction or by performing a tag-checked
> > access), the kernel should always pre-colour (even if colour 0) any
> > allocated page. There might not be an obvious security risk but I feel
> > uneasy about letting colours leak between address spaces (different user
> > processes or between kernel and user).
>
> On sparc, tags 0 and 15 are special in that 0 means untagged memory and
> 15 means match any tag in the address. Colour 0 is the default for any
> newly faulted in page on sparc.
With MTE we don't have match-all/any tag in memory, only in the virtual
address/pointer. So if we turn on MTE for all pages and the user
accesses an address with a 0 tag, the underlying memory needs to be
coloured with the same 0 value.
> > Since we already need such loop in the kernel, we might as well allow
> > user space to require a certain colour. This comes in handy for large
> > malloc() and another advantage is that the C library won't be stuck
> > trying to paint the whole range (think GB).
>
> If kernel is going to pre-color all pages in a vma, we will need to
> store the default tag in the vma. It will add more time to page fault
> handling code. On sparc M7, kernel will need to execute additional 128
> stxa instructions to set the tags on a normal page.
As I said, since the user can retrieve an old colour using ldxa, the
kernel should perform this operation anyway on any newly allocated page
(unless you clear the existing colour on page freeing).
> >> We can try to store tags for an entire region in vma but that is
> >> expensive, plus on sparc tags are set in userspace with no
> >> participation from kernel and now we need a way for userspace to
> >> communicate the tags to kernel.
> >
> > We can't support finer granularity through the mmap() syscall and, as
> > you said, the vma is not the right thing to store the individual tags.
> > With the above extension to mmap(), we'd have to store a colour per vma
> > and prevent merging if different colours (we could as well use the
> > pkeys mechanism we already have in the kernel but use a colour per vma
> > instead of a key).
>
> Since tags can change on any part of mmap region on sparc at any time
> without kernel being involved, I am not sure I see much reason for
> kernel to enforce any tag related restrictions.
It's not enforcing a tag, more like the default colour for a faulted in
page. Anyway, if sparc is going with default 0/untagged, that's fine as
well. We may add this mmap() option to arm64 only.
> >> From sparc point of view, making kernel responsible for assigning tags
> >> to a page on page fault is full of pitfalls.
> >
> > This could be just some arm64-specific but if you plan to deploy it more
> > generically for sparc (at the C library level), you may find this
> > useful.
>
> Common semantics from app developer point of view will be very useful to
> maintain. If arm64 says mmap with MAP_FIXED and a tagged address will
> return a pre-colored page, I would rather have it be the same on any
> architecture. Is there a use case that justifies kernel doing this extra
> work?
So if a database program is doing an anonymous mmap(PROT_TBI) of 100GB,
IIUC for sparc the faulted-in pages will have random colours (on 64-byte
granularity). Ignoring the information leak from prior uses of such
pages, it would be the responsibility of the db program to issue the
stxa. On arm64, since we also want to do this via malloc(), any large
allocation would require all pages to be faulted in so that malloc() can
set the write colour before being handed over to the user. That's what
we want to avoid and the user is free to repaint the memory as it likes.
--
Catalin
^ permalink raw reply
* Re: [PATCH] IB/mlx5: Limit to 64-bit builds
From: Guenter Roeck @ 2019-05-30 12:37 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
Ariel Levkovich
In-Reply-To: <20190530122251.GD6251@mtr-leonro.mtl.com>
On 5/30/19 5:22 AM, Leon Romanovsky wrote:
> On Thu, May 30, 2019 at 04:35:44AM -0700, Guenter Roeck wrote:
>> 32-bit builds fail with errors such as
>>
>> ERROR: "__udivdi3" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!
>
> It is fixed in rdma-rc.
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/jgg-for-rc&id=37eb86c4507abcb14fc346863e83aa8751aa4675
>
Excellent, and sorry for the noise.
Guenter
> Thanks
>
>>
>> Fixes: 25c13324d03d ("IB/mlx5: Add steering SW ICM device memory type")
>> Cc: Ariel Levkovich <lariel@mellanox.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/infiniband/hw/mlx5/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/Kconfig b/drivers/infiniband/hw/mlx5/Kconfig
>> index ea248def4556..574b97da7a43 100644
>> --- a/drivers/infiniband/hw/mlx5/Kconfig
>> +++ b/drivers/infiniband/hw/mlx5/Kconfig
>> @@ -1,7 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> config MLX5_INFINIBAND
>> tristate "Mellanox 5th generation network adapters (ConnectX series) support"
>> - depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
>> + depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE && 64BIT
>> ---help---
>> This driver provides low-level InfiniBand support for
>> Mellanox Connect-IB PCI Express host channel adapters (HCAs).
>> --
>> 2.7.4
>>
>
^ permalink raw reply
* Re: [PATCH] IB/mlx5: Limit to 64-bit builds
From: Leon Romanovsky @ 2019-05-30 12:22 UTC (permalink / raw)
To: Guenter Roeck
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
Ariel Levkovich
In-Reply-To: <1559216144-2085-1-git-send-email-linux@roeck-us.net>
On Thu, May 30, 2019 at 04:35:44AM -0700, Guenter Roeck wrote:
> 32-bit builds fail with errors such as
>
> ERROR: "__udivdi3" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!
It is fixed in rdma-rc.
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/jgg-for-rc&id=37eb86c4507abcb14fc346863e83aa8751aa4675
Thanks
>
> Fixes: 25c13324d03d ("IB/mlx5: Add steering SW ICM device memory type")
> Cc: Ariel Levkovich <lariel@mellanox.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/infiniband/hw/mlx5/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/Kconfig b/drivers/infiniband/hw/mlx5/Kconfig
> index ea248def4556..574b97da7a43 100644
> --- a/drivers/infiniband/hw/mlx5/Kconfig
> +++ b/drivers/infiniband/hw/mlx5/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config MLX5_INFINIBAND
> tristate "Mellanox 5th generation network adapters (ConnectX series) support"
> - depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
> + depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE && 64BIT
> ---help---
> This driver provides low-level InfiniBand support for
> Mellanox Connect-IB PCI Express host channel adapters (HCAs).
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] IB/mlx5: Limit to 64-bit builds
From: Guenter Roeck @ 2019-05-30 11:35 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
Guenter Roeck, Ariel Levkovich
32-bit builds fail with errors such as
ERROR: "__udivdi3" [drivers/infiniband/hw/mlx5/mlx5_ib.ko] undefined!
Fixes: 25c13324d03d ("IB/mlx5: Add steering SW ICM device memory type")
Cc: Ariel Levkovich <lariel@mellanox.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/infiniband/hw/mlx5/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx5/Kconfig b/drivers/infiniband/hw/mlx5/Kconfig
index ea248def4556..574b97da7a43 100644
--- a/drivers/infiniband/hw/mlx5/Kconfig
+++ b/drivers/infiniband/hw/mlx5/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config MLX5_INFINIBAND
tristate "Mellanox 5th generation network adapters (ConnectX series) support"
- depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
+ depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE && 64BIT
---help---
This driver provides low-level InfiniBand support for
Mellanox Connect-IB PCI Express host channel adapters (HCAs).
--
2.7.4
^ permalink raw reply related
* Re: [PATCH rdma-next v2 13/17] RDMA/core: Get sum value of all counters when perform a sysfs stat read
From: Leon Romanovsky @ 2019-05-30 7:04 UTC (permalink / raw)
To: Mark Zhang
Cc: Jason Gunthorpe, Doug Ledford, RDMA mailing list, Majd Dibbiny,
Saeed Mahameed, linux-netdev
In-Reply-To: <6e0b034c-b647-749f-fba7-2ac51a12d327@mellanox.com>
On Thu, May 30, 2019 at 06:01:46AM +0000, Mark Zhang wrote:
> On 5/29/2019 11:44 PM, Jason Gunthorpe wrote:
> > On Wed, May 29, 2019 at 02:05:24PM +0300, Leon Romanovsky wrote:
> >> On Wed, May 22, 2019 at 02:26:36PM -0300, Jason Gunthorpe wrote:
> >>> On Mon, Apr 29, 2019 at 11:34:49AM +0300, Leon Romanovsky wrote:
> >>>> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> >>>> index c56ffc61ab1e..8ae4906a60e7 100644
> >>>> +++ b/drivers/infiniband/core/device.c
> >>>> @@ -1255,7 +1255,11 @@ int ib_register_device(struct ib_device *device, const char *name)
> >>>> goto dev_cleanup;
> >>>> }
> >>>>
> >>>> - rdma_counter_init(device);
> >>>> + ret = rdma_counter_init(device);
> >>>> + if (ret) {
> >>>> + dev_warn(&device->dev, "Couldn't initialize counter\n");
> >>>> + goto sysfs_cleanup;
> >>>> + }
> >>>
> >>> Don't put this things randomly, if there is some reason it should be
> >>> after sysfs it needs a comment, otherwise if it is just allocating
> >>> memory it belongs earlier, and the unwind should be done in release.
> >>>
> >>> I also think it is very strange/wrong that both sysfs and counters are
> >>> allocating the same alloc_hw_stats object
> >>>
> >>> Why can't they share?
> >>
> >> They can, but we wanted to separate "legacy" counters which were exposed
> >> through sysfs and "new" counters which can be enabled/disable automatically.
> >
> > Is there any cross contamination through the hw_stats? If no they
> > should just share. >
>
> sysfs hw_stats holds the port counter, while this one holds the
> history value of all dynamically allocated counters. They can not share.
> port_counter =
> default_counter + running_dynamic_counter + history_dynamic_counter
I'm not saying that it is right thing to do, but nothing prevents from you
to add extra field to port_counter exactly for that.
Thanks
>
> > Jason
> >
>
^ permalink raw reply
* Re: [PATCH rdma-next v2 13/17] RDMA/core: Get sum value of all counters when perform a sysfs stat read
From: Mark Zhang @ 2019-05-30 6:01 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky
Cc: Doug Ledford, RDMA mailing list, Majd Dibbiny, Saeed Mahameed,
linux-netdev
In-Reply-To: <20190529154438.GB8567@ziepe.ca>
On 5/29/2019 11:44 PM, Jason Gunthorpe wrote:
> On Wed, May 29, 2019 at 02:05:24PM +0300, Leon Romanovsky wrote:
>> On Wed, May 22, 2019 at 02:26:36PM -0300, Jason Gunthorpe wrote:
>>> On Mon, Apr 29, 2019 at 11:34:49AM +0300, Leon Romanovsky wrote:
>>>> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
>>>> index c56ffc61ab1e..8ae4906a60e7 100644
>>>> +++ b/drivers/infiniband/core/device.c
>>>> @@ -1255,7 +1255,11 @@ int ib_register_device(struct ib_device *device, const char *name)
>>>> goto dev_cleanup;
>>>> }
>>>>
>>>> - rdma_counter_init(device);
>>>> + ret = rdma_counter_init(device);
>>>> + if (ret) {
>>>> + dev_warn(&device->dev, "Couldn't initialize counter\n");
>>>> + goto sysfs_cleanup;
>>>> + }
>>>
>>> Don't put this things randomly, if there is some reason it should be
>>> after sysfs it needs a comment, otherwise if it is just allocating
>>> memory it belongs earlier, and the unwind should be done in release.
>>>
>>> I also think it is very strange/wrong that both sysfs and counters are
>>> allocating the same alloc_hw_stats object
>>>
>>> Why can't they share?
>>
>> They can, but we wanted to separate "legacy" counters which were exposed
>> through sysfs and "new" counters which can be enabled/disable automatically.
>
> Is there any cross contamination through the hw_stats? If no they
> should just share. >
sysfs hw_stats holds the port counter, while this one holds the
history value of all dynamically allocated counters. They can not share.
port_counter =
default_counter + running_dynamic_counter + history_dynamic_counter
> Jason
>
^ permalink raw reply
* [PATCH mlx5-next 6/6] {IB,net}/mlx5: Constify rep ops functions pointers
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Parav Pandit,
Mark Bloch
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Parav Pandit <parav@mellanox.com>
Currently for every representor type and for every single vport,
representer function pointers copy is stored even though they don't
change from one to other vport.
Additionally priv data entry for the rep is not passed during
registration, but its copied. It is used (set and cleared) by the user
of the reps.
As we want to scale vports, to simplify and also to split constants
from data,
1. Rename mlx5_eswitch_rep_if to mlx5_eswitch_rep_ops as to match _ops
prefix with other standard netdev, ibdev ops.
2. Constify the IB and Ethernet rep ops structure.
3. Instead of storing copy of all rep function pointers, store copy
per eswitch rep type.
4. Split data and function pointers to mlx5_eswitch_rep_ops and
mlx5_eswitch_rep_data.
Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/infiniband/hw/mlx5/ib_rep.c | 19 +++++-----
drivers/infiniband/hw/mlx5/ib_rep.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 15 ++++----
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 1 +
.../mellanox/mlx5/core/eswitch_offloads.c | 38 +++++++++----------
include/linux/mlx5/eswitch.h | 20 +++++-----
7 files changed, 49 insertions(+), 48 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
index cbcc40d776b9..22e651cb5534 100644
--- a/drivers/infiniband/hw/mlx5/ib_rep.c
+++ b/drivers/infiniband/hw/mlx5/ib_rep.c
@@ -60,7 +60,7 @@ mlx5_ib_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
if (!__mlx5_ib_add(ibdev, profile))
return -EINVAL;
- rep->rep_if[REP_IB].priv = ibdev;
+ rep->rep_data[REP_IB].priv = ibdev;
return 0;
}
@@ -70,13 +70,13 @@ mlx5_ib_vport_rep_unload(struct mlx5_eswitch_rep *rep)
{
struct mlx5_ib_dev *dev;
- if (!rep->rep_if[REP_IB].priv ||
+ if (!rep->rep_data[REP_IB].priv ||
rep->vport != MLX5_VPORT_UPLINK)
return;
dev = mlx5_ib_rep_to_dev(rep);
__mlx5_ib_remove(dev, dev->profile, MLX5_IB_STAGE_MAX);
- rep->rep_if[REP_IB].priv = NULL;
+ rep->rep_data[REP_IB].priv = NULL;
}
static void *mlx5_ib_vport_get_proto_dev(struct mlx5_eswitch_rep *rep)
@@ -84,16 +84,17 @@ static void *mlx5_ib_vport_get_proto_dev(struct mlx5_eswitch_rep *rep)
return mlx5_ib_rep_to_dev(rep);
}
+static const struct mlx5_eswitch_rep_ops rep_ops = {
+ .load = mlx5_ib_vport_rep_load,
+ .unload = mlx5_ib_vport_rep_unload,
+ .get_proto_dev = mlx5_ib_vport_get_proto_dev,
+};
+
void mlx5_ib_register_vport_reps(struct mlx5_core_dev *mdev)
{
struct mlx5_eswitch *esw = mdev->priv.eswitch;
- struct mlx5_eswitch_rep_if rep_if = {};
-
- rep_if.load = mlx5_ib_vport_rep_load;
- rep_if.unload = mlx5_ib_vport_rep_unload;
- rep_if.get_proto_dev = mlx5_ib_vport_get_proto_dev;
- mlx5_eswitch_register_vport_reps(esw, &rep_if, REP_IB);
+ mlx5_eswitch_register_vport_reps(esw, &rep_ops, REP_IB);
}
void mlx5_ib_unregister_vport_reps(struct mlx5_core_dev *mdev)
diff --git a/drivers/infiniband/hw/mlx5/ib_rep.h b/drivers/infiniband/hw/mlx5/ib_rep.h
index c995102b0276..22adce2d6795 100644
--- a/drivers/infiniband/hw/mlx5/ib_rep.h
+++ b/drivers/infiniband/hw/mlx5/ib_rep.h
@@ -72,6 +72,6 @@ struct net_device *mlx5_ib_get_rep_netdev(struct mlx5_eswitch *esw,
static inline
struct mlx5_ib_dev *mlx5_ib_rep_to_dev(struct mlx5_eswitch_rep *rep)
{
- return rep->rep_if[REP_IB].priv;
+ return rep->rep_data[REP_IB].priv;
}
#endif /* __MLX5_IB_REP_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 91e24f1cead8..33f8f99681a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -1752,7 +1752,7 @@ mlx5e_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
}
rpriv->netdev = netdev;
- rep->rep_if[REP_ETH].priv = rpriv;
+ rep->rep_data[REP_ETH].priv = rpriv;
INIT_LIST_HEAD(&rpriv->vport_sqs_list);
if (rep->vport == MLX5_VPORT_UPLINK) {
@@ -1826,16 +1826,17 @@ static void *mlx5e_vport_rep_get_proto_dev(struct mlx5_eswitch_rep *rep)
return rpriv->netdev;
}
+static const struct mlx5_eswitch_rep_ops rep_ops = {
+ .load = mlx5e_vport_rep_load,
+ .unload = mlx5e_vport_rep_unload,
+ .get_proto_dev = mlx5e_vport_rep_get_proto_dev
+};
+
void mlx5e_rep_register_vport_reps(struct mlx5_core_dev *mdev)
{
struct mlx5_eswitch *esw = mdev->priv.eswitch;
- struct mlx5_eswitch_rep_if rep_if = {};
-
- rep_if.load = mlx5e_vport_rep_load;
- rep_if.unload = mlx5e_vport_rep_unload;
- rep_if.get_proto_dev = mlx5e_vport_rep_get_proto_dev;
- mlx5_eswitch_register_vport_reps(esw, &rep_if, REP_ETH);
+ mlx5_eswitch_register_vport_reps(esw, &rep_ops, REP_ETH);
}
void mlx5e_rep_unregister_vport_reps(struct mlx5_core_dev *mdev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index c40c025afd99..e34573fd88c1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -91,7 +91,7 @@ struct mlx5e_rep_priv {
static inline
struct mlx5e_rep_priv *mlx5e_rep_to_rep_priv(struct mlx5_eswitch_rep *rep)
{
- return rep->rep_if[REP_ETH].priv;
+ return rep->rep_data[REP_ETH].priv;
}
struct mlx5e_neigh {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index b524813cccac..135d9a29bbdf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -173,6 +173,7 @@ struct mlx5_esw_offload {
struct mutex peer_mutex;
DECLARE_HASHTABLE(encap_tbl, 8);
DECLARE_HASHTABLE(mod_hdr_tbl, 8);
+ const struct mlx5_eswitch_rep_ops *rep_ops[NUM_REP_TYPES];
u8 inline_mode;
u64 num_flows;
u8 encap;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 05cb2fffd887..d6246ee042fa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -332,7 +332,7 @@ static int esw_set_global_vlan_pop(struct mlx5_eswitch *esw, u8 val)
esw_debug(esw->dev, "%s applying global %s policy\n", __func__, val ? "pop" : "none");
for (vf_vport = 1; vf_vport < esw->enabled_vports; vf_vport++) {
rep = &esw->offloads.vport_reps[vf_vport];
- if (atomic_read(&rep->rep_if[REP_ETH].state) != REP_LOADED)
+ if (atomic_read(&rep->rep_data[REP_ETH].state) != REP_LOADED)
continue;
err = __mlx5_eswitch_set_vport_vlan(esw, rep->vport, 0, 0, val);
@@ -1276,7 +1276,7 @@ int esw_offloads_init_reps(struct mlx5_eswitch *esw)
ether_addr_copy(rep->hw_id, hw_id);
for (rep_type = 0; rep_type < NUM_REP_TYPES; rep_type++)
- atomic_set(&rep->rep_if[rep_type].state,
+ atomic_set(&rep->rep_data[rep_type].state,
REP_UNREGISTERED);
}
@@ -1286,9 +1286,9 @@ int esw_offloads_init_reps(struct mlx5_eswitch *esw)
static void __esw_offloads_unload_rep(struct mlx5_eswitch *esw,
struct mlx5_eswitch_rep *rep, u8 rep_type)
{
- if (atomic_cmpxchg(&rep->rep_if[rep_type].state,
+ if (atomic_cmpxchg(&rep->rep_data[rep_type].state,
REP_LOADED, REP_REGISTERED) == REP_LOADED)
- rep->rep_if[rep_type].unload(rep);
+ esw->offloads.rep_ops[rep_type]->unload(rep);
}
static void __unload_reps_special_vport(struct mlx5_eswitch *esw, u8 rep_type)
@@ -1349,11 +1349,11 @@ static int __esw_offloads_load_rep(struct mlx5_eswitch *esw,
{
int err = 0;
- if (atomic_cmpxchg(&rep->rep_if[rep_type].state,
+ if (atomic_cmpxchg(&rep->rep_data[rep_type].state,
REP_REGISTERED, REP_LOADED) == REP_REGISTERED) {
- err = rep->rep_if[rep_type].load(esw->dev, rep);
+ err = esw->offloads.rep_ops[rep_type]->load(esw->dev, rep);
if (err)
- atomic_set(&rep->rep_if[rep_type].state,
+ atomic_set(&rep->rep_data[rep_type].state,
REP_REGISTERED);
}
@@ -2216,21 +2216,17 @@ int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8 *encap)
}
void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
- struct mlx5_eswitch_rep_if *__rep_if,
+ const struct mlx5_eswitch_rep_ops *ops,
u8 rep_type)
{
- struct mlx5_eswitch_rep_if *rep_if;
+ struct mlx5_eswitch_rep_data *rep_data;
struct mlx5_eswitch_rep *rep;
int i;
+ esw->offloads.rep_ops[rep_type] = ops;
mlx5_esw_for_all_reps(esw, i, rep) {
- rep_if = &rep->rep_if[rep_type];
- rep_if->load = __rep_if->load;
- rep_if->unload = __rep_if->unload;
- rep_if->get_proto_dev = __rep_if->get_proto_dev;
- rep_if->priv = __rep_if->priv;
-
- atomic_set(&rep_if->state, REP_REGISTERED);
+ rep_data = &rep->rep_data[rep_type];
+ atomic_set(&rep_data->state, REP_REGISTERED);
}
}
EXPORT_SYMBOL(mlx5_eswitch_register_vport_reps);
@@ -2245,7 +2241,7 @@ void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type)
__unload_reps_all_vport(esw, max_vf, rep_type);
mlx5_esw_for_all_reps(esw, i, rep)
- atomic_set(&rep->rep_if[rep_type].state, REP_UNREGISTERED);
+ atomic_set(&rep->rep_data[rep_type].state, REP_UNREGISTERED);
}
EXPORT_SYMBOL(mlx5_eswitch_unregister_vport_reps);
@@ -2254,7 +2250,7 @@ void *mlx5_eswitch_get_uplink_priv(struct mlx5_eswitch *esw, u8 rep_type)
struct mlx5_eswitch_rep *rep;
rep = mlx5_eswitch_get_rep(esw, MLX5_VPORT_UPLINK);
- return rep->rep_if[rep_type].priv;
+ return rep->rep_data[rep_type].priv;
}
void *mlx5_eswitch_get_proto_dev(struct mlx5_eswitch *esw,
@@ -2265,9 +2261,9 @@ void *mlx5_eswitch_get_proto_dev(struct mlx5_eswitch *esw,
rep = mlx5_eswitch_get_rep(esw, vport);
- if (atomic_read(&rep->rep_if[rep_type].state) == REP_LOADED &&
- rep->rep_if[rep_type].get_proto_dev)
- return rep->rep_if[rep_type].get_proto_dev(rep);
+ if (atomic_read(&rep->rep_data[rep_type].state) == REP_LOADED &&
+ esw->offloads.rep_ops[rep_type]->get_proto_dev)
+ return esw->offloads.rep_ops[rep_type]->get_proto_dev(rep);
return NULL;
}
EXPORT_SYMBOL(mlx5_eswitch_get_proto_dev);
diff --git a/include/linux/mlx5/eswitch.h b/include/linux/mlx5/eswitch.h
index 0ca77dd1429c..d81ee4df181c 100644
--- a/include/linux/mlx5/eswitch.h
+++ b/include/linux/mlx5/eswitch.h
@@ -29,17 +29,19 @@ enum {
};
struct mlx5_eswitch_rep;
-struct mlx5_eswitch_rep_if {
- int (*load)(struct mlx5_core_dev *dev,
- struct mlx5_eswitch_rep *rep);
- void (*unload)(struct mlx5_eswitch_rep *rep);
- void *(*get_proto_dev)(struct mlx5_eswitch_rep *rep);
- void *priv;
- atomic_t state;
+struct mlx5_eswitch_rep_ops {
+ int (*load)(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep);
+ void (*unload)(struct mlx5_eswitch_rep *rep);
+ void *(*get_proto_dev)(struct mlx5_eswitch_rep *rep);
+};
+
+struct mlx5_eswitch_rep_data {
+ void *priv;
+ atomic_t state;
};
struct mlx5_eswitch_rep {
- struct mlx5_eswitch_rep_if rep_if[NUM_REP_TYPES];
+ struct mlx5_eswitch_rep_data rep_data[NUM_REP_TYPES];
u16 vport;
u8 hw_id[ETH_ALEN];
u16 vlan;
@@ -47,7 +49,7 @@ struct mlx5_eswitch_rep {
};
void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
- struct mlx5_eswitch_rep_if *rep_if,
+ const struct mlx5_eswitch_rep_ops *ops,
u8 rep_type);
void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type);
void *mlx5_eswitch_get_proto_dev(struct mlx5_eswitch *esw,
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 5/6] {IB,net}/mlx5: No need to typecast from void* to mlx5_ib_dev*
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Parav Pandit
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Parav Pandit <parav@mellanox.com>
Avoid typecasting from void* to mlx5_ib_dev* or mlx5e_rep_priv*
as it is not needed.
Signed-off-by: Parav Pandit <parav@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/infiniband/hw/mlx5/ib_rep.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/ib_rep.h b/drivers/infiniband/hw/mlx5/ib_rep.h
index 1d9778da8a50..c995102b0276 100644
--- a/drivers/infiniband/hw/mlx5/ib_rep.h
+++ b/drivers/infiniband/hw/mlx5/ib_rep.h
@@ -72,6 +72,6 @@ struct net_device *mlx5_ib_get_rep_netdev(struct mlx5_eswitch *esw,
static inline
struct mlx5_ib_dev *mlx5_ib_rep_to_dev(struct mlx5_eswitch_rep *rep)
{
- return (struct mlx5_ib_dev *)rep->rep_if[REP_IB].priv;
+ return rep->rep_if[REP_IB].priv;
}
#endif /* __MLX5_IB_REP_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index 83b573b1abac..c40c025afd99 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -91,7 +91,7 @@ struct mlx5e_rep_priv {
static inline
struct mlx5e_rep_priv *mlx5e_rep_to_rep_priv(struct mlx5_eswitch_rep *rep)
{
- return (struct mlx5e_rep_priv *)rep->rep_if[REP_ETH].priv;
+ return rep->rep_if[REP_ETH].priv;
}
struct mlx5e_neigh {
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 4/6] net/mlx5: E-Switch, Honor eswitch functions changed event cap
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Vu Pham,
Parav Pandit
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Vu Pham <vuhuong@mellanox.com>
Whenever device supports eswitch functions changed event, honor
such device setting. Do not limit it to ECPF.
Signed-off-by: Parav Pandit <parav@mellanox.com>
Signed-off-by: Vu Pham <vuhuong@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 13 +++++++++++++
.../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 6 +++---
include/linux/mlx5/mlx5_ifc.h | 4 +++-
4 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 052bd70e4aa6..5e9319d3d90c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -533,7 +533,7 @@ static u64 gather_async_events_mask(struct mlx5_core_dev *dev)
if (MLX5_CAP_GEN(dev, max_num_of_monitor_counters))
async_event_mask |= (1ull << MLX5_EVENT_TYPE_MONITOR_COUNTER);
- if (mlx5_core_is_ecpf_esw_manager(dev))
+ if (mlx5_eswitch_is_funcs_handler(dev))
async_event_mask |=
(1ull << MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 320dd83dd301..b524813cccac 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -406,6 +406,18 @@ static inline u16 mlx5_eswitch_manager_vport(struct mlx5_core_dev *dev)
MLX5_VPORT_ECPF : MLX5_VPORT_PF;
}
+static inline bool mlx5_eswitch_is_funcs_handler(struct mlx5_core_dev *dev)
+{
+ /* Ideally device should have the functions changed supported
+ * capability regardless of it being ECPF or PF wherever such
+ * event should be processed such as on eswitch manager device.
+ * However, some ECPF based device might not have this capability
+ * set. Hence OR for ECPF check to cover such device.
+ */
+ return MLX5_CAP_ESW(dev, esw_functions_changed) ||
+ mlx5_core_is_ecpf_esw_manager(dev);
+}
+
static inline int mlx5_eswitch_uplink_idx(struct mlx5_eswitch *esw)
{
/* Uplink always locate at the last element of the array.*/
@@ -500,6 +512,7 @@ static inline void mlx5_eswitch_cleanup(struct mlx5_eswitch *esw) {}
static inline int mlx5_eswitch_enable_sriov(struct mlx5_eswitch *esw, int nvfs, int mode) { return 0; }
static inline void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw) {}
static inline bool mlx5_esw_lag_prereq(struct mlx5_core_dev *dev0, struct mlx5_core_dev *dev1) { return true; }
+static inline bool mlx5_eswitch_is_funcs_handler(struct mlx5_core_dev *dev) { return false; }
#define FDB_MAX_CHAIN 1
#define FDB_SLOW_PATH_CHAIN (FDB_MAX_CHAIN + 1)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 83689678b400..05cb2fffd887 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -1836,7 +1836,7 @@ static int esw_functions_changed_event(struct notifier_block *nb,
static void esw_functions_changed_event_init(struct mlx5_eswitch *esw,
u16 vf_nvports)
{
- if (!mlx5_core_is_ecpf_esw_manager(esw->dev))
+ if (!mlx5_eswitch_is_funcs_handler(esw->dev))
return;
MLX5_NB_INIT(&esw->esw_funcs.nb, esw_functions_changed_event,
@@ -1847,7 +1847,7 @@ static void esw_functions_changed_event_init(struct mlx5_eswitch *esw,
static void esw_functions_changed_event_cleanup(struct mlx5_eswitch *esw)
{
- if (!mlx5_core_is_ecpf_esw_manager(esw->dev))
+ if (!mlx5_eswitch_is_funcs_handler(esw->dev))
return;
mlx5_eq_notifier_unregister(esw->dev, &esw->esw_funcs.nb);
@@ -1905,7 +1905,7 @@ void esw_offloads_cleanup(struct mlx5_eswitch *esw)
esw_functions_changed_event_cleanup(esw);
- if (mlx5_core_is_ecpf_esw_manager(esw->dev))
+ if (mlx5_eswitch_is_funcs_handler(esw->dev))
num_vfs = esw->esw_funcs.num_vfs;
else
num_vfs = esw->dev->priv.sriov.num_vfs;
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 0780242a757a..6513b985c5e9 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -665,7 +665,9 @@ struct mlx5_ifc_e_switch_cap_bits {
u8 vport_svlan_insert[0x1];
u8 vport_cvlan_insert_if_not_exist[0x1];
u8 vport_cvlan_insert_overwrite[0x1];
- u8 reserved_at_5[0x16];
+ u8 reserved_at_5[0x14];
+ u8 esw_functions_changed[0x1];
+ u8 reserved_at_1a[0x1];
u8 ecpf_vport_exists[0x1];
u8 counter_eswitch_affinity[0x1];
u8 merged_eswitch[0x1];
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 3/6] net/mlx5: E-Switch, Replace host_params event with functions_changed event
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Vu Pham,
Parav Pandit, Bodong Wang
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Vu Pham <vuhuong@mellanox.com>
To support sriov on a E-Switch manager, num_vfs are queried
to the firmware whenever E-Switch manager is notified by
esw_functions_changed event.
Replace host_params event with esw_functions_changed event that reflects
more appropriate naming.
While at it, also correct num_vfs type from int to u16 as expected by
the function mlx5_esw_query_functions().
Signed-off-by: Vu Pham <vuhuong@mellanox.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Bodong Wang <bodong@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/ecpf.c | 27 --------
.../net/ethernet/mellanox/mlx5/core/ecpf.h | 4 --
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 3 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 32 ++++++++-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 6 +-
.../mellanox/mlx5/core/eswitch_offloads.c | 69 +++++++++++--------
.../net/ethernet/mellanox/mlx5/core/events.c | 4 +-
include/linux/mlx5/device.h | 2 +-
include/linux/mlx5/mlx5_ifc.h | 6 +-
10 files changed, 86 insertions(+), 71 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 937ba4bcb056..7d3aec98e31f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -316,7 +316,7 @@ static int mlx5_internal_err_ret_value(struct mlx5_core_dev *dev, u16 op,
case MLX5_CMD_OP_DESTROY_GENERAL_OBJECT:
case MLX5_CMD_OP_DEALLOC_MEMIC:
case MLX5_CMD_OP_PAGE_FAULT_RESUME:
- case MLX5_CMD_OP_QUERY_HOST_PARAMS:
+ case MLX5_CMD_OP_QUERY_ESW_FUNCTIONS:
return MLX5_CMD_STAT_OK;
case MLX5_CMD_OP_QUERY_HCA_CAP:
@@ -628,7 +628,7 @@ const char *mlx5_command_str(int command)
MLX5_COMMAND_STR_CASE(QUERY_MODIFY_HEADER_CONTEXT);
MLX5_COMMAND_STR_CASE(ALLOC_MEMIC);
MLX5_COMMAND_STR_CASE(DEALLOC_MEMIC);
- MLX5_COMMAND_STR_CASE(QUERY_HOST_PARAMS);
+ MLX5_COMMAND_STR_CASE(QUERY_ESW_FUNCTIONS);
default: return "unknown command opcode";
}
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
index 4746f2d28fb6..1bcf8b8f9713 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
@@ -83,30 +83,3 @@ void mlx5_ec_cleanup(struct mlx5_core_dev *dev)
mlx5_peer_pf_cleanup(dev);
}
-
-static int mlx5_query_host_params_context(struct mlx5_core_dev *dev,
- u32 *out, int outlen)
-{
- u32 in[MLX5_ST_SZ_DW(query_host_params_in)] = {};
-
- MLX5_SET(query_host_params_in, in, opcode,
- MLX5_CMD_OP_QUERY_HOST_PARAMS);
-
- return mlx5_cmd_exec(dev, in, sizeof(in), out, outlen);
-}
-
-int mlx5_query_host_params_num_vfs(struct mlx5_core_dev *dev, int *num_vf)
-{
- u32 out[MLX5_ST_SZ_DW(query_host_params_out)] = {};
- int err;
-
- err = mlx5_query_host_params_context(dev, out, sizeof(out));
- if (err)
- return err;
-
- *num_vf = MLX5_GET(query_host_params_out, out,
- host_params_context.host_num_of_vfs);
- mlx5_core_dbg(dev, "host_num_of_vfs %d\n", *num_vf);
-
- return 0;
-}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.h b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.h
index 346372df218f..d3d7a00a02ac 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.h
@@ -16,7 +16,6 @@ enum {
bool mlx5_read_embedded_cpu(struct mlx5_core_dev *dev);
int mlx5_ec_init(struct mlx5_core_dev *dev);
void mlx5_ec_cleanup(struct mlx5_core_dev *dev);
-int mlx5_query_host_params_num_vfs(struct mlx5_core_dev *dev, int *num_vf);
#else /* CONFIG_MLX5_ESWITCH */
@@ -24,9 +23,6 @@ static inline bool
mlx5_read_embedded_cpu(struct mlx5_core_dev *dev) { return false; }
static inline int mlx5_ec_init(struct mlx5_core_dev *dev) { return 0; }
static inline void mlx5_ec_cleanup(struct mlx5_core_dev *dev) {}
-static inline int
-mlx5_query_host_params_num_vfs(struct mlx5_core_dev *dev, int *num_vf)
-{ return -EOPNOTSUPP; }
#endif /* CONFIG_MLX5_ESWITCH */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 23883d1fa22f..052bd70e4aa6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -534,7 +534,8 @@ static u64 gather_async_events_mask(struct mlx5_core_dev *dev)
async_event_mask |= (1ull << MLX5_EVENT_TYPE_MONITOR_COUNTER);
if (mlx5_core_is_ecpf_esw_manager(dev))
- async_event_mask |= (1ull << MLX5_EVENT_TYPE_HOST_PARAMS_CHANGE);
+ async_event_mask |=
+ (1ull << MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED);
return async_event_mask;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 9ea0ccfe5ef5..d8935232964a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1686,13 +1686,41 @@ static int eswitch_vport_event(struct notifier_block *nb,
return NOTIFY_OK;
}
+static int query_esw_functions(struct mlx5_core_dev *dev,
+ u32 *out, int outlen)
+{
+ u32 in[MLX5_ST_SZ_DW(query_esw_functions_in)] = {0};
+
+ MLX5_SET(query_esw_functions_in, in, opcode,
+ MLX5_CMD_OP_QUERY_ESW_FUNCTIONS);
+
+ return mlx5_cmd_exec(dev, in, sizeof(in), out, outlen);
+}
+
+int mlx5_esw_query_functions(struct mlx5_core_dev *dev, u16 *num_vfs)
+{
+ u32 out[MLX5_ST_SZ_DW(query_esw_functions_out)] = {0};
+ int err;
+
+ err = query_esw_functions(dev, out, sizeof(out));
+ if (err)
+ return err;
+
+ *num_vfs = MLX5_GET(query_esw_functions_out, out,
+ host_params_context.host_num_of_vfs);
+ esw_debug(dev, "host_num_of_vfs=%d\n", *num_vfs);
+
+ return 0;
+}
+
/* Public E-Switch API */
#define ESW_ALLOWED(esw) ((esw) && MLX5_ESWITCH_MANAGER((esw)->dev))
int mlx5_eswitch_enable_sriov(struct mlx5_eswitch *esw, int nvfs, int mode)
{
- int vf_nvports = 0, total_nvports = 0;
struct mlx5_vport *vport;
+ int total_nvports = 0;
+ u16 vf_nvports = 0;
int err;
int i, enabled_events;
@@ -1712,7 +1740,7 @@ int mlx5_eswitch_enable_sriov(struct mlx5_eswitch *esw, int nvfs, int mode)
if (mode == SRIOV_OFFLOADS) {
if (mlx5_core_is_ecpf_esw_manager(esw->dev)) {
- err = mlx5_query_host_params_num_vfs(esw->dev, &vf_nvports);
+ err = mlx5_esw_query_functions(esw->dev, &vf_nvports);
if (err)
return err;
total_nvports = esw->total_vports;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index ed3fad689ec9..320dd83dd301 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -190,7 +190,7 @@ struct mlx5_host_work {
struct mlx5_eswitch *esw;
};
-struct mlx5_host_info {
+struct mlx5_esw_functions {
struct mlx5_nb nb;
u16 num_vfs;
};
@@ -219,7 +219,7 @@ struct mlx5_eswitch {
int mode;
int nvports;
u16 manager_vport;
- struct mlx5_host_info host_info;
+ struct mlx5_esw_functions esw_funcs;
};
void esw_offloads_cleanup(struct mlx5_eswitch *esw);
@@ -386,6 +386,8 @@ bool mlx5_esw_lag_prereq(struct mlx5_core_dev *dev0,
bool mlx5_esw_multipath_prereq(struct mlx5_core_dev *dev0,
struct mlx5_core_dev *dev1);
+int mlx5_esw_query_functions(struct mlx5_core_dev *dev, u16 *num_vfs);
+
#define MLX5_DEBUG_ESWITCH_MASK BIT(3)
#define esw_info(__dev, format, ...) \
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index e09ae27485ee..83689678b400 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -41,7 +41,6 @@
#include "en.h"
#include "fs_core.h"
#include "lib/devcom.h"
-#include "ecpf.h"
#include "lib/eq.h"
/* There are two match-all miss flows, one for unicast dst mac and
@@ -1782,57 +1781,79 @@ static void esw_offloads_steering_cleanup(struct mlx5_eswitch *esw)
esw_prio_tag_acls_cleanup(esw);
}
-static void esw_host_params_event_handler(struct work_struct *work)
+static void esw_functions_changed_event_handler(struct work_struct *work)
{
struct mlx5_host_work *host_work;
struct mlx5_eswitch *esw;
- int err, num_vf = 0;
+ u16 num_vfs = 0;
+ int err;
host_work = container_of(work, struct mlx5_host_work, work);
esw = host_work->esw;
- err = mlx5_query_host_params_num_vfs(esw->dev, &num_vf);
- if (err || num_vf == esw->host_info.num_vfs)
+ err = mlx5_esw_query_functions(esw->dev, &num_vfs);
+ if (err || num_vfs == esw->esw_funcs.num_vfs)
goto out;
/* Number of VFs can only change from "0 to x" or "x to 0". */
- if (esw->host_info.num_vfs > 0) {
- esw_offloads_unload_vf_reps(esw, esw->host_info.num_vfs);
+ if (esw->esw_funcs.num_vfs > 0) {
+ esw_offloads_unload_vf_reps(esw, esw->esw_funcs.num_vfs);
} else {
- err = esw_offloads_load_vf_reps(esw, num_vf);
+ err = esw_offloads_load_vf_reps(esw, num_vfs);
if (err)
goto out;
}
- esw->host_info.num_vfs = num_vf;
+ esw->esw_funcs.num_vfs = num_vfs;
out:
kfree(host_work);
}
-static int esw_host_params_event(struct notifier_block *nb,
- unsigned long type, void *data)
+static int esw_functions_changed_event(struct notifier_block *nb,
+ unsigned long type, void *data)
{
+ struct mlx5_esw_functions *esw_funcs;
struct mlx5_host_work *host_work;
- struct mlx5_host_info *host_info;
struct mlx5_eswitch *esw;
host_work = kzalloc(sizeof(*host_work), GFP_ATOMIC);
if (!host_work)
return NOTIFY_DONE;
- host_info = mlx5_nb_cof(nb, struct mlx5_host_info, nb);
- esw = container_of(host_info, struct mlx5_eswitch, host_info);
+ esw_funcs = mlx5_nb_cof(nb, struct mlx5_esw_functions, nb);
+ esw = container_of(esw_funcs, struct mlx5_eswitch, esw_funcs);
host_work->esw = esw;
- INIT_WORK(&host_work->work, esw_host_params_event_handler);
+ INIT_WORK(&host_work->work, esw_functions_changed_event_handler);
queue_work(esw->work_queue, &host_work->work);
return NOTIFY_OK;
}
+static void esw_functions_changed_event_init(struct mlx5_eswitch *esw,
+ u16 vf_nvports)
+{
+ if (!mlx5_core_is_ecpf_esw_manager(esw->dev))
+ return;
+
+ MLX5_NB_INIT(&esw->esw_funcs.nb, esw_functions_changed_event,
+ ESW_FUNCTIONS_CHANGED);
+ mlx5_eq_notifier_register(esw->dev, &esw->esw_funcs.nb);
+ esw->esw_funcs.num_vfs = vf_nvports;
+}
+
+static void esw_functions_changed_event_cleanup(struct mlx5_eswitch *esw)
+{
+ if (!mlx5_core_is_ecpf_esw_manager(esw->dev))
+ return;
+
+ mlx5_eq_notifier_unregister(esw->dev, &esw->esw_funcs.nb);
+ flush_workqueue(esw->work_queue);
+}
+
int esw_offloads_init(struct mlx5_eswitch *esw, int vf_nvports,
int total_nvports)
{
@@ -1848,12 +1869,7 @@ int esw_offloads_init(struct mlx5_eswitch *esw, int vf_nvports,
esw_offloads_devcom_init(esw);
- if (mlx5_core_is_ecpf_esw_manager(esw->dev)) {
- MLX5_NB_INIT(&esw->host_info.nb, esw_host_params_event,
- HOST_PARAMS_CHANGE);
- mlx5_eq_notifier_register(esw->dev, &esw->host_info.nb);
- esw->host_info.num_vfs = vf_nvports;
- }
+ esw_functions_changed_event_init(esw, vf_nvports);
mlx5_rdma_enable_roce(esw->dev);
@@ -1887,13 +1903,12 @@ void esw_offloads_cleanup(struct mlx5_eswitch *esw)
{
u16 num_vfs;
- if (mlx5_core_is_ecpf_esw_manager(esw->dev)) {
- mlx5_eq_notifier_unregister(esw->dev, &esw->host_info.nb);
- flush_workqueue(esw->work_queue);
- num_vfs = esw->host_info.num_vfs;
- } else {
+ esw_functions_changed_event_cleanup(esw);
+
+ if (mlx5_core_is_ecpf_esw_manager(esw->dev))
+ num_vfs = esw->esw_funcs.num_vfs;
+ else
num_vfs = esw->dev->priv.sriov.num_vfs;
- }
mlx5_rdma_disable_roce(esw->dev);
esw_offloads_devcom_cleanup(esw);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/events.c b/drivers/net/ethernet/mellanox/mlx5/core/events.c
index a81e8d2168d8..8bcf3426b9c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/events.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/events.c
@@ -108,8 +108,8 @@ static const char *eqe_type_str(u8 type)
return "MLX5_EVENT_TYPE_STALL_EVENT";
case MLX5_EVENT_TYPE_CMD:
return "MLX5_EVENT_TYPE_CMD";
- case MLX5_EVENT_TYPE_HOST_PARAMS_CHANGE:
- return "MLX5_EVENT_TYPE_HOST_PARAMS_CHANGE";
+ case MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED:
+ return "MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED";
case MLX5_EVENT_TYPE_PAGE_REQUEST:
return "MLX5_EVENT_TYPE_PAGE_REQUEST";
case MLX5_EVENT_TYPE_PAGE_FAULT:
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index fc2b6e807f06..5e760067ac41 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -342,7 +342,7 @@ enum mlx5_event {
MLX5_EVENT_TYPE_PAGE_FAULT = 0xc,
MLX5_EVENT_TYPE_NIC_VPORT_CHANGE = 0xd,
- MLX5_EVENT_TYPE_HOST_PARAMS_CHANGE = 0xe,
+ MLX5_EVENT_TYPE_ESW_FUNCTIONS_CHANGED = 0xe,
MLX5_EVENT_TYPE_DCT_DRAINED = 0x1c,
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index feaa909bf14f..0780242a757a 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -155,7 +155,7 @@ enum {
MLX5_CMD_OP_QUERY_XRQ_DC_PARAMS_ENTRY = 0x725,
MLX5_CMD_OP_SET_XRQ_DC_PARAMS_ENTRY = 0x726,
MLX5_CMD_OP_QUERY_XRQ_ERROR_PARAMS = 0x727,
- MLX5_CMD_OP_QUERY_HOST_PARAMS = 0x740,
+ MLX5_CMD_OP_QUERY_ESW_FUNCTIONS = 0x740,
MLX5_CMD_OP_QUERY_VPORT_STATE = 0x750,
MLX5_CMD_OP_MODIFY_VPORT_STATE = 0x751,
MLX5_CMD_OP_QUERY_ESW_VPORT_CONTEXT = 0x752,
@@ -9721,7 +9721,7 @@ struct mlx5_ifc_host_params_context_bits {
u8 reserved_at_80[0x180];
};
-struct mlx5_ifc_query_host_params_in_bits {
+struct mlx5_ifc_query_esw_functions_in_bits {
u8 opcode[0x10];
u8 reserved_at_10[0x10];
@@ -9731,7 +9731,7 @@ struct mlx5_ifc_query_host_params_in_bits {
u8 reserved_at_40[0x40];
};
-struct mlx5_ifc_query_host_params_out_bits {
+struct mlx5_ifc_query_esw_functions_out_bits {
u8 status[0x8];
u8 reserved_at_8[0x18];
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 2/6] net/mlx5: Introduce termination table bits
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Eli Britstein,
Oz Shlomo
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Eli Britstein <elibr@mellanox.com>
Termination table is a flow table with a termination flag. The flag
allows the firmware to assume that the the specified actions are the last
actions list. This assumption allows the FW to safely perform potential
looping logic (e.g. hairpin). Introduce the bits for this attribute.
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Oz Shlomo <ozsh@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 3 +++
include/linux/mlx5/fs.h | 1 +
include/linux/mlx5/mlx5_ifc.h | 6 ++++--
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
index 013b1ca4a791..bb24c3797218 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
@@ -147,6 +147,7 @@ static int mlx5_cmd_create_flow_table(struct mlx5_flow_root_namespace *ns,
{
int en_encap = !!(ft->flags & MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT);
int en_decap = !!(ft->flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP);
+ int term = !!(ft->flags & MLX5_FLOW_TABLE_TERMINATION);
u32 out[MLX5_ST_SZ_DW(create_flow_table_out)] = {0};
u32 in[MLX5_ST_SZ_DW(create_flow_table_in)] = {0};
struct mlx5_core_dev *dev = ns->dev;
@@ -167,6 +168,8 @@ static int mlx5_cmd_create_flow_table(struct mlx5_flow_root_namespace *ns,
en_decap);
MLX5_SET(create_flow_table_in, in, flow_table_context.reformat_en,
en_encap);
+ MLX5_SET(create_flow_table_in, in, flow_table_context.termination_table,
+ term);
switch (ft->op_mod) {
case FS_FT_OP_MOD_NORMAL:
diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
index e690ba0f965c..2ddaa97f2179 100644
--- a/include/linux/mlx5/fs.h
+++ b/include/linux/mlx5/fs.h
@@ -47,6 +47,7 @@ enum {
enum {
MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT = BIT(0),
MLX5_FLOW_TABLE_TUNNEL_EN_DECAP = BIT(1),
+ MLX5_FLOW_TABLE_TERMINATION = BIT(2),
};
#define LEFTOVERS_RULE_NUM 2
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 7ee422e38826..feaa909bf14f 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -382,7 +382,8 @@ struct mlx5_ifc_flow_table_prop_layout_bits {
u8 reformat_and_modify_action[0x1];
u8 reserved_at_15[0x2];
u8 table_miss_action_domain[0x1];
- u8 reserved_at_18[0x8];
+ u8 termination_table[0x1];
+ u8 reserved_at_19[0x7];
u8 reserved_at_20[0x2];
u8 log_max_ft_size[0x6];
u8 log_max_modify_header_context[0x8];
@@ -7239,7 +7240,8 @@ struct mlx5_ifc_create_flow_table_out_bits {
struct mlx5_ifc_flow_table_context_bits {
u8 reformat_en[0x1];
u8 decap_en[0x1];
- u8 reserved_at_2[0x2];
+ u8 reserved_at_2[0x1];
+ u8 termination_table[0x1];
u8 table_miss_action[0x4];
u8 level[0x8];
u8 reserved_at_10[0x8];
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 1/6] net/mlx5: Add core dump register access HW bits
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>
From: Moshe Shemesh <moshe@mellanox.com>
Add Firmware core dump registers and HW definitions.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
include/linux/mlx5/driver.h | 1 +
include/linux/mlx5/mlx5_ifc.h | 17 ++++++++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 5a27246db883..b5431f7d97cb 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -107,6 +107,7 @@ enum {
MLX5_REG_FPGA_CAP = 0x4022,
MLX5_REG_FPGA_CTRL = 0x4023,
MLX5_REG_FPGA_ACCESS_REG = 0x4024,
+ MLX5_REG_CORE_DUMP = 0x402e,
MLX5_REG_PCAP = 0x5001,
MLX5_REG_PMTU = 0x5003,
MLX5_REG_PTYS = 0x5004,
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 5e74305e2e57..7ee422e38826 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -715,7 +715,9 @@ struct mlx5_ifc_qos_cap_bits {
};
struct mlx5_ifc_debug_cap_bits {
- u8 reserved_at_0[0x20];
+ u8 core_dump_general[0x1];
+ u8 core_dump_qp[0x1];
+ u8 reserved_at_2[0x1e];
u8 reserved_at_20[0x2];
u8 stall_detect[0x1];
@@ -2531,6 +2533,7 @@ union mlx5_ifc_hca_cap_union_bits {
struct mlx5_ifc_e_switch_cap_bits e_switch_cap;
struct mlx5_ifc_vector_calc_cap_bits vector_calc_cap;
struct mlx5_ifc_qos_cap_bits qos_cap;
+ struct mlx5_ifc_debug_cap_bits debug_cap;
struct mlx5_ifc_fpga_cap_bits fpga_cap;
u8 reserved_at_0[0x8000];
};
@@ -8546,6 +8549,18 @@ struct mlx5_ifc_qcam_reg_bits {
u8 reserved_at_1c0[0x80];
};
+struct mlx5_ifc_core_dump_reg_bits {
+ u8 reserved_at_0[0x18];
+ u8 core_dump_type[0x8];
+
+ u8 reserved_at_20[0x30];
+ u8 vhca_id[0x10];
+
+ u8 reserved_at_60[0x8];
+ u8 qpn[0x18];
+ u8 reserved_at_80[0x180];
+};
+
struct mlx5_ifc_pcap_reg_bits {
u8 reserved_at_0[0x8];
u8 local_port[0x8];
--
2.21.0
^ permalink raw reply related
* [PATCH mlx5-next 0/6] Mellanox, mlx5-next minor updates 2019-05-29
From: Saeed Mahameed @ 2019-05-29 22:50 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
Hi All,
This series provides some low level updates for mlx5 driver needed for
both rdma and netdev trees.
Eli adds termination flow steering table bits and hardware definitions.
Moshe introduces the core dump HW access registers definitions.
Parav refactors and cleans-up VF representors functions handlers.
Vu renames host_params bits to function_changed bits and add the
support for eswitch functions change event in the eswitch general case.
(for both legacy and switchdev modes).
In case of no objection this series will be applied to mlx5-next branch.
Thanks,
Saeed.
Eli Britstein (1):
net/mlx5: Introduce termination table bits
Moshe Shemesh (1):
net/mlx5: Add core dump register access HW bits
Parav Pandit (2):
{IB,net}/mlx5: No need to typecast from void* to mlx5_ib_dev*
{IB,net}/mlx5: Constify rep ops functions pointers
Vu Pham (2):
net/mlx5: E-Switch, Replace host_params event with functions_changed
event
net/mlx5: E-Switch, Honor eswitch functions changed event cap
drivers/infiniband/hw/mlx5/ib_rep.c | 19 ++--
drivers/infiniband/hw/mlx5/ib_rep.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/ecpf.c | 27 -----
.../net/ethernet/mellanox/mlx5/core/ecpf.h | 4 -
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 15 +--
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 5 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 32 +++++-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 20 +++-
.../mellanox/mlx5/core/eswitch_offloads.c | 107 ++++++++++--------
.../net/ethernet/mellanox/mlx5/core/events.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/fs_cmd.c | 3 +
include/linux/mlx5/device.h | 2 +-
include/linux/mlx5/driver.h | 1 +
include/linux/mlx5/eswitch.h | 20 ++--
include/linux/mlx5/fs.h | 1 +
include/linux/mlx5/mlx5_ifc.h | 33 ++++--
18 files changed, 177 insertions(+), 124 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
From: Khalid Aziz @ 2019-05-29 19:16 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, kvm, Christian Koenig, Szabolcs Nagy, Will Deacon,
dri-devel, linux-mm, Lee Smith, linux-kselftest,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, linux-arm-kernel, Dave Martin, Evgeniy Stepanov,
linux-media, Kees Cook, Ruben Ayrapetyan, Andrey Konovalov,
Kevin Brodsky, Alex Williamson, Mauro Carvalho Chehab
In-Reply-To: <20190529142008.5quqv3wskmpwdfbu@mbp>
On 5/29/19 8:20 AM, Catalin Marinas wrote:
> Hi Khalid,
>
> On Tue, May 28, 2019 at 05:33:04PM -0600, Khalid Aziz wrote:
>> On Tue, 2019-05-28 at 16:40 +0100, Catalin Marinas wrote:
>>> I think another aspect is how we define the ABI. Is allowing tags to
>>> mlock() for example something specific to arm64 or would sparc ADI
>>> need the same? In the absence of other architectures defining such
>>> ABI, my preference would be to keep the wrappers in the arch code.
>>>
>>> Assuming sparc won't implement untagged_addr(), we can place the
>>> macros back in the generic code but, as per the review here, we need
>>> to be more restrictive on where we allow tagged addresses. For
>>> example, if mmap() gets a tagged address with MAP_FIXED, is it
>>> expected to return the tag?
>>
>> I would recommend against any ABI differences between ARM64 MTE/TBI and
>> sparc ADI unless it simply can not be helped. My understanding of
>> MTE/TBI is limited, so I will explain how sparc ADI works. On sparc, a
>> tagged address has no meaning until following steps happen:
>
> Before we go into the MTE/ADI similarities or differences, just to
> clarify that TBI is something that we supported from the start of the
> arm64 kernel port. TBI (top byte ignore) allows a user pointer to have
> non-zero top byte and dereference it without causing a fault (the
> hardware masks it out). The user/kernel ABI does not allow such tagged
> pointers into the kernel, nor would the kernel return any such tagged
> addresses.
>
> With MTE (memory tagging extensions), the top-byte meaning is changed
> from no longer being ignored to actually being checked against a tag in
> the physical RAM (we call it allocation tag).
>
>> 1. set the user mode PSTATE.mcde bit. This acts as the master switch to
>> enable ADI for a process.
>>
>> 2. set TTE.mcd bit on TLB entries that match the address range ADI is
>> being enabled on.
>
> Something close enough for MTE, with the difference that enabling it is
> not a PSTATE bit but rather a system control bit (SCTLR_EL1 register),
> so only the kernel can turn it on/off for the user.
>
>> 3. Store version tag for the range of addresses userspace wants ADI
>> enabled on using "stxa" instruction. These tags are stored in physical
>> memory by the memory controller.
>
> Do you have an "ldxa" instruction to load the tags from physical memory?
Yes, "ldxa" can be used to read current tag for any memory location.
Kernel uses it to read the tags for a physical page being swapped out
and restores those tags when the page is swapped back in.
>
>> Steps 1 and 2 are accomplished by userspace by calling mprotect() with
>> PROT_ADI. Tags are set by storing tags in a loop, for example:
>>
>> version = 10;
>> tmp_addr = shmaddr;
>> end = shmaddr + BUFFER_SIZE;
>> while (tmp_addr < end) {
>> asm volatile(
>> "stxa %1, [%0]0x90\n\t"
>> :
>> : "r" (tmp_addr), "r" (version));
>> tmp_addr += adi_blksz;
>> }
>
> On arm64, a sequence similar to the above would live in the libc. So a
> malloc() call will tag the memory and return the tagged address to thePre-coloring could easily be done by
> user.
>
> We were not planning for a PROT_ADI/MTE but rather have MTE enabled for
> all user memory ranges. We may revisit this before we upstream the MTE
> support (probably some marginal benefit for the hardware not fetching
> the tags from memory if we don't need to, e.g. code sections).
>
> Given that we already have the TBI feature and with MTE enabled the top
> byte is no longer ignored, we are planning for an explicit opt-in by the
> user via prctl() to enable MTE.
OK. I had initially proposed enabling ADI for a process using prctl().
Feedback I got was prctl was not a desirable interface and I ended up
making mprotect() with PROT_ADI enable ADI on the process instead. Just
something to keep in mind.
>
>> With these semantics, giving mmap() or shamat() a tagged address is
>> meaningless since no tags have been stored at the addresses mmap() will
>> allocate and one can not store tags before memory range has been
>> allocated. If we choose to allow tagged addresses to come into mmap()
>> and shmat(), sparc code can strip the tags unconditionally and that may
>> help simplify ABI and/or code.
>
> We could say that with TBI (pre-MTE support), the top byte is actually
> ignored on mmap(). Now, if you pass a MAP_FIXED with a tagged address,
> should the user expect the same tagged address back or stripping the tag
> is acceptable? If we want to keep the current mmap() semantics, I'd say
> the same tag is returned. However, with MTE this also implies that the
> memory was coloured.
>
Is assigning a tag aprivileged operationon ARM64? I am thinking not
since you mentioned libc could do it in a loop for malloc'd memory.
mmap() can return the same tagged address but I am uneasy about kernel
pre-coloring the pages. Database can mmap 100's of GB of memory. That is
lot of work being offloaded to the kernel to pre-color the page even if
done in batches as pages are faulted in.
>>> My thoughts on allowing tags (quick look):
>>>
>>> brk - no
>>> get_mempolicy - yes
>>> madvise - yes
>>> mbind - yes
>>> mincore - yes
>>> mlock, mlock2, munlock - yes
>>> mmap - no (we may change this with MTE but not for TBI)
>>> mmap_pgoff - not used on arm64
>>> mprotect - yes
>>> mremap - yes for old_address, no for new_address (on par with mmap)
>>> msync - yes
>>> munmap - probably no (mmap does not return tagged ptrs)
>>> remap_file_pages - no (also deprecated syscall)
>>> shmat, shmdt - shall we allow tagged addresses on shared memory?
>>>
>>> The above is only about the TBI ABI while ignoring hardware MTE. For
>>> the latter, we may want to change the mmap() to allow pre-colouring
>>> on page fault which means that munmap()/mprotect() should also
>>> support tagged pointers. Possibly mremap() as well but we need to
>>> decide whether it should allow re-colouring the page (probably no,
>>> in which case old_address and new_address should have the same tag).
>>> For some of these we'll end up with arm64 specific wrappers again,
>>> unless sparc ADI adopts exactly the same ABI restrictions.
>>
>> Let us keep any restrictions common across ARM64 and sparc. pre-
>> coloring on sparc in the kernel would mean kernel will have to execute
>> stxa instructions in a loop for each page being faulted in.
>
> Since the user can probe the pre-existing colour in a faulted-in page
> (either with some 'ldxa' instruction or by performing a tag-checked
> access), the kernel should always pre-colour (even if colour 0) any
> allocated page. There might not be an obvious security risk but I feel
> uneasy about letting colours leak between address spaces (different user
> processes or between kernel and user).
On sparc, tags 0 and 15 are special in that 0 means untagged memory and
15 means match any tag in the address. Colour 0 is the default for any
newly faulted in page on sparc.
>
> Since we already need such loop in the kernel, we might as well allow
> user space to require a certain colour. This comes in handy for large
> malloc() and another advantage is that the C library won't be stuck
> trying to paint the whole range (think GB).
If kernel is going to pre-color all pages in a vma, we will need to
store the default tag in the vma. It will add more time to page fault
handling code. On sparc M7, kernel will need to execute additional 128
stxa instructions to set the tags on a normal page.
>
>> Not that big a deal but doesn't that assume the entire page has the
>> same tag which is dedcued from the upper bits of address? Shouldn't we
>> support tags at the same granularity level as what the hardware
>> supports?
>
> That's mostly about large malloc() optimisation via mmap(), the latter
> working on page granularity already. There is another use-case for
> pre-coloured thread stacks, also allocated via anonymous mmap().
>
>> We went through this discussion for sparc and decision was to support
>> tags at the same granularity as hardware. That means we can not deduce
>> tags from the first address that pioints into an mmap or shmat region.
>> Those tags and the upper bytes of colored address could change for
>> every cacheline sized block (64-bytes on sparc M7).
>
> It's 16-byte for arm64, so smaller than the cacheline.
>
>> We can try to store tags for an entire region in vma but that is
>> expensive, plus on sparc tags are set in userspace with no
>> participation from kernel and now we need a way for userspace to
>> communicate the tags to kernel.
>
> We can't support finer granularity through the mmap() syscall and, as
> you said, the vma is not the right thing to store the individual tags.
> With the above extension to mmap(), we'd have to store a colour per vma
> and prevent merging if different colours (we could as well use the
> pkeys mechanism we already have in the kernel but use a colour per vma
> instead of a key).
Since tags can change on any part of mmap region on sparc at any time
without kernel being involved, I am not sure I see much reason for
kernel to enforce any tag related restrictions.
>
> Of course, the user is allowed to change the in-memory colours at a
> finer granularity and the kernel will preserve them during swapping
> out/in, page migration etc. The above mmap() proposal is just for the
> first fault-in of a page in a given range/vma.
>
>> From sparc point of view, making kernel responsible for assigning tags
>> to a page on page fault is full of pitfalls.
>
> This could be just some arm64-specific but if you plan to deploy it more
> generically for sparc (at the C library level), you may find this
> useful.
>
Common semantics from app developer point of view will be very useful to
maintain. If arm64 says mmap with MAP_FIXED and a tagged address will
return a pre-colored page, I would rather have it be the same on any
architecture. Is there a use case that justifies kernel doing this extra
work?
--
Khalid
^ permalink raw reply
* [PATCH, RESEND] RDMA/srp: Accept again source addresses that do not have a port number
From: Bart Van Assche @ 2019-05-29 16:38 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Bart Van Assche,
Laurence Oberman, stable
The function srp_parse_in() is used both for parsing source address
specifications and for target address specifications. Target addresses
must have a port number. Having to specify a port number for source
addresses is inconvenient. Make sure that srp_parse_in() supports again
parsing addresses with no port number.
Cc: Laurence Oberman <loberman@redhat.com>
Cc: <stable@vger.kernel.org>
Fixes: c62adb7def71 ("IB/srp: Fix IPv6 address parsing") # v4.17.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index be9ddcad8f28..87848faa7502 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3481,13 +3481,14 @@ static const match_table_t srp_opt_tokens = {
* @net: [in] Network namespace.
* @sa: [out] Address family, IP address and port number.
* @addr_port_str: [in] IP address and port number.
+ * @has_port: [out] Whether or not @addr_port_str includes a port number.
*
* Parse the following address formats:
* - IPv4: <ip_address>:<port>, e.g. 1.2.3.4:5.
* - IPv6: \[<ipv6_address>\]:<port>, e.g. [1::2:3%4]:5.
*/
static int srp_parse_in(struct net *net, struct sockaddr_storage *sa,
- const char *addr_port_str)
+ const char *addr_port_str, bool *has_port)
{
char *addr_end, *addr = kstrdup(addr_port_str, GFP_KERNEL);
char *port_str;
@@ -3496,9 +3497,12 @@ static int srp_parse_in(struct net *net, struct sockaddr_storage *sa,
if (!addr)
return -ENOMEM;
port_str = strrchr(addr, ':');
- if (!port_str)
- return -EINVAL;
- *port_str++ = '\0';
+ if (port_str && strchr(port_str, ']'))
+ port_str = NULL;
+ if (port_str)
+ *port_str++ = '\0';
+ if (has_port)
+ *has_port = port_str != NULL;
ret = inet_pton_with_scope(net, AF_INET, addr, port_str, sa);
if (ret && addr[0]) {
addr_end = addr + strlen(addr) - 1;
@@ -3520,6 +3524,7 @@ static int srp_parse_options(struct net *net, const char *buf,
char *p;
substring_t args[MAX_OPT_ARGS];
unsigned long long ull;
+ bool has_port;
int opt_mask = 0;
int token;
int ret = -EINVAL;
@@ -3618,7 +3623,8 @@ static int srp_parse_options(struct net *net, const char *buf,
ret = -ENOMEM;
goto out;
}
- ret = srp_parse_in(net, &target->rdma_cm.src.ss, p);
+ ret = srp_parse_in(net, &target->rdma_cm.src.ss, p,
+ NULL);
if (ret < 0) {
pr_warn("bad source parameter '%s'\n", p);
kfree(p);
@@ -3634,7 +3640,10 @@ static int srp_parse_options(struct net *net, const char *buf,
ret = -ENOMEM;
goto out;
}
- ret = srp_parse_in(net, &target->rdma_cm.dst.ss, p);
+ ret = srp_parse_in(net, &target->rdma_cm.dst.ss, p,
+ &has_port);
+ if (!has_port)
+ ret = -EINVAL;
if (ret < 0) {
pr_warn("bad dest parameter '%s'\n", p);
kfree(p);
--
2.22.0.rc1
^ permalink raw reply related
* Re: [PATCH iproute2-next 1/4] rdma: Add an option to query,set net namespace sharing sys parameter
From: David Ahern @ 2019-05-29 16:12 UTC (permalink / raw)
To: Parav Pandit; +Cc: netdev, linux-rdma, stephen, leonro
In-Reply-To: <20190521142244.8452-2-parav@mellanox.com>
On 5/21/19 8:22 AM, Parav Pandit wrote:
> diff --git a/rdma/sys.c b/rdma/sys.c
> new file mode 100644
> index 00000000..78e5198f
> --- /dev/null
> +++ b/rdma/sys.c
> @@ -0,0 +1,143 @@
> +/*
> + * sys.c RDMA tool
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
Please use the SPDX line like the other rdma files.
> +
> +#include "rdma.h"
> +
> +static int sys_help(struct rd *rd)
> +{
> + pr_out("Usage: %s system show [ netns ]\n", rd->filename);
> + pr_out(" %s system set netns { shared | exclusive }\n", rd->filename);
> + return 0;
> +}
> +
> +static const char *netns_modes_str[] = {
> + "exclusive",
> + "shared",
> +};
> +
> +static int sys_show_parse_cb(const struct nlmsghdr *nlh, void *data)
> +{
> + struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
> + struct rd *rd = data;
> +
> + mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
> +
> + if (tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]) {
> + const char *mode_str;
> + uint8_t netns_mode;
> +
> + netns_mode =
> + mnl_attr_get_u8(tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]);
> +
> + if (netns_mode <= ARRAY_SIZE(netns_modes_str))
> + mode_str = netns_modes_str[netns_mode];
> + else
> + mode_str = "unknown";
> +
> + if (rd->json_output)
> + jsonw_string_field(rd->jw, "netns", mode_str);
> + else
> + pr_out("netns %s\n", mode_str);
> + }
> + return MNL_CB_OK;
> +}
> +
> +static int sys_show_no_args(struct rd *rd)
> +{
> + uint32_t seq;
> + int ret;
> +
> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_SYS_GET,
> + &seq, (NLM_F_REQUEST | NLM_F_ACK));
> + ret = rd_send_msg(rd);
> + if (ret)
> + return ret;
> +
> + ret = rd_recv_msg(rd, sys_show_parse_cb, rd, seq);
> + return ret;
since you are fixing the header, why not just
return rd_recv_msg(rd, sys_show_parse_cb, rd, seq);
like the other functions?
^ permalink raw reply
* Re: [PATCH] mlx5: avoid 64-bit division
From: Jason Gunthorpe @ 2019-05-29 16:08 UTC (permalink / raw)
To: Michal Kubecek
Cc: Leon Romanovsky, Doug Ledford, Ariel Levkovich, linux-rdma,
linux-kernel
In-Reply-To: <20190520111902.7104DE0184@unicorn.suse.cz>
On Mon, May 20, 2019 at 01:19:02PM +0200, Michal Kubecek wrote:
> Commit 25c13324d03d ("IB/mlx5: Add steering SW ICM device memory type")
> breaks i386 build by introducing three 64-bit divisions. As the divisor
> is MLX5_SW_ICM_BLOCK_SIZE() which is always a power of 2, we can replace
> the division with bit operations.
>
> Fixes: 25c13324d03d ("IB/mlx5: Add steering SW ICM device memory type")
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/hw/mlx5/cmd.c | 9 +++++++--
> drivers/infiniband/hw/mlx5/main.c | 2 +-
> 2 files changed, 8 insertions(+), 3 deletions(-)
Applied to for-rc, thanks
Jason
^ permalink raw reply
* Re: [PATCH rdma-next v2 13/17] RDMA/core: Get sum value of all counters when perform a sysfs stat read
From: Jason Gunthorpe @ 2019-05-29 15:44 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, RDMA mailing list, Majd Dibbiny, Mark Zhang,
Saeed Mahameed, linux-netdev
In-Reply-To: <20190529110524.GU4633@mtr-leonro.mtl.com>
On Wed, May 29, 2019 at 02:05:24PM +0300, Leon Romanovsky wrote:
> On Wed, May 22, 2019 at 02:26:36PM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 29, 2019 at 11:34:49AM +0300, Leon Romanovsky wrote:
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index c56ffc61ab1e..8ae4906a60e7 100644
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -1255,7 +1255,11 @@ int ib_register_device(struct ib_device *device, const char *name)
> > > goto dev_cleanup;
> > > }
> > >
> > > - rdma_counter_init(device);
> > > + ret = rdma_counter_init(device);
> > > + if (ret) {
> > > + dev_warn(&device->dev, "Couldn't initialize counter\n");
> > > + goto sysfs_cleanup;
> > > + }
> >
> > Don't put this things randomly, if there is some reason it should be
> > after sysfs it needs a comment, otherwise if it is just allocating
> > memory it belongs earlier, and the unwind should be done in release.
> >
> > I also think it is very strange/wrong that both sysfs and counters are
> > allocating the same alloc_hw_stats object
> >
> > Why can't they share?
>
> They can, but we wanted to separate "legacy" counters which were exposed
> through sysfs and "new" counters which can be enabled/disable automatically.
Is there any cross contamination through the hw_stats? If no they
should just share.
Jason
^ permalink raw reply
* Re: [PATCH rdma-next v2 13/17] RDMA/core: Get sum value of all counters when perform a sysfs stat read
From: Jason Gunthorpe @ 2019-05-29 15:41 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, RDMA mailing list, Majd Dibbiny, Mark Zhang,
Saeed Mahameed, linux-netdev
In-Reply-To: <20190529111544.GV4633@mtr-leonro.mtl.com>
On Wed, May 29, 2019 at 02:15:44PM +0300, Leon Romanovsky wrote:
> On Wed, May 22, 2019 at 02:10:42PM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 29, 2019 at 11:34:49AM +0300, Leon Romanovsky wrote:
> > > From: Mark Zhang <markz@mellanox.com>
> > >
> > > Since a QP can only be bound to one counter, then if it is bound to a
> > > separate counter, for backward compatibility purpose, the statistic
> > > value must be:
> > > * stat of default counter
> > > + stat of all running allocated counters
> > > + stat of all deallocated counters (history stats)
> > >
> > > Signed-off-by: Mark Zhang <markz@mellanox.com>
> > > Reviewed-by: Majd Dibbiny <majd@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/core/counters.c | 99 +++++++++++++++++++++++++++++-
> > > drivers/infiniband/core/device.c | 8 ++-
> > > drivers/infiniband/core/sysfs.c | 10 ++-
> > > include/rdma/rdma_counter.h | 5 +-
> > > 4 files changed, 113 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
> > > index 36cd9eca1e46..f598b1cdb241 100644
> > > +++ b/drivers/infiniband/core/counters.c
> > > @@ -146,6 +146,20 @@ static int __rdma_counter_bind_qp(struct rdma_counter *counter,
> > > return ret;
> > > }
> > >
> > > +static void counter_history_stat_update(const struct rdma_counter *counter)
> > > +{
> > > + struct ib_device *dev = counter->device;
> > > + struct rdma_port_counter *port_counter;
> > > + int i;
> > > +
> > > + port_counter = &dev->port_data[counter->port].port_counter;
> > > + if (!port_counter->hstats)
> > > + return;
> > > +
> > > + for (i = 0; i < counter->stats->num_counters; i++)
> > > + port_counter->hstats->value[i] += counter->stats->value[i];
> > > +}
> > > +
> > > static int __rdma_counter_unbind_qp(struct ib_qp *qp, bool force)
> > > {
> > > struct rdma_counter *counter = qp->counter;
> > > @@ -285,8 +299,10 @@ int rdma_counter_unbind_qp(struct ib_qp *qp, bool force)
> > > return ret;
> > >
> > > rdma_restrack_put(&counter->res);
> > > - if (atomic_dec_and_test(&counter->usecnt))
> > > + if (atomic_dec_and_test(&counter->usecnt)) {
> > > + counter_history_stat_update(counter);
> > > rdma_counter_dealloc(counter);
> > > + }
> > >
> > > return 0;
> > > }
> > > @@ -307,21 +323,98 @@ int rdma_counter_query_stats(struct rdma_counter *counter)
> > > return ret;
> > > }
> > >
> > > -void rdma_counter_init(struct ib_device *dev)
> > > +static u64 get_running_counters_hwstat_sum(struct ib_device *dev,
> > > + u8 port, u32 index)
> > > +{
> > > + struct rdma_restrack_entry *res;
> > > + struct rdma_restrack_root *rt;
> > > + struct rdma_counter *counter;
> > > + unsigned long id = 0;
> > > + u64 sum = 0;
> > > +
> > > + rt = &dev->res[RDMA_RESTRACK_COUNTER];
> > > + xa_lock(&rt->xa);
> > > + xa_for_each(&rt->xa, id, res) {
> > > + if (!rdma_restrack_get(res))
> > > + continue;
> >
> > Why do we need to get refcounts if we are holding the xa_lock?
>
> Don't we need to protect an entry itself from disappearing?
xa_lock prevents xa_erase and xa_erase should be done before any
parallel kfree.
Jason
^ 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