From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x341.google.com (mail-ot1-x341.google.com. [2607:f8b0:4864:20::341]) by gmr-mx.google.com with ESMTPS id t79si1091967qke.0.2019.02.13.08.08.39 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 13 Feb 2019 08:08:39 -0800 (PST) Received: by mail-ot1-x341.google.com with SMTP id n8so4949648otl.6 for ; Wed, 13 Feb 2019 08:08:39 -0800 (PST) MIME-Version: 1.0 References: <20190212200928.GA8891@leonid-Inspiron-3421> In-Reply-To: <20190212200928.GA8891@leonid-Inspiron-3421> From: Jon Mason Date: Wed, 13 Feb 2019 11:08:27 -0500 Message-ID: Subject: Re: [PATCH v2] NTB: add new parameter to peer_db_addr() db_bit and db_data Content-Type: text/plain; charset="UTF-8" To: Leonid Ravich Cc: linux-ntb List-ID: On Tue, Feb 12, 2019 at 3:09 PM Leonid Ravich wrote: > > Hey > resending this with fixed patch format > > -- Comments that you want ignored need to move after the description of the patch. I removed this by hand. > > NTB door bell usage depends on NTB hardware. > > ex: intel NTB gen1 has one peer door bell register which can be controlled > by the bitmap writen to it, while Intel NTB gen3 has a registers > per door bell and the data trigering the each door bell is always 1. > > therefore exposing only peer door bell address forcing the user > to be aware of such low level details > > Acked-by: Logan Gunthorpe > Acked-by: Dave Jiang > Acked-by: Allen Hubbe These are usually added after your signed-off-by line. I moved this, when I added it to the ntb branch. Thanks, Jon > > Signed-off-by: Leonid Ravich > --- > drivers/ntb/hw/intel/ntb_hw_gen1.c | 25 +++++++++++++++++++------ > drivers/ntb/hw/intel/ntb_hw_gen1.h | 5 +++-- > drivers/ntb/hw/intel/ntb_hw_gen3.c | 33 ++++++++++++++++++++++++++++++++- > drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 9 ++++++++- > include/linux/ntb.h | 10 +++++++--- > 5 files changed, 69 insertions(+), 13 deletions(-) > > diff --git a/drivers/ntb/hw/intel/ntb_hw_gen1.c b/drivers/ntb/hw/intel/ntb_hw_gen1.c > index 2ad263f..bb57ec2 100644 > --- a/drivers/ntb/hw/intel/ntb_hw_gen1.c > +++ b/drivers/ntb/hw/intel/ntb_hw_gen1.c > @@ -180,7 +180,7 @@ int ndev_mw_to_bar(struct intel_ntb_dev *ndev, int idx) > return ndev->reg->mw_bar[idx]; > } > > -static inline int ndev_db_addr(struct intel_ntb_dev *ndev, > +void ndev_db_addr(struct intel_ntb_dev *ndev, > phys_addr_t *db_addr, resource_size_t *db_size, > phys_addr_t reg_addr, unsigned long reg) > { > @@ -196,8 +196,6 @@ static inline int ndev_db_addr(struct intel_ntb_dev *ndev, > *db_size = ndev->reg->db_size; > dev_dbg(&ndev->ntb.pdev->dev, "Peer db size %llx\n", *db_size); > } > - > - return 0; > } > > u64 ndev_db_read(struct intel_ntb_dev *ndev, > @@ -1111,13 +1109,28 @@ int intel_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits) > ndev->self_reg->db_mask); > } > > -int intel_ntb_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr, > - resource_size_t *db_size) > +static int intel_ntb_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr, > + resource_size_t *db_size, u64 *db_data, int db_bit) > { > + u64 db_bits; > struct intel_ntb_dev *ndev = ntb_ndev(ntb); > > - return ndev_db_addr(ndev, db_addr, db_size, ndev->peer_addr, > + if (unlikely(db_bit >= BITS_PER_LONG_LONG)) > + return -EINVAL; > + > + db_bits = BIT_ULL(db_bit); > + > + if (unlikely(db_bits & ~ntb_ndev(ntb)->db_valid_mask)) > + return -EINVAL; > + > + ndev_db_addr(ndev, db_addr, db_size, ndev->peer_addr, > ndev->peer_reg->db_bell); > + > + if (db_data) > + *db_data = db_bits; > + > + > + return 0; > } > > static int intel_ntb_peer_db_set(struct ntb_dev *ntb, u64 db_bits) > diff --git a/drivers/ntb/hw/intel/ntb_hw_gen1.h b/drivers/ntb/hw/intel/ntb_hw_gen1.h > index ad8ec14..544cf5c 100644 > --- a/drivers/ntb/hw/intel/ntb_hw_gen1.h > +++ b/drivers/ntb/hw/intel/ntb_hw_gen1.h > @@ -147,6 +147,9 @@ > int ndev_init_isr(struct intel_ntb_dev *ndev, int msix_min, int msix_max, > int msix_shift, int total_shift); > enum ntb_topo xeon_ppd_topo(struct intel_ntb_dev *ndev, u8 ppd); > +void ndev_db_addr(struct intel_ntb_dev *ndev, > + phys_addr_t *db_addr, resource_size_t *db_size, > + phys_addr_t reg_addr, unsigned long reg); > u64 ndev_db_read(struct intel_ntb_dev *ndev, void __iomem *mmio); > int ndev_db_write(struct intel_ntb_dev *ndev, u64 db_bits, > void __iomem *mmio); > @@ -166,8 +169,6 @@ u64 intel_ntb_link_is_up(struct ntb_dev *ntb, enum ntb_speed *speed, > u64 intel_ntb_db_vector_mask(struct ntb_dev *ntb, int db_vector); > int intel_ntb_db_set_mask(struct ntb_dev *ntb, u64 db_bits); > int intel_ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits); > -int intel_ntb_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr, > - resource_size_t *db_size); > int intel_ntb_spad_is_unsafe(struct ntb_dev *ntb); > int intel_ntb_spad_count(struct ntb_dev *ntb); > u32 intel_ntb_spad_read(struct ntb_dev *ntb, int idx); > diff --git a/drivers/ntb/hw/intel/ntb_hw_gen3.c b/drivers/ntb/hw/intel/ntb_hw_gen3.c > index b3fa247..f475b56 100644 > --- a/drivers/ntb/hw/intel/ntb_hw_gen3.c > +++ b/drivers/ntb/hw/intel/ntb_hw_gen3.c > @@ -532,6 +532,37 @@ static int intel_ntb3_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > return 0; > } > > +int intel_ntb3_peer_db_addr(struct ntb_dev *ntb, phys_addr_t *db_addr, > + resource_size_t *db_size, > + u64 *db_data, int db_bit) > +{ > + phys_addr_t db_addr_base; > + struct intel_ntb_dev *ndev = ntb_ndev(ntb); > + > + if (unlikely(db_bit >= BITS_PER_LONG_LONG)) > + return -EINVAL; > + > + if (unlikely(BIT_ULL(db_bit) & ~ntb_ndev(ntb)->db_valid_mask)) > + return -EINVAL; > + > + ndev_db_addr(ndev, &db_addr_base, db_size, ndev->peer_addr, > + ndev->peer_reg->db_bell); > + > + if (db_addr) { > + *db_addr = db_addr_base + (db_bit * 4); > + dev_dbg(&ndev->ntb.pdev->dev, "Peer db addr %llx db bit %d\n", > + *db_addr, db_bit); > + } > + > + if (db_data) { > + *db_data = 1; > + dev_dbg(&ndev->ntb.pdev->dev, "Peer db data %llx db bit %d\n", > + *db_data, db_bit); > + } > + > + return 0; > +} > + > static int intel_ntb3_peer_db_set(struct ntb_dev *ntb, u64 db_bits) > { > struct intel_ntb_dev *ndev = ntb_ndev(ntb); > @@ -584,7 +615,7 @@ static int intel_ntb3_db_clear(struct ntb_dev *ntb, u64 db_bits) > .db_clear = intel_ntb3_db_clear, > .db_set_mask = intel_ntb_db_set_mask, > .db_clear_mask = intel_ntb_db_clear_mask, > - .peer_db_addr = intel_ntb_peer_db_addr, > + .peer_db_addr = intel_ntb3_peer_db_addr, > .peer_db_set = intel_ntb3_peer_db_set, > .spad_is_unsafe = intel_ntb_spad_is_unsafe, > .spad_count = intel_ntb_spad_count, > diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c > index 5ee5f40..9fa1b99 100644 > --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c > +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c > @@ -707,11 +707,16 @@ static u64 switchtec_ntb_db_read_mask(struct ntb_dev *ntb) > > static int switchtec_ntb_peer_db_addr(struct ntb_dev *ntb, > phys_addr_t *db_addr, > - resource_size_t *db_size) > + resource_size_t *db_size, > + u64 *db_data, > + int db_bit) > { > struct switchtec_ntb *sndev = ntb_sndev(ntb); > unsigned long offset; > > + if (unlikely(db_bit >= BITS_PER_LONG_LONG)) > + return -EINVAL; > + > offset = (unsigned long)sndev->mmio_peer_dbmsg->odb - > (unsigned long)sndev->stdev->mmio; > > @@ -721,6 +726,8 @@ static int switchtec_ntb_peer_db_addr(struct ntb_dev *ntb, > *db_addr = pci_resource_start(ntb->pdev, 0) + offset; > if (db_size) > *db_size = sizeof(u32); > + if (db_data) > + *db_data = BIT_ULL(db_bit) << sndev->db_peer_shift; > > return 0; > } > diff --git a/include/linux/ntb.h b/include/linux/ntb.h > index 181d166..56a92e3 100644 > --- a/include/linux/ntb.h > +++ b/include/linux/ntb.h > @@ -296,7 +296,8 @@ struct ntb_dev_ops { > int (*db_clear_mask)(struct ntb_dev *ntb, u64 db_bits); > > int (*peer_db_addr)(struct ntb_dev *ntb, > - phys_addr_t *db_addr, resource_size_t *db_size); > + phys_addr_t *db_addr, resource_size_t *db_size, > + u64 *db_data, int db_bit); > u64 (*peer_db_read)(struct ntb_dev *ntb); > int (*peer_db_set)(struct ntb_dev *ntb, u64 db_bits); > int (*peer_db_clear)(struct ntb_dev *ntb, u64 db_bits); > @@ -1078,6 +1079,8 @@ static inline int ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits) > * @ntb: NTB device context. > * @db_addr: OUT - The address of the peer doorbell register. > * @db_size: OUT - The number of bytes to write the peer doorbell register. > + * @db_data: OUT - The data of peer doorbell register > + * @db_bit: door bell bit number > * > * Return the address of the peer doorbell register. This may be used, for > * example, by drivers that offload memory copy operations to a dma engine. > @@ -1091,12 +1094,13 @@ static inline int ntb_db_clear_mask(struct ntb_dev *ntb, u64 db_bits) > */ > static inline int ntb_peer_db_addr(struct ntb_dev *ntb, > phys_addr_t *db_addr, > - resource_size_t *db_size) > + resource_size_t *db_size, > + u64 *db_data, int db_bit) > { > if (!ntb->ops->peer_db_addr) > return -EINVAL; > > - return ntb->ops->peer_db_addr(ntb, db_addr, db_size); > + return ntb->ops->peer_db_addr(ntb, db_addr, db_size, db_data, db_bit); > } > > /** > -- > 1.9.3 > > -- > You received this message because you are subscribed to the Google Groups "linux-ntb" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com. > To post to this group, send email to linux-ntb@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20190212200928.GA8891%40leonid-Inspiron-3421. > For more options, visit https://groups.google.com/d/optout.