All of lore.kernel.org
 help / color / mirror / Atom feed
From: Logan Gunthorpe <logang@deltatee.com>
To: Allen Hubbe <Allen.Hubbe@dell.com>, 'Jon Mason' <jdmason@kudzu.us>
Cc: linux-ntb@googlegroups.com, linux-kernel@vger.kernel.org,
	'Dave Jiang' <dave.jiang@intel.com>,
	'Serge Semin' <fancer.lancer@gmail.com>,
	'Kurt Schwemmer' <kurt.schwemmer@microsemi.com>,
	'Stephen Bates' <sbates@raithlin.com>,
	'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>
Subject: Re: New NTB API Issue
Date: Fri, 23 Jun 2017 14:39:01 -0600	[thread overview]
Message-ID: <eeaed83a-cd2b-91f1-e199-eaa6452edd17@deltatee.com> (raw)
In-Reply-To: <000101d2ec53$f2830840$d78918c0$@dell.com>



On 23/06/17 01:07 PM, Allen Hubbe wrote:
> The client's haven't been fully ported to the multi-port api yet.  They were only minimally changed to call the new api, but so far other than that they have only been made to work as they had before.

So is it intended to eventually send the align parameters via spads?
This seems like it would take a lot of spads or multiplexing the spads
with a few doorbells. This gets a bit nasty.


> If those are BARs, that corresponds to "outbound", writing something to the BAR at mwA0.
> A more complete picture might be:
> 
> Host A BARs (aka "outbound" or "peer" memory windows):
> peer_mwA0: resource at 0xA00000000 - 0xA00200000 (2MB)
> peer_mwA1: resource at 0xA10000000 - 0xA10400000 (4MB)
> peer_mwA2: resource at 0xA20000000 - 0xa20010000 (64k)
> 
> Host A MWs (aka "inbound" memory windows):
> mwA0: 64k max size, aligned to 64k, size aligned to 64k
> mwA1: 2MB max size, aligned to 4k, size aligned to 4k

I don't really like the separation of inbound and output as you describe
it. It doesn't really match my hardware. In switchtec, each partition
has some number of BARs and each BAR has a single translation which sets
the peer and destination address. The translation really exists inside
the switch hardware, not on either side. But any translation can be
programmed by any peer. Saying that there's an opposite inbound window
to every outbound window is not an accurate abstraction for us.

I _suspect_ the IDT hardware is similar but, based on Serge's driver, I
think the translation can only be programmed by the peer that the BAR is
resident in (as opposed to from any side like in the switchtec hardwer).
(This poses some problems for getting the IDT code to actually work with
existing clients.)

> Outbound memory (aka "peer mw") windows come with a pci resource.  We can get the size of the resource, it's physical address, and set up outbound translation if the hardware has that (IDT).
> 
> Inbound memory windows (aka "mw") are only used to set up inbound translation, if the hardware has that (Intel, AMD).
> 
> To set up end-to-end memory window so that A can write to B, let's use peer_mwA1 and mwB0.
> 
> A: ntb_peer_mw_get_addr(peer_mwA1) -> base 0xA10000000, size 4MB
> B: ntb_mw_get_align(port4**, mwB0) -> aligned 4k, aligned 4k, max size 1MB
> ** Serge: do we need port info here, why?
> 
> Side A has a resource size of 4MB, but B only supports inbound translation up to 1MB.  Side A can only use the first quarter of the 4MB resource.
> 
> Side B needs to allocate memory aligned to 4k (the dma address must be aligned to 4k after dma mapping), and a multiple of 4k in size.  B may need to set inbound translation so that incoming writes go into this memory.  A may also need to set outbound translation.
> 
> A: ntb_peer_mw_set_trans(port1**, peer_mwA1, dma_mem_addr, dma_mem_size)
> B: ntb_mw_set_trans(port4**, mwB0, dma_mem_addr, dma_mem_size)
> ** Serge: do we also need the opposing side MW index here?
> 
> ** Logan: would those changes to the api suit your needs?

Not really, no. Except for the confusion with the mw_get_align issue the
new API, as it is, suits my hardware well. What you're proposing doesn't
fix my issue and doesn't match my hardware. Though, I interpreted
ntb_peer_mw_set_trans somewhat differently from what you describe. I did
not expect the client would need to call both functions but some clients
could optionally use ntb_peer_mw_set_trans to set the translation from
the opposite side (thus needing to send the DMA address over spads or
msgs). Though, without an actual in-kernel user it's hard to know what
is actually intended.

It's worth noticing that the IDT driver only provides peer_mw_set_trans
and not mw_set_trans. I assumed it's because the hardware's memory
windows can only be configured from the opposite side.

Pragmatically, the only change I need for everything to work as I expect
is for mw_get_align to be called only after link up. However, given all
the confusion I'm wondering if these changes are even ready for
upstream. Without actual in-kernel client code it's hard to know if the
API is correct or that everyone is even interpreting it in the same way.

Logan

  reply	other threads:[~2017-06-23 20:39 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-15 20:37 [RFC PATCH 00/13] Switchtec NTB Support Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 01/13] switchtec: move structure definitions into a common header Logan Gunthorpe
2017-06-17  5:11   ` Greg Kroah-Hartman
2017-06-15 20:37 ` [RFC PATCH 02/13] switchtec: export class symbol for use in upper layer driver Logan Gunthorpe
2017-06-17  5:11   ` Greg Kroah-Hartman
2017-06-17 16:16     ` Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 03/13] switchtec: add ntb hardware register definitions Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 04/13] switchtec: add link event notifier block Logan Gunthorpe
2017-06-17  5:14   ` Greg Kroah-Hartman
2017-06-17 16:20     ` Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 05/13] switchtec_ntb: introduce initial ntb driver Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows Logan Gunthorpe
2017-06-17  5:16   ` Greg Kroah-Hartman
2017-06-17 16:39     ` Logan Gunthorpe
2017-06-18  0:33       ` Greg Kroah-Hartman
2017-06-15 20:37 ` [RFC PATCH 07/13] switchtec_ntb: initialize hardware for doorbells and messages Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 08/13] switchtec_ntb: add skeleton ntb driver Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 09/13] switchtec_ntb: add link management Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 10/13] switchtec_ntb: implement doorbell registers Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 11/13] switchtec_ntb: implement scratchpad registers Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 12/13] switchtec_ntb: add memory window support Logan Gunthorpe
2017-06-15 20:37 ` [RFC PATCH 13/13] switchtec_ntb: update switchtec documentation with notes for ntb Logan Gunthorpe
2017-06-16 13:53 ` [RFC PATCH 00/13] Switchtec NTB Support Allen Hubbe
2017-06-16 13:53   ` Allen Hubbe
2017-06-16 13:53   ` Allen Hubbe
2017-06-16 14:09   ` Logan Gunthorpe
2017-06-16 15:34     ` Allen Hubbe
2017-06-16 15:34       ` Allen Hubbe
2017-06-16 15:34       ` Allen Hubbe
2017-06-16 16:47       ` Logan Gunthorpe
2017-06-16 17:39         ` Serge Semin
2017-06-16 18:08         ` Allen Hubbe
2017-06-16 18:08           ` Allen Hubbe
2017-06-16 18:08           ` Allen Hubbe
2017-06-16 19:17           ` Logan Gunthorpe
2017-06-16 16:33     ` Serge Semin
2017-06-16 17:08       ` Logan Gunthorpe
2017-06-16 18:38         ` Serge Semin
2017-06-16 18:38           ` Serge Semin
2017-06-16 19:34           ` Logan Gunthorpe
2017-06-16 20:21             ` Serge Semin
2017-06-17  5:09               ` 'Greg Kroah-Hartman'
2017-06-17 16:11                 ` Logan Gunthorpe
2017-06-17 16:15                 ` Logan Gunthorpe
2017-06-19 19:14                 ` Jon Mason
2017-06-19 20:07 ` Jon Mason
2017-06-19 20:27   ` Logan Gunthorpe
2017-06-19 21:09     ` Jon Mason
2017-06-22 16:17   ` New NTB API Issue Logan Gunthorpe
2017-06-22 18:32     ` Allen Hubbe
2017-06-22 18:32       ` Allen Hubbe
2017-06-22 18:40       ` Logan Gunthorpe
2017-06-22 22:12         ` Allen Hubbe
2017-06-22 22:12           ` Allen Hubbe
2017-06-22 22:19           ` Logan Gunthorpe
2017-06-22 22:42             ` Allen Hubbe
2017-06-22 22:42               ` Allen Hubbe
2017-06-22 22:49               ` Logan Gunthorpe
2017-06-23 13:18                 ` Allen Hubbe
2017-06-23 13:18                   ` Allen Hubbe
2017-06-23 16:51                   ` Logan Gunthorpe
2017-06-23 19:07                     ` Allen Hubbe
2017-06-23 19:07                       ` Allen Hubbe
2017-06-23 20:39                       ` Logan Gunthorpe [this message]
2017-06-23 22:06                         ` Allen Hubbe
2017-06-23 22:06                           ` Allen Hubbe
2017-06-23 23:06                           ` Logan Gunthorpe
2017-06-23 21:47                       ` Serge Semin
2017-06-23 21:47                         ` Serge Semin
2017-06-23 21:59                         ` Logan Gunthorpe
2017-06-23 22:01                           ` Allen Hubbe
2017-06-23 22:01                             ` Allen Hubbe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eeaed83a-cd2b-91f1-e199-eaa6452edd17@deltatee.com \
    --to=logang@deltatee.com \
    --cc=Allen.Hubbe@dell.com \
    --cc=dave.jiang@intel.com \
    --cc=fancer.lancer@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdmason@kudzu.us \
    --cc=kurt.schwemmer@microsemi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ntb@googlegroups.com \
    --cc=sbates@raithlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.