diff mbox

[03/37] IB/rdmavt: Add protection domain to rdmavt.

Message ID 20151207204309.8144.26707.stgit@phlsvslse11.ph.intel.com (mailing list archive)
State Superseded
Headers show

Commit Message

Dennis Dalessandro Dec. 7, 2015, 8:43 p.m. UTC
Add datastructure for and allocation/deallocation of protection domains for
RDMAVT.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/sw/rdmavt/Makefile |    3 +
 drivers/infiniband/sw/rdmavt/pd.c     |  106 +++++++++++++++++++++++++++++++++
 drivers/infiniband/sw/rdmavt/pd.h     |   61 +++++++++++++++++++
 drivers/infiniband/sw/rdmavt/vt.c     |   12 ++++
 drivers/infiniband/sw/rdmavt/vt.h     |    1 
 include/rdma/rdma_vt.h                |   34 +++++++++--
 6 files changed, 212 insertions(+), 5 deletions(-)
 create mode 100644 drivers/infiniband/sw/rdmavt/pd.c
 create mode 100644 drivers/infiniband/sw/rdmavt/pd.h


--
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

Comments

Hefty, Sean Dec. 7, 2015, 9:13 p.m. UTC | #1
PiArc3RydWN0IGliX3BkICpydnRfYWxsb2NfcGQoc3RydWN0IGliX2RldmljZSAqaWJkZXYsDQo+
ICsJCQkgICBzdHJ1Y3QgaWJfdWNvbnRleHQgKmNvbnRleHQsDQo+ICsJCQkgICBzdHJ1Y3QgaWJf
dWRhdGEgKnVkYXRhKQ0KPiArew0KPiArCXN0cnVjdCBydnRfZGV2X2luZm8gKmRldiA9IGliX3Rv
X3J2dChpYmRldik7DQo+ICsJc3RydWN0IHJ2dF9wZCAqcGQ7DQo+ICsJc3RydWN0IGliX3BkICpy
ZXQ7DQo+ICsNCj4gKwlwZCA9IGttYWxsb2Moc2l6ZW9mKCpwZCksIEdGUF9LRVJORUwpOw0KPiAr
CWlmICghcGQpIHsNCj4gKwkJcmV0ID0gRVJSX1BUUigtRU5PTUVNKTsNCj4gKwkJZ290byBiYWls
Ow0KPiArCX0NCj4gKwkvKg0KPiArCSAqIFRoaXMgaXMgYWN0dWFsbHkgdG90YWxseSBhcmJpdHJh
cnkuICBTb21lIGNvcnJlY3RuZXNzIHRlc3RzDQo+ICsJICogYXNzdW1lIHRoZXJlJ3MgYSBtYXhp
bXVtIG51bWJlciBvZiBQRHMgdGhhdCBjYW4gYmUgYWxsb2NhdGVkLg0KPiArCSAqIFdlIGRvbid0
IGFjdHVhbGx5IGhhdmUgdGhpcyBsaW1pdCwgYnV0IHdlIGZhaWwgdGhlIHRlc3QgaWYNCj4gKwkg
KiB3ZSBhbGxvdyBhbGxvY2F0aW9ucyBvZiBtb3JlIHRoYW4gd2UgcmVwb3J0IGZvciB0aGlzIHZh
bHVlLg0KPiArCSAqLw0KDQpXaHkgbm90IHRyYXAgdGhpcyBpbiB1c2VyIHNwYWNlLCByYXRoZXIg
dGhhbiBmb3JjaW5nIHRoZSBrZXJuZWwgdG8gc3VwcG9ydCBzb21lIHRlc3QgcHJvZ3JhbT8NCg0K
--
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
Leon Romanovsky Dec. 8, 2015, 6:28 a.m. UTC | #2
On Mon, Dec 07, 2015 at 03:43:10PM -0500, Dennis Dalessandro wrote:
> +
> +/*
> + * Things that are driver specific, module parameters in hfi1 and qib
> + */
> +struct rvt_driver_params {
> +	int max_pds;
Can it be negative value?
> +};
--
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
Leon Romanovsky Dec. 8, 2015, 7:20 a.m. UTC | #3
On Tue, Dec 08, 2015 at 08:28:17AM +0200, Leon Romanovsky wrote:
> On Mon, Dec 07, 2015 at 03:43:10PM -0500, Dennis Dalessandro wrote:
> > +
> > +/*
> > + * Things that are driver specific, module parameters in hfi1 and qib
> > + */
> > +struct rvt_driver_params {
> > +	int max_pds;
> Can it be negative value?
> > +};
Forget my question, I see, you removed this variable in the following patch.
--
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
Ira Weiny Dec. 8, 2015, 7:08 p.m. UTC | #4
On Mon, Dec 07, 2015 at 03:13:16PM -0600, Sean Hefty wrote:
> > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
> > +			   struct ib_ucontext *context,
> > +			   struct ib_udata *udata)
> > +{
> > +	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
> > +	struct rvt_pd *pd;
> > +	struct ib_pd *ret;
> > +
> > +	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> > +	if (!pd) {
> > +		ret = ERR_PTR(-ENOMEM);
> > +		goto bail;
> > +	}
> > +	/*
> > +	 * This is actually totally arbitrary.  Some correctness tests
> > +	 * assume there's a maximum number of PDs that can be allocated.
> > +	 * We don't actually have this limit, but we fail the test if
> > +	 * we allow allocations of more than we report for this value.
> > +	 */
> 
> Why not trap this in user space, rather than forcing the kernel to support some test program?
> 

What do you mean "trap this in user space"?  This code is not supporting any
particular test program.

If users try and allocate more protection domains then are advertised then an
error should be returned.

Perhaps the comment should be removed if it is confusing?

Ira

--
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
Hefty, Sean Dec. 8, 2015, 7:17 p.m. UTC | #5
> > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
> > > +			   struct ib_ucontext *context,
> > > +			   struct ib_udata *udata)
> > > +{
> > > +	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
> > > +	struct rvt_pd *pd;
> > > +	struct ib_pd *ret;
> > > +
> > > +	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> > > +	if (!pd) {
> > > +		ret = ERR_PTR(-ENOMEM);
> > > +		goto bail;
> > > +	}
> > > +	/*
> > > +	 * This is actually totally arbitrary.  Some correctness tests
> > > +	 * assume there's a maximum number of PDs that can be allocated.
> > > +	 * We don't actually have this limit, but we fail the test if
> > > +	 * we allow allocations of more than we report for this value.
> > > +	 */
> >
> > Why not trap this in user space, rather than forcing the kernel to
> support some test program?
> >
> 
> What do you mean "trap this in user space"?  This code is not supporting
> any
> particular test program.
> 
> If users try and allocate more protection domains then are advertised then
> an
> error should be returned.
> 
> Perhaps the comment should be removed if it is confusing?

There is no theoretical limit on the number of PDs, or likely most other resources.  So why define and enforce an arbitrary limit?  The justification given was that some test program would fail.  If there's a concern about that test program passing, then enforce the limit in user space.

I didn't find the comment confusing.  Just the choice to let a test app drive the design.
--
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
Ira Weiny Dec. 8, 2015, 7:52 p.m. UTC | #6
On Tue, Dec 08, 2015 at 01:17:48PM -0600, Sean Hefty wrote:
> > > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
> > > > +			   struct ib_ucontext *context,
> > > > +			   struct ib_udata *udata)
> > > > +{
> > > > +	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
> > > > +	struct rvt_pd *pd;
> > > > +	struct ib_pd *ret;
> > > > +
> > > > +	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> > > > +	if (!pd) {
> > > > +		ret = ERR_PTR(-ENOMEM);
> > > > +		goto bail;
> > > > +	}
> > > > +	/*
> > > > +	 * This is actually totally arbitrary.  Some correctness tests
> > > > +	 * assume there's a maximum number of PDs that can be allocated.
> > > > +	 * We don't actually have this limit, but we fail the test if
> > > > +	 * we allow allocations of more than we report for this value.
> > > > +	 */
> > >
> > > Why not trap this in user space, rather than forcing the kernel to
> > support some test program?
> > >
> > 
> > What do you mean "trap this in user space"?  This code is not supporting
> > any
> > particular test program.
> > 
> > If users try and allocate more protection domains then are advertised then
> > an
> > error should be returned.
> > 
> > Perhaps the comment should be removed if it is confusing?
> 
> There is no theoretical limit on the number of PDs, or likely most other
> resources.  So why define and enforce an arbitrary limit?  The justification
> given was that some test program would fail.  If there's a concern about that
> test program passing, then enforce the limit in user space.

I did not interpret this as being driven by a test but rather by the IBTA spec.

This may be pedantic but, wouldn't it be confusing from a user POV to see an
advertised limit but then not be constrained by it?  I'm not opposed to setting
this limit arbitrarily high, but I still think the implementation should
enforce that limit.

> 
> I didn't find the comment confusing.  Just the choice to let a test app drive the design.

Perhaps "confusing" is the wrong word.  But I did not interpreted the comment
as saying that the implementation was driven but a test program.

Ira

--
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
Dennis Dalessandro Dec. 10, 2015, 4:40 p.m. UTC | #7
On Tue, Dec 08, 2015 at 08:28:17AM +0200, Leon Romanovsky wrote:
>On Mon, Dec 07, 2015 at 03:43:10PM -0500, Dennis Dalessandro wrote:
>> +
>> +/*
>> + * Things that are driver specific, module parameters in hfi1 and qib
>> + */
>> +struct rvt_driver_params {
>> +	int max_pds;
>Can it be negative value?
>> +};

If so no protection domains would ever get allocated. I don't think anything 
else would break though if a driver were to do that.

-Denny
--
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
Dennis Dalessandro Dec. 10, 2015, 4:45 p.m. UTC | #8
On Tue, Dec 08, 2015 at 09:20:27AM +0200, Leon Romanovsky wrote:
>On Tue, Dec 08, 2015 at 08:28:17AM +0200, Leon Romanovsky wrote:
>> On Mon, Dec 07, 2015 at 03:43:10PM -0500, Dennis Dalessandro wrote:
>> > +
>> > +/*
>> > + * Things that are driver specific, module parameters in hfi1 and qib
>> > + */
>> > +struct rvt_driver_params {
>> > +	int max_pds;
>> Can it be negative value?
>> > +};
>Forget my question, I see, you removed this variable in the following patch.

Well removed it from the rvt structure, but the concept still exists. I'm 
now using struct ib_device_attr.max_pd.

-Denny
--
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
Dennis Dalessandro Dec. 10, 2015, 4:49 p.m. UTC | #9
On Tue, Dec 08, 2015 at 02:52:36PM -0500, ira. weiny wrote:
>On Tue, Dec 08, 2015 at 01:17:48PM -0600, Sean Hefty wrote:
>> > > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
>> > > > +			   struct ib_ucontext *context,
>> > > > +			   struct ib_udata *udata)
>> > > > +{
>> > > > +	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
>> > > > +	struct rvt_pd *pd;
>> > > > +	struct ib_pd *ret;
>> > > > +
>> > > > +	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
>> > > > +	if (!pd) {
>> > > > +		ret = ERR_PTR(-ENOMEM);
>> > > > +		goto bail;
>> > > > +	}
>> > > > +	/*
>> > > > +	 * This is actually totally arbitrary.  Some correctness tests
>> > > > +	 * assume there's a maximum number of PDs that can be allocated.
>> > > > +	 * We don't actually have this limit, but we fail the test if
>> > > > +	 * we allow allocations of more than we report for this value.
>> > > > +	 */
>> > >
>> > > Why not trap this in user space, rather than forcing the kernel to
>> > support some test program?
>> > >
>> > 
>> > What do you mean "trap this in user space"?  This code is not supporting
>> > any
>> > particular test program.
>> > 
>> > If users try and allocate more protection domains then are advertised then
>> > an
>> > error should be returned.
>> > 
>> > Perhaps the comment should be removed if it is confusing?
>> 
>> There is no theoretical limit on the number of PDs, or likely most other
>> resources.  So why define and enforce an arbitrary limit?  The justification
>> given was that some test program would fail.  If there's a concern about that
>> test program passing, then enforce the limit in user space.
>
>I did not interpret this as being driven by a test but rather by the IBTA spec.
>
>This may be pedantic but, wouldn't it be confusing from a user POV to see an
>advertised limit but then not be constrained by it?  I'm not opposed to setting
>this limit arbitrarily high, but I still think the implementation should
>enforce that limit.
>
>> 
>> I didn't find the comment confusing.  Just the choice to let a test app drive the design.
>
>Perhaps "confusing" is the wrong word.  But I did not interpreted the comment
>as saying that the implementation was driven but a test program.

How about I reword the comment to say that while we could continue 
allocating PDs being only constrained by system resources, the spec says 
there is a limit so we enforce that?

-Denny
--
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
Leon Romanovsky Dec. 10, 2015, 5:54 p.m. UTC | #10
On Thu, Dec 10, 2015 at 11:40:48AM -0500, Dennis Dalessandro wrote:
> On Tue, Dec 08, 2015 at 08:28:17AM +0200, Leon Romanovsky wrote:
> >On Mon, Dec 07, 2015 at 03:43:10PM -0500, Dennis Dalessandro wrote:
> >>+
> >>+/*
> >>+ * Things that are driver specific, module parameters in hfi1 and qib
> >>+ */
> >>+struct rvt_driver_params {
> >>+	int max_pds;
> >Can it be negative value?
> >>+};
> 
> If so no protection domains would ever get allocated. I don't think anything
> else would break though if a driver were to do that.
In such way, it should be "unsigned int".

> 
> -Denny
> --
> 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
--
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
Ira Weiny Dec. 10, 2015, 6:59 p.m. UTC | #11
On Thu, Dec 10, 2015 at 11:49:40AM -0500, Dalessandro, Dennis wrote:
> On Tue, Dec 08, 2015 at 02:52:36PM -0500, ira. weiny wrote:
> >On Tue, Dec 08, 2015 at 01:17:48PM -0600, Sean Hefty wrote:
> >>> > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
> >>> > > +			   struct ib_ucontext *context,
> >>> > > +			   struct ib_udata *udata)
> >>> > > +{
> >>> > > +	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
> >>> > > +	struct rvt_pd *pd;
> >>> > > +	struct ib_pd *ret;
> >>> > > +
> >>> > > +	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> >>> > > +	if (!pd) {
> >>> > > +		ret = ERR_PTR(-ENOMEM);
> >>> > > +		goto bail;
> >>> > > +	}
> >>> > > +	/*
> >>> > > +	 * This is actually totally arbitrary.  Some correctness 
> >>tests
> >>> > > +	 * assume there's a maximum number of PDs that can be 
> >>allocated.
> >>> > > +	 * We don't actually have this limit, but we fail the test if
> >>> > > +	 * we allow allocations of more than we report for this 
> >>value.
> >>> > > +	 */
> >>> >
> >>> > Why not trap this in user space, rather than forcing the kernel to
> >>> support some test program?
> >>> >
> >>> 
> >>> What do you mean "trap this in user space"?  This code is not supporting
> >>> any
> >>> particular test program.
> >>> 
> >>> If users try and allocate more protection domains then are advertised 
> >>then
> >>> an
> >>> error should be returned.
> >>> 
> >>> Perhaps the comment should be removed if it is confusing?
> >>
> >>There is no theoretical limit on the number of PDs, or likely most other
> >>resources.  So why define and enforce an arbitrary limit?  The 
> >>justification
> >>given was that some test program would fail.  If there's a concern about 
> >>that
> >>test program passing, then enforce the limit in user space.
> >
> >I did not interpret this as being driven by a test but rather by the IBTA 
> >spec.
> >
> >This may be pedantic but, wouldn't it be confusing from a user POV to see 
> >an
> >advertised limit but then not be constrained by it?  I'm not opposed to 
> >setting
> >this limit arbitrarily high, but I still think the implementation should
> >enforce that limit.
> >
> >>
> >>I didn't find the comment confusing.  Just the choice to let a test app 
> >>drive the design.
> >
> >Perhaps "confusing" is the wrong word.  But I did not interpreted the 
> >comment
> >as saying that the implementation was driven but a test program.
> 
> How about I reword the comment to say that while we could continue 
> allocating PDs being only constrained by system resources, the spec says 
> there is a limit so we enforce that?

I'm ok with that, Sean?

Ira

> 
> -Denny
--
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
Hefty, Sean Dec. 10, 2015, 7:10 p.m. UTC | #12
> > How about I reword the comment to say that while we could continue
> > allocating PDs being only constrained by system resources, the spec says
> > there is a limit so we enforce that?
> 
> I'm ok with that, Sean?

That's fine
--
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/drivers/infiniband/sw/rdmavt/Makefile b/drivers/infiniband/sw/rdmavt/Makefile
index 134d2d0..c6751bb 100644
--- a/drivers/infiniband/sw/rdmavt/Makefile
+++ b/drivers/infiniband/sw/rdmavt/Makefile
@@ -7,4 +7,5 @@ 
 #
 obj-$(CONFIG_INFINIBAND_RDMAVT) += rdmavt.o
 
-rdmavt-y := vt.o dma.o
+rdmavt-y := vt.o dma.o pd.o
+
diff --git a/drivers/infiniband/sw/rdmavt/pd.c b/drivers/infiniband/sw/rdmavt/pd.c
new file mode 100644
index 0000000..2f7a97f
--- /dev/null
+++ b/drivers/infiniband/sw/rdmavt/pd.c
@@ -0,0 +1,106 @@ 
+/*
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <linux/slab.h>
+#include "pd.h"
+
+struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
+			   struct ib_ucontext *context,
+			   struct ib_udata *udata)
+{
+	struct rvt_dev_info *dev = ib_to_rvt(ibdev);
+	struct rvt_pd *pd;
+	struct ib_pd *ret;
+
+	pd = kmalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		ret = ERR_PTR(-ENOMEM);
+		goto bail;
+	}
+	/*
+	 * This is actually totally arbitrary.  Some correctness tests
+	 * assume there's a maximum number of PDs that can be allocated.
+	 * We don't actually have this limit, but we fail the test if
+	 * we allow allocations of more than we report for this value.
+	 */
+
+	spin_lock(&dev->n_pds_lock);
+	if (dev->n_pds_allocated == dev->dparms.max_pds) {
+		spin_unlock(&dev->n_pds_lock);
+		kfree(pd);
+		ret = ERR_PTR(-ENOMEM);
+		goto bail;
+	}
+
+	dev->n_pds_allocated++;
+	spin_unlock(&dev->n_pds_lock);
+
+	/* ib_alloc_pd() will initialize pd->ibpd. */
+	pd->user = udata ? 1 : 0;
+
+	ret = &pd->ibpd;
+
+bail:
+	return ret;
+}
+
+int rvt_dealloc_pd(struct ib_pd *ibpd)
+{
+	struct rvt_pd *pd = ibpd_to_rvtpd(ibpd);
+	struct rvt_dev_info *dev = ib_to_rvt(ibpd->device);
+
+	spin_lock(&dev->n_pds_lock);
+	dev->n_pds_allocated--;
+	spin_unlock(&dev->n_pds_lock);
+
+	kfree(pd);
+
+	return 0;
+}
diff --git a/drivers/infiniband/sw/rdmavt/pd.h b/drivers/infiniband/sw/rdmavt/pd.h
new file mode 100644
index 0000000..d501c38
--- /dev/null
+++ b/drivers/infiniband/sw/rdmavt/pd.h
@@ -0,0 +1,61 @@ 
+#ifndef DEF_RDMAVTPD_H
+#define DEF_RDMAVTPD_H
+
+/*
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <rdma/rdma_vt.h>
+
+struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
+			   struct ib_ucontext *context,
+			   struct ib_udata *udata);
+int rvt_dealloc_pd(struct ib_pd *ibpd);
+
+#endif          /* DEF_RDMAVTPD_H */
diff --git a/drivers/infiniband/sw/rdmavt/vt.c b/drivers/infiniband/sw/rdmavt/vt.c
index c0f5988..fc977b3 100644
--- a/drivers/infiniband/sw/rdmavt/vt.c
+++ b/drivers/infiniband/sw/rdmavt/vt.c
@@ -82,9 +82,21 @@  int rvt_register_device(struct rvt_dev_info *rdi)
 	 * come up with a better mechanism that simplifies the code at some
 	 * point.
 	 */
+
+	/* DMA Operations */
 	rdi->ibdev.dma_ops =
 		rdi->ibdev.dma_ops ? : &rvt_default_dma_mapping_ops;
 
+	/* Protection Domain */
+	rdi->ibdev.alloc_pd =
+		rdi->ibdev.alloc_pd ? : rvt_alloc_pd;
+	rdi->ibdev.dealloc_pd =
+		rdi->ibdev.dealloc_pd ? : rvt_dealloc_pd;
+
+	spin_lock_init(&rdi->n_pds_lock);
+	rdi->n_pds_allocated = 0;
+
+	/* We are now good to announce we exist */
 	return ib_register_device(&rdi->ibdev, rdi->port_callback);
 }
 EXPORT_SYMBOL(rvt_register_device);
diff --git a/drivers/infiniband/sw/rdmavt/vt.h b/drivers/infiniband/sw/rdmavt/vt.h
index a19a3af..0d3cc79 100644
--- a/drivers/infiniband/sw/rdmavt/vt.h
+++ b/drivers/infiniband/sw/rdmavt/vt.h
@@ -53,5 +53,6 @@ 
 
 #include <rdma/rdma_vt.h>
 #include "dma.h"
+#include "pd.h"
 
 #endif          /* DEF_RDMAVT_H */
diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h
index da87fb2..b4ddcd3 100644
--- a/include/rdma/rdma_vt.h
+++ b/include/rdma/rdma_vt.h
@@ -57,16 +57,42 @@ 
  */
 
 #include "ib_verbs.h"
+
+/*
+ * Things that are driver specific, module parameters in hfi1 and qib
+ */
+struct rvt_driver_params {
+	int max_pds;
+};
+
+/* Protection domain */
+struct rvt_pd {
+	struct ib_pd ibpd;
+	int user;               /* non-zero if created from user space */
+};
+
 struct rvt_dev_info {
 	struct ib_device ibdev;
+
+	/* Driver specific */
+	struct rvt_driver_params dparms;
 	int (*port_callback)(struct ib_device *, u8, struct kobject *);
 
-	/*
-	 * TODO:
-	 *	need to reflect module parameters that may vary by dev
-	 */
+	/* Internal use */
+	int n_pds_allocated;
+	spinlock_t n_pds_lock; /* Protect pd allocated count */
 };
 
+static inline struct rvt_pd *ibpd_to_rvtpd(struct ib_pd *ibpd)
+{
+	return container_of(ibpd, struct rvt_pd, ibpd);
+}
+
+static inline struct rvt_dev_info *ib_to_rvt(struct ib_device *ibdev)
+{
+	return  container_of(ibdev, struct rvt_dev_info, ibdev);
+}
+
 int rvt_register_device(struct rvt_dev_info *rvd);
 void rvt_unregister_device(struct rvt_dev_info *rvd);