diff mbox

[v2,for-rc,3/4] RDMA/vmw_pvrdma: Add UAR SRQ macros in ABI header file

Message ID 20171220175051.GA22751@bryantan-devbox.prom.eng.vmware.com.prom.eng.vmware.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show

Commit Message

Bryan Tan Dec. 20, 2017, 5:50 p.m. UTC
Support for SRQs were added in the vmw_pvrdma userlevel library
before two necessary macros were added into the kernel ABI header
file. Add the two UAR SRQ macros that are required by the userlevel
library so that the library can rely on the kernel ABI header file
for these SRQ macro definitions.

Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
Reviewed-by: Adit Ranadive <aditr@vmware.com>
Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
Signed-off-by: Bryan Tan <bryantan@vmware.com>
---
 include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Leon Romanovsky Dec. 21, 2017, 12:59 p.m. UTC | #1
On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
>
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr@vmware.com>
> Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Bryan Tan <bryantan@vmware.com>
> ---
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> index aaa352f..4007cac 100644
> --- a/include/uapi/rdma/vmw_pvrdma-abi.h
> +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> @@ -58,6 +58,8 @@
>  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
>  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
>  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
>

Didn't we see that BIT() in UAPI is a wrong thing to do? Why don't you
add normal (1 << 30) ?

>  enum pvrdma_wr_opcode {
>  	PVRDMA_WR_RDMA_WRITE,
> --
> 1.8.5.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 22, 2017, 4:17 p.m. UTC | #2
On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
> 
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr@vmware.com>
> Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Bryan Tan <bryantan@vmware.com>
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> index aaa352f..4007cac 100644
> +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> @@ -58,6 +58,8 @@
>  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
>  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
>  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */

Thinking about this.. These constants are not used in the kernel, so
what are they for, and why are they a 'uapi' header?

I'm guessing this is the 'PCI BAR' ABI and the kernel does't use UAR?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 28, 2017, 4:48 a.m. UTC | #3
On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> Support for SRQs were added in the vmw_pvrdma userlevel library
> before two necessary macros were added into the kernel ABI header
> file. Add the two UAR SRQ macros that are required by the userlevel
> library so that the library can rely on the kernel ABI header file
> for these SRQ macro definitions.
> 
> Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> Reviewed-by: Adit Ranadive <aditr@vmware.com>
> Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Bryan Tan <bryantan@vmware.com>
>  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
>  1 file changed, 2 insertions(+)

I decided to put this into for-next, since I really didn't think I
could defend it in -rc, as the kernel doesn't need or use the header
contents.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adit Ranadive Dec. 28, 2017, 5:38 p.m. UTC | #4
On Fri, Dec 22, 2017 at 09:17:24AM -0700, Jason Gunthorpe wrote:
> On Wed, Dec 20, 2017 at 09:50:57AM -0800, Bryan Tan wrote:
> > Support for SRQs were added in the vmw_pvrdma userlevel library
> > before two necessary macros were added into the kernel ABI header
> > file. Add the two UAR SRQ macros that are required by the userlevel
> > library so that the library can rely on the kernel ABI header file
> > for these SRQ macro definitions.
> > 
> > Fixes: 8b10ba783c9d ("RDMA/vmw_pvrdma: Add shared receive queue support")
> > Reviewed-by: Adit Ranadive <aditr@vmware.com>
> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> > Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> > Signed-off-by: Bryan Tan <bryantan@vmware.com>
> >  include/uapi/rdma/vmw_pvrdma-abi.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
> > index aaa352f..4007cac 100644
> > +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> > @@ -58,6 +58,8 @@
> >  #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
> >  #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
> >  #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
> > +#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
> > +#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
> 
> Thinking about this.. These constants are not used in the kernel, so
> what are they for, and why are they a 'uapi' header?
> 
> I'm guessing this is the 'PCI BAR' ABI and the kernel does't use UAR?

Hi Jason,

Sorry for the late reply on this thread. We do create a UAR page for the kernel
clients to use. Currently, we don't use the SRQ bits in the kernel and only in 
userspace. But all the other offsets, bits are used when writing to the driver
specific UAR page.

Thanks,
Adit
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
index aaa352f..4007cac 100644
--- a/include/uapi/rdma/vmw_pvrdma-abi.h
+++ b/include/uapi/rdma/vmw_pvrdma-abi.h
@@ -58,6 +58,8 @@ 
 #define PVRDMA_UAR_CQ_ARM_SOL		BIT(29)		/* Arm solicited bit. */
 #define PVRDMA_UAR_CQ_ARM		BIT(30)		/* Arm bit. */
 #define PVRDMA_UAR_CQ_POLL		BIT(31)		/* Poll bit. */
+#define PVRDMA_UAR_SRQ_OFFSET		8		/* SRQ doorbell. */
+#define PVRDMA_UAR_SRQ_RECV		BIT(30)		/* Recv bit. */
 
 enum pvrdma_wr_opcode {
 	PVRDMA_WR_RDMA_WRITE,