LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 5/7] ocxl: afu_irq only deals with IRQ IDs, not offsets
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

The use of offsets is required only in the frontend, so alter
the IRQ API to only work with IRQ IDs in the backend.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/afu_irq.c       | 34 +++++++++++++++----------------
 drivers/misc/ocxl/context.c       |  7 +++++--
 drivers/misc/ocxl/file.c          | 13 +++++++-----
 drivers/misc/ocxl/ocxl_internal.h | 10 +++++----
 drivers/misc/ocxl/trace.h         | 12 ++++-------
 5 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index 11ab996657a2..2d410cd6f817 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -14,14 +14,14 @@ struct afu_irq {
 	struct eventfd_ctx *ev_ctx;
 };
 
-static int irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
+int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
 {
 	return (offset - ctx->afu->irq_base_offset) >> PAGE_SHIFT;
 }
 
-static u64 irq_id_to_offset(struct ocxl_context *ctx, int id)
+u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
 {
-	return ctx->afu->irq_base_offset + (id << PAGE_SHIFT);
+	return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
 }
 
 static irqreturn_t afu_irq_handler(int virq, void *data)
@@ -69,7 +69,7 @@ static void release_afu_irq(struct afu_irq *irq)
 	kfree(irq->name);
 }
 
-int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
 {
 	struct afu_irq *irq;
 	int rc;
@@ -101,11 +101,11 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
 	if (rc)
 		goto err_alloc;
 
-	*irq_offset = irq_id_to_offset(ctx, irq->id);
-
-	trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq,
-				*irq_offset);
+	trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq);
 	mutex_unlock(&ctx->irq_lock);
+
+	*irq_id = irq->id;
+
 	return 0;
 
 err_alloc:
@@ -123,7 +123,7 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
 	trace_ocxl_afu_irq_free(ctx->pasid, irq->id);
 	if (ctx->mapping)
 		unmap_mapping_range(ctx->mapping,
-				irq_id_to_offset(ctx, irq->id),
+				ocxl_irq_id_to_offset(ctx, irq->id),
 				1 << PAGE_SHIFT, 1);
 	release_afu_irq(irq);
 	if (irq->ev_ctx)
@@ -132,14 +132,13 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
 	kfree(irq);
 }
 
-int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset)
+int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id)
 {
 	struct afu_irq *irq;
-	int id = irq_offset_to_id(ctx, irq_offset);
 
 	mutex_lock(&ctx->irq_lock);
 
-	irq = idr_find(&ctx->irq_idr, id);
+	irq = idr_find(&ctx->irq_idr, irq_id);
 	if (!irq) {
 		mutex_unlock(&ctx->irq_lock);
 		return -EINVAL;
@@ -161,14 +160,14 @@ void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
 	mutex_unlock(&ctx->irq_lock);
 }
 
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id, int eventfd)
 {
 	struct afu_irq *irq;
 	struct eventfd_ctx *ev_ctx;
-	int rc = 0, id = irq_offset_to_id(ctx, irq_offset);
+	int rc = 0;
 
 	mutex_lock(&ctx->irq_lock);
-	irq = idr_find(&ctx->irq_idr, id);
+	irq = idr_find(&ctx->irq_idr, irq_id);
 	if (!irq) {
 		rc = -EINVAL;
 		goto unlock;
@@ -186,14 +185,13 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
 	return rc;
 }
 
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset)
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
 {
 	struct afu_irq *irq;
-	int id = irq_offset_to_id(ctx, irq_offset);
 	u64 addr = 0;
 
 	mutex_lock(&ctx->irq_lock);
-	irq = idr_find(&ctx->irq_idr, id);
+	irq = idr_find(&ctx->irq_idr, irq_id);
 	if (irq)
 		addr = irq->trigger_page;
 	mutex_unlock(&ctx->irq_lock);
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 9a37e9632cd9..c04887591837 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -93,8 +93,9 @@ static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
 		u64 offset, struct ocxl_context *ctx)
 {
 	u64 trigger_addr;
+	int irq_id = ocxl_irq_offset_to_id(ctx, offset);
 
-	trigger_addr = ocxl_afu_irq_get_addr(ctx, offset);
+	trigger_addr = ocxl_afu_irq_get_addr(ctx, irq_id);
 	if (!trigger_addr)
 		return VM_FAULT_SIGBUS;
 
@@ -154,12 +155,14 @@ static const struct vm_operations_struct ocxl_vmops = {
 static int check_mmap_afu_irq(struct ocxl_context *ctx,
 			struct vm_area_struct *vma)
 {
+	int irq_id = ocxl_irq_offset_to_id(ctx, vma->vm_pgoff << PAGE_SHIFT);
+
 	/* only one page */
 	if (vma_pages(vma) != 1)
 		return -EINVAL;
 
 	/* check offset validty */
-	if (!ocxl_afu_irq_get_addr(ctx, vma->vm_pgoff << PAGE_SHIFT))
+	if (!ocxl_afu_irq_get_addr(ctx, irq_id))
 		return -EINVAL;
 
 	/*
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 665422c6c8a0..d13297336253 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -196,6 +196,7 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 {
 	struct ocxl_context *ctx = file->private_data;
 	struct ocxl_ioctl_irq_fd irq_fd;
+	int irq_id;
 	u64 irq_offset;
 	long rc;
 	bool closed;
@@ -217,12 +218,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 		break;
 
 	case OCXL_IOCTL_IRQ_ALLOC:
-		rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
+		rc = ocxl_afu_irq_alloc(ctx, &irq_id);
 		if (!rc) {
+			irq_offset = ocxl_irq_id_to_offset(ctx, irq_id);
 			rc = copy_to_user((u64 __user *) args, &irq_offset,
 					sizeof(irq_offset));
 			if (rc) {
-				ocxl_afu_irq_free(ctx, irq_offset);
+				ocxl_afu_irq_free(ctx, irq_id);
 				return -EFAULT;
 			}
 		}
@@ -233,7 +235,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 				sizeof(irq_offset));
 		if (rc)
 			return -EFAULT;
-		rc = ocxl_afu_irq_free(ctx, irq_offset);
+		irq_id = ocxl_irq_offset_to_id(ctx, irq_offset);
+		rc = ocxl_afu_irq_free(ctx, irq_id);
 		break;
 
 	case OCXL_IOCTL_IRQ_SET_FD:
@@ -243,8 +246,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 		if (irq_fd.reserved)
 			return -EINVAL;
-		rc = ocxl_afu_irq_set_fd(ctx, irq_fd.irq_offset,
-					irq_fd.eventfd);
+		irq_id = ocxl_irq_offset_to_id(ctx, irq_fd.irq_offset);
+		rc = ocxl_afu_irq_set_fd(ctx, irq_id, irq_fd.eventfd);
 		break;
 
 	case OCXL_IOCTL_GET_METADATA:
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 4fc7e9597ede..871155b464da 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -140,12 +140,14 @@ void ocxl_context_detach_all(struct ocxl_afu *afu);
 int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
 void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
 
-int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
-int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
+int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
+u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
 void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id,
 			int eventfd);
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
 
 /**
  * Free an AFU
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index 68bf2f173a1a..a081e644f5f3 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -107,16 +107,14 @@ DEFINE_EVENT(ocxl_fault_handler, ocxl_fault_ack,
 );
 
 TRACE_EVENT(ocxl_afu_irq_alloc,
-	TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq,
-		u64 irq_offset),
-	TP_ARGS(pasid, irq_id, virq, hw_irq, irq_offset),
+	TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq),
+	TP_ARGS(pasid, irq_id, virq, hw_irq),
 
 	TP_STRUCT__entry(
 		__field(int, pasid)
 		__field(int, irq_id)
 		__field(unsigned int, virq)
 		__field(int, hw_irq)
-		__field(u64, irq_offset)
 	),
 
 	TP_fast_assign(
@@ -124,15 +122,13 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
 		__entry->irq_id = irq_id;
 		__entry->virq = virq;
 		__entry->hw_irq = hw_irq;
-		__entry->irq_offset = irq_offset;
 	),
 
-	TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d irq_offset=%#llx",
+	TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d",
 		__entry->pasid,
 		__entry->irq_id,
 		__entry->virq,
-		__entry->hw_irq,
-		__entry->irq_offset
+		__entry->hw_irq
 	)
 );
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 4/7] ocxl: Allow external drivers to use OpenCAPI contexts
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
	Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

Most OpenCAPI operations require a valid context, so
exposing these functions to external drivers is necessary.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 drivers/misc/ocxl/context.c       |  9 +++++--
 drivers/misc/ocxl/file.c          |  2 +-
 drivers/misc/ocxl/ocxl_internal.h |  6 -----
 include/misc/ocxl.h               | 45 +++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 371ef17bba33..9a37e9632cd9 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -8,6 +8,7 @@ struct ocxl_context *ocxl_context_alloc(void)
 {
 	return kzalloc(sizeof(struct ocxl_context), GFP_KERNEL);
 }
+EXPORT_SYMBOL_GPL(ocxl_context_alloc);
 
 int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
 		struct address_space *mapping)
@@ -43,6 +44,7 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
 	ocxl_afu_get(afu);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ocxl_context_init);
 
 /*
  * Callback for when a translation fault triggers an error
@@ -63,7 +65,7 @@ static void xsl_fault_error(void *data, u64 addr, u64 dsisr)
 	wake_up_all(&ctx->events_wq);
 }
 
-int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
 {
 	int rc;
 
@@ -75,7 +77,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
 	}
 
 	rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid,
-			current->mm->context.id, ctx->tidr, amr, current->mm,
+			mm->context.id, ctx->tidr, amr, mm,
 			xsl_fault_error, ctx);
 	if (rc)
 		goto out;
@@ -85,6 +87,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
 	mutex_unlock(&ctx->status_mutex);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(ocxl_context_attach);
 
 static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
 		u64 offset, struct ocxl_context *ctx)
@@ -243,6 +246,7 @@ int ocxl_context_detach(struct ocxl_context *ctx)
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ocxl_context_detach);
 
 void ocxl_context_detach_all(struct ocxl_afu *afu)
 {
@@ -280,3 +284,4 @@ void ocxl_context_free(struct ocxl_context *ctx)
 	ocxl_afu_put(ctx->afu);
 	kfree(ctx);
 }
+EXPORT_SYMBOL_GPL(ocxl_context_free);
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 1f17f8706e29..665422c6c8a0 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -94,7 +94,7 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
 		return -EINVAL;
 
 	amr = arg.amr & mfspr(SPRN_UAMOR);
-	rc = ocxl_context_attach(ctx, amr);
+	rc = ocxl_context_attach(ctx, amr, current->mm);
 	return rc;
 }
 
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 05930a29f606..4fc7e9597ede 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -133,15 +133,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
  */
 int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
 
-struct ocxl_context *ocxl_context_alloc(void);
-int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
-			struct address_space *mapping);
-int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
 int ocxl_context_mmap(struct ocxl_context *ctx,
 			struct vm_area_struct *vma);
-int ocxl_context_detach(struct ocxl_context *ctx);
 void ocxl_context_detach_all(struct ocxl_afu *afu);
-void ocxl_context_free(struct ocxl_context *ctx);
 
 int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
 void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 7d611ea68a59..a5e94da6c8db 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -116,6 +116,51 @@ const struct ocxl_fn_config *ocxl_function_config(struct ocxl_fn *fn);
  */
 void ocxl_function_close(struct ocxl_fn *fn);
 
+// Context allocation
+
+/**
+ * Allocate space for a new OpenCAPI context
+ *
+ * Returns NULL on failure
+ */
+struct ocxl_context *ocxl_context_alloc(void);
+
+/**
+ * Initialize an OpenCAPI context
+ *
+ * @ctx: The OpenCAPI context to initialize
+ * @afu: The AFU the context belongs to
+ * @mapping: The mapping to unmap when the context is closed (may be NULL)
+ */
+int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
+			struct address_space *mapping);
+
+/**
+ * Free an OpenCAPI context
+ *
+ * @ctx: The OpenCAPI context to free
+ */
+void ocxl_context_free(struct ocxl_context *ctx);
+
+/**
+ * Grant access to an MM to an OpenCAPI context
+ * @ctx: The OpenCAPI context to attach
+ * @amr: The value of the AMR register to restrict access
+ * @mm: The mm to attach to the context
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
+				struct mm_struct *mm);
+
+/**
+ * Detach an MM from an OpenCAPI context
+ * @ctx: The OpenCAPI context to attach
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ocxl_context_detach(struct ocxl_context *ctx);
+
 // AFU Metadata
 
 /**
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 3/7] ocxl: Create a clear delineation between ocxl backend & frontend
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

The OCXL driver contains both frontend code for interacting with userspace,
as well as backend code for interacting with the hardware.

This patch separates the backend code from the frontend so that it can be
used by other device drivers that communicate via OpenCAPI.

Relocate dev, cdev & sysfs files to the frontend code to allow external
drivers to maintain their own devices.

Reference counting on the device in the backend is replaced with kref
counting.

Move file & sysfs layer initialisation from core.c (backend) to
pci.c (frontend).

Create an ocxl_function oriented interface for initing devices &
enumerating AFUs.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/context.c       |   2 +-
 drivers/misc/ocxl/core.c          | 205 +++++++++++++++++++-----------
 drivers/misc/ocxl/file.c          | 125 ++++++++++++------
 drivers/misc/ocxl/ocxl_internal.h |  39 +++---
 drivers/misc/ocxl/pci.c           |  61 ++++-----
 drivers/misc/ocxl/sysfs.c         |  58 +++++----
 include/misc/ocxl.h               | 121 ++++++++++++++++--
 7 files changed, 416 insertions(+), 195 deletions(-)

diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 3498a0199bde..371ef17bba33 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -238,7 +238,7 @@ int ocxl_context_detach(struct ocxl_context *ctx)
 	}
 	rc = ocxl_link_remove_pe(ctx->afu->fn->link, ctx->pasid);
 	if (rc) {
-		dev_warn(&ctx->afu->dev,
+		dev_warn(&dev->dev,
 			"Couldn't remove PE entry cleanly: %d\n", rc);
 	}
 	return 0;
diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
index 2fd0c700e8a0..c632ec372342 100644
--- a/drivers/misc/ocxl/core.c
+++ b/drivers/misc/ocxl/core.c
@@ -13,16 +13,6 @@ static void ocxl_fn_put(struct ocxl_fn *fn)
 	put_device(&fn->dev);
 }
 
-struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
-{
-	return (get_device(&afu->dev) == NULL) ? NULL : afu;
-}
-
-void ocxl_afu_put(struct ocxl_afu *afu)
-{
-	put_device(&afu->dev);
-}
-
 static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
 {
 	struct ocxl_afu *afu;
@@ -31,6 +21,7 @@ static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
 	if (!afu)
 		return NULL;
 
+	kref_init(&afu->kref);
 	mutex_init(&afu->contexts_lock);
 	mutex_init(&afu->afu_control_lock);
 	idr_init(&afu->contexts_idr);
@@ -39,32 +30,26 @@ static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
 	return afu;
 }
 
-static void free_afu(struct ocxl_afu *afu)
+static void afu_release(struct kref *kref)
 {
+	struct ocxl_afu *afu = container_of(kref, struct ocxl_afu, kref);
+
 	idr_destroy(&afu->contexts_idr);
 	ocxl_fn_put(afu->fn);
 	kfree(afu);
 }
 
-static void free_afu_dev(struct device *dev)
+void ocxl_afu_get(struct ocxl_afu *afu)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(dev);
-
-	ocxl_unregister_afu(afu);
-	free_afu(afu);
+	kref_get(&afu->kref);
 }
+EXPORT_SYMBOL_GPL(ocxl_afu_get);
 
-static int set_afu_device(struct ocxl_afu *afu, const char *location)
+void ocxl_afu_put(struct ocxl_afu *afu)
 {
-	struct ocxl_fn *fn = afu->fn;
-	int rc;
-
-	afu->dev.parent = &fn->dev;
-	afu->dev.release = free_afu_dev;
-	rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
-		afu->config.idx);
-	return rc;
+	kref_put(&afu->kref, afu_release);
 }
+EXPORT_SYMBOL_GPL(ocxl_afu_put);
 
 static int assign_afu_actag(struct ocxl_afu *afu)
 {
@@ -233,27 +218,25 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
 	if (rc)
 		return rc;
 
-	rc = set_afu_device(afu, dev_name(&dev->dev));
-	if (rc)
-		return rc;
-
 	rc = assign_afu_actag(afu);
 	if (rc)
 		return rc;
 
 	rc = assign_afu_pasid(afu);
-	if (rc) {
-		reclaim_afu_actag(afu);
-		return rc;
-	}
+	if (rc)
+		goto err_free_actag;
 
 	rc = map_mmio_areas(afu);
-	if (rc) {
-		reclaim_afu_pasid(afu);
-		reclaim_afu_actag(afu);
-		return rc;
-	}
+	if (rc)
+		goto err_free_pasid;
+
 	return 0;
+
+err_free_pasid:
+	reclaim_afu_pasid(afu);
+err_free_actag:
+	reclaim_afu_actag(afu);
+	return rc;
 }
 
 static void deconfigure_afu(struct ocxl_afu *afu)
@@ -265,16 +248,8 @@ static void deconfigure_afu(struct ocxl_afu *afu)
 
 static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
 {
-	int rc;
-
 	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
-	/*
-	 * Char device creation is the last step, as processes can
-	 * call our driver immediately, so all our inits must be finished.
-	 */
-	rc = ocxl_create_cdev(afu);
-	if (rc)
-		return rc;
+
 	return 0;
 }
 
@@ -282,11 +257,10 @@ static void deactivate_afu(struct ocxl_afu *afu)
 {
 	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
 
-	ocxl_destroy_cdev(afu);
 	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
 }
 
-int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
+static int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
 {
 	int rc;
 	struct ocxl_afu *afu;
@@ -297,41 +271,29 @@ int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
 
 	rc = configure_afu(afu, afu_idx, dev);
 	if (rc) {
-		free_afu(afu);
+		ocxl_afu_put(afu);
 		return rc;
 	}
 
-	rc = ocxl_register_afu(afu);
-	if (rc)
-		goto err;
-
-	rc = ocxl_sysfs_add_afu(afu);
-	if (rc)
-		goto err;
-
 	rc = activate_afu(dev, afu);
-	if (rc)
-		goto err_sys;
+	if (rc) {
+		deconfigure_afu(afu);
+		ocxl_afu_put(afu);
+		return rc;
+	}
 
 	list_add_tail(&afu->list, &fn->afu_list);
-	return 0;
 
-err_sys:
-	ocxl_sysfs_remove_afu(afu);
-err:
-	deconfigure_afu(afu);
-	device_unregister(&afu->dev);
-	return rc;
+	return 0;
 }
 
-void remove_afu(struct ocxl_afu *afu)
+static void remove_afu(struct ocxl_afu *afu)
 {
 	list_del(&afu->list);
 	ocxl_context_detach_all(afu);
 	deactivate_afu(afu);
-	ocxl_sysfs_remove_afu(afu);
 	deconfigure_afu(afu);
-	device_unregister(&afu->dev);
+	ocxl_afu_put(afu); // matches the implicit get in alloc_afu
 }
 
 static struct ocxl_fn *alloc_function(void)
@@ -358,7 +320,7 @@ static void free_function(struct ocxl_fn *fn)
 
 static void free_function_dev(struct device *dev)
 {
-	struct ocxl_fn *fn = to_ocxl_function(dev);
+	struct ocxl_fn *fn = container_of(dev, struct ocxl_fn, dev);
 
 	free_function(fn);
 }
@@ -372,7 +334,6 @@ static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
 	rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
 	if (rc)
 		return rc;
-	pci_set_drvdata(dev, fn);
 	return 0;
 }
 
@@ -490,7 +451,7 @@ static void deconfigure_function(struct ocxl_fn *fn)
 	pci_disable_device(dev);
 }
 
-struct ocxl_fn *init_function(struct pci_dev *dev)
+static struct ocxl_fn *init_function(struct pci_dev *dev)
 {
 	struct ocxl_fn *fn;
 	int rc;
@@ -514,8 +475,104 @@ struct ocxl_fn *init_function(struct pci_dev *dev)
 	return fn;
 }
 
-void remove_function(struct ocxl_fn *fn)
+// Device detection & initialisation
+
+struct ocxl_fn *ocxl_function_open(struct pci_dev *dev)
+{
+	int rc, afu_count = 0;
+	u8 afu;
+	struct ocxl_fn *fn;
+
+	if (!radix_enabled()) {
+		dev_err(&dev->dev, "Unsupported memory model (hash)\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	fn = init_function(dev);
+	if (IS_ERR(fn)) {
+		dev_err(&dev->dev, "function init failed: %li\n",
+			PTR_ERR(fn));
+		return fn;
+	}
+
+	for (afu = 0; afu <= fn->config.max_afu_index; afu++) {
+		rc = ocxl_config_check_afu_index(dev, &fn->config, afu);
+		if (rc > 0) {
+			rc = init_afu(dev, fn, afu);
+			if (rc) {
+				dev_err(&dev->dev,
+					"Can't initialize AFU index %d\n", afu);
+				continue;
+			}
+			afu_count++;
+		}
+	}
+	dev_info(&dev->dev, "%d AFU(s) configured\n", afu_count);
+	return fn;
+}
+EXPORT_SYMBOL_GPL(ocxl_function_open);
+
+struct list_head *ocxl_function_afu_list(struct ocxl_fn *fn)
+{
+	return &fn->afu_list;
+}
+EXPORT_SYMBOL_GPL(ocxl_function_afu_list);
+
+struct ocxl_afu *ocxl_function_fetch_afu(struct ocxl_fn *fn, u8 afu_idx)
+{
+	struct ocxl_afu *afu;
+
+	list_for_each_entry(afu, &fn->afu_list, list) {
+		if (afu->config.idx == afu_idx)
+			return afu;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ocxl_function_fetch_afu);
+
+const struct ocxl_fn_config *ocxl_function_config(struct ocxl_fn *fn)
 {
+	return &fn->config;
+}
+EXPORT_SYMBOL_GPL(ocxl_function_config);
+
+void ocxl_function_close(struct ocxl_fn *fn)
+{
+	struct ocxl_afu *afu, *tmp;
+
+	list_for_each_entry_safe(afu, tmp, &fn->afu_list, list) {
+		if (afu->private && afu->private_release)
+			afu->private_release(afu->private);
+		remove_afu(afu);
+	}
+
 	deconfigure_function(fn);
 	device_unregister(&fn->dev);
 }
+EXPORT_SYMBOL_GPL(ocxl_function_close);
+
+// AFU Metadata
+
+struct ocxl_afu_config *ocxl_afu_config(struct ocxl_afu *afu)
+{
+	return &afu->config;
+}
+EXPORT_SYMBOL_GPL(ocxl_afu_config);
+
+void ocxl_afu_set_private(struct ocxl_afu *afu, void *private,
+			void (*private_release)(void *private))
+{
+	afu->private = private;
+	afu->private_release = private_release;
+}
+EXPORT_SYMBOL_GPL(ocxl_afu_set_private);
+
+void *ocxl_afu_get_private(struct ocxl_afu *afu)
+{
+	if (afu)
+		return afu->private;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ocxl_afu_get_private);
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 009e09b7ded5..1f17f8706e29 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -17,12 +17,10 @@ static struct class *ocxl_class;
 static struct mutex minors_idr_lock;
 static struct idr minors_idr;
 
-static struct ocxl_afu *find_and_get_afu(dev_t devno)
+static struct ocxl_file_info *find_file_info(dev_t devno)
 {
-	struct ocxl_afu *afu;
-	int afu_minor;
+	struct ocxl_file_info *info;
 
-	afu_minor = MINOR(devno);
 	/*
 	 * We don't declare an RCU critical section here, as our AFU
 	 * is protected by a reference counter on the device. By the time the
@@ -30,56 +28,52 @@ static struct ocxl_afu *find_and_get_afu(dev_t devno)
 	 * the device is already at 0, so no user API will access that AFU and
 	 * this function can't return it.
 	 */
-	afu = idr_find(&minors_idr, afu_minor);
-	if (afu)
-		ocxl_afu_get(afu);
-	return afu;
+	info = idr_find(&minors_idr, MINOR(devno));
+	return info;
 }
 
-static int allocate_afu_minor(struct ocxl_afu *afu)
+static int allocate_minor(struct ocxl_file_info *info)
 {
 	int minor;
 
 	mutex_lock(&minors_idr_lock);
-	minor = idr_alloc(&minors_idr, afu, 0, OCXL_NUM_MINORS, GFP_KERNEL);
+	minor = idr_alloc(&minors_idr, info, 0, OCXL_NUM_MINORS, GFP_KERNEL);
 	mutex_unlock(&minors_idr_lock);
 	return minor;
 }
 
-static void free_afu_minor(struct ocxl_afu *afu)
+static void free_minor(struct ocxl_file_info *info)
 {
 	mutex_lock(&minors_idr_lock);
-	idr_remove(&minors_idr, MINOR(afu->dev.devt));
+	idr_remove(&minors_idr, MINOR(info->dev.devt));
 	mutex_unlock(&minors_idr_lock);
 }
 
 static int afu_open(struct inode *inode, struct file *file)
 {
-	struct ocxl_afu *afu;
+	struct ocxl_file_info *info;
 	struct ocxl_context *ctx;
 	int rc;
 
 	pr_debug("%s for device %x\n", __func__, inode->i_rdev);
 
-	afu = find_and_get_afu(inode->i_rdev);
-	if (!afu)
+	info = find_file_info(inode->i_rdev);
+	if (!info)
 		return -ENODEV;
 
 	ctx = ocxl_context_alloc();
 	if (!ctx) {
 		rc = -ENOMEM;
-		goto put_afu;
+		goto err;
 	}
 
-	rc = ocxl_context_init(ctx, afu, inode->i_mapping);
+	rc = ocxl_context_init(ctx, info->afu, inode->i_mapping);
 	if (rc)
-		goto put_afu;
+		goto err;
 	file->private_data = ctx;
-	ocxl_afu_put(afu);
 	return 0;
 
-put_afu:
-	ocxl_afu_put(afu);
+err:
 	return rc;
 }
 
@@ -204,11 +198,16 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 	struct ocxl_ioctl_irq_fd irq_fd;
 	u64 irq_offset;
 	long rc;
+	bool closed;
 
 	pr_debug("%s for context %d, command %s\n", __func__, ctx->pasid,
 		CMD_STR(cmd));
 
-	if (ctx->status == CLOSED)
+	mutex_lock(&ctx->status_mutex);
+	closed = (ctx->status == CLOSED);
+	mutex_unlock(&ctx->status_mutex);
+
+	if (closed)
 		return -EIO;
 
 	switch (cmd) {
@@ -468,39 +467,83 @@ static const struct file_operations ocxl_afu_fops = {
 	.release        = afu_release,
 };
 
-int ocxl_create_cdev(struct ocxl_afu *afu)
+// Called when there are no more consumers of the AFU
+static void ocxl_file_release(void *private)
 {
-	int rc;
+	struct ocxl_file_info *info = private;
 
-	cdev_init(&afu->cdev, &ocxl_afu_fops);
-	rc = cdev_add(&afu->cdev, afu->dev.devt, 1);
-	if (rc) {
-		dev_err(&afu->dev, "Unable to add afu char device: %d\n", rc);
-		return rc;
-	}
-	return 0;
+	ocxl_sysfs_unregister_afu(info->afu);
+	device_unregister(&info->dev);
+
+	free_minor(info);
 }
 
-void ocxl_destroy_cdev(struct ocxl_afu *afu)
+// Free the info struct
+static void info_release(struct device *dev)
 {
-	cdev_del(&afu->cdev);
+	struct ocxl_file_info *info = container_of(dev, struct ocxl_file_info, dev);
+	kfree(info);
 }
 
-int ocxl_register_afu(struct ocxl_afu *afu)
+int ocxl_file_register_afu(struct ocxl_afu *afu)
 {
 	int minor;
+	int rc;
+	struct ocxl_file_info *info;
+	struct ocxl_fn *fn = afu->fn;
+	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (info == NULL)
+		return -ENOMEM;
+
+	info->afu = afu;
 
-	minor = allocate_afu_minor(afu);
-	if (minor < 0)
+	minor = allocate_minor(info);
+	if (minor < 0) {
+		kfree(info);
 		return minor;
-	afu->dev.devt = MKDEV(MAJOR(ocxl_dev), minor);
-	afu->dev.class = ocxl_class;
-	return device_register(&afu->dev);
+	}
+
+	info->dev.parent = &fn->dev;
+	info->dev.devt = MKDEV(MAJOR(ocxl_dev), minor);
+	info->dev.class = ocxl_class;
+	info->dev.release = info_release;
+
+	ocxl_afu_set_private(afu, info, ocxl_file_release);
+
+	rc = dev_set_name(&info->dev, "%s.%s.%hhu",
+		afu->config.name, dev_name(&pci_dev->dev), afu->config.idx);
+	if (rc)
+		return rc;
+
+	rc = device_register(&info->dev);
+	if (rc)
+		return rc;
+
+	return ocxl_sysfs_register_afu(afu);
 }
 
-void ocxl_unregister_afu(struct ocxl_afu *afu)
+int ocxl_file_make_visible(struct ocxl_afu *afu)
 {
-	free_afu_minor(afu);
+	int rc;
+	struct ocxl_file_info *info = ocxl_afu_get_private(afu);
+
+	cdev_init(&info->cdev, &ocxl_afu_fops);
+	rc = cdev_add(&info->cdev, info->dev.devt, 1);
+	if (rc) {
+		dev_err(&info->dev, "Unable to add afu char device: %d\n", rc);
+		return rc;
+	}
+
+	return 0;
+}
+
+void ocxl_file_make_invisible(struct ocxl_afu *afu)
+{
+	struct ocxl_file_info *info = ocxl_afu_get_private(afu);
+
+	cdev_del(&info->cdev);
 }
 
 static char *ocxl_devnode(struct device *dev, umode_t *mode)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 81086534dab5..05930a29f606 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -11,9 +11,6 @@
 #define MAX_IRQ_PER_LINK	2000
 #define MAX_IRQ_PER_CONTEXT	MAX_IRQ_PER_LINK
 
-#define to_ocxl_function(d) container_of(d, struct ocxl_fn, dev)
-#define to_ocxl_afu(d) container_of(d, struct ocxl_afu, dev)
-
 extern struct pci_driver ocxl_pci_driver;
 
 struct ocxl_fn {
@@ -30,11 +27,17 @@ struct ocxl_fn {
 	void *link;
 };
 
+struct ocxl_file_info {
+	struct ocxl_afu *afu;
+	struct device dev;
+	struct cdev cdev;
+	struct bin_attribute attr_global_mmio;
+};
+
 struct ocxl_afu {
+	struct kref kref;
 	struct ocxl_fn *fn;
 	struct list_head list;
-	struct device dev;
-	struct cdev cdev;
 	struct ocxl_afu_config config;
 	int pasid_base;
 	int pasid_count; /* opened contexts */
@@ -48,7 +51,9 @@ struct ocxl_afu {
 	u64 irq_base_offset;
 	void __iomem *global_mmio_ptr;
 	u64 pp_mmio_start;
-	struct bin_attribute attr_global_mmio;
+	void *private;
+	void (*private_release)(void *private);
+
 };
 
 enum ocxl_context_status {
@@ -91,13 +96,11 @@ struct ocxl_process_element {
 	__be32 software_state;
 };
 
-struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
-void ocxl_afu_put(struct ocxl_afu *afu);
-
 int ocxl_create_cdev(struct ocxl_afu *afu);
 void ocxl_destroy_cdev(struct ocxl_afu *afu);
-int ocxl_register_afu(struct ocxl_afu *afu);
-void ocxl_unregister_afu(struct ocxl_afu *afu);
+int ocxl_file_register_afu(struct ocxl_afu *afu);
+int ocxl_file_make_visible(struct ocxl_afu *afu);
+void ocxl_file_make_invisible(struct ocxl_afu *afu);
 
 int ocxl_file_init(void);
 void ocxl_file_exit(void);
@@ -140,8 +143,8 @@ int ocxl_context_detach(struct ocxl_context *ctx);
 void ocxl_context_detach_all(struct ocxl_afu *afu);
 void ocxl_context_free(struct ocxl_context *ctx);
 
-int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
-void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
+int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
+void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
 
 int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
 int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
@@ -150,9 +153,11 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
 			int eventfd);
 u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
 
-struct ocxl_fn *init_function(struct pci_dev *dev);
-void remove_function(struct ocxl_fn *fn);
-int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx);
-void remove_afu(struct ocxl_afu *afu);
+/**
+ * Free an AFU
+ *
+ * afu: The AFU to free
+ */
+void ocxl_free_afu(struct ocxl_afu *afu);
 
 #endif /* _OCXL_INTERNAL_H_ */
diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
index 5926c863716c..a1ed2bb02e4b 100644
--- a/drivers/misc/ocxl/pci.c
+++ b/drivers/misc/ocxl/pci.c
@@ -16,47 +16,48 @@ MODULE_DEVICE_TABLE(pci, ocxl_pci_tbl);
 
 static int ocxl_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	int rc, afu_count = 0;
-	u8 afu;
+	int rc;
+	struct ocxl_afu *afu, *tmp;
 	struct ocxl_fn *fn;
+	struct list_head *afu_list;
 
-	if (!radix_enabled()) {
-		dev_err(&dev->dev, "Unsupported memory model (hash)\n");
-		return -ENODEV;
-	}
-
-	fn = init_function(dev);
-	if (IS_ERR(fn)) {
-		dev_err(&dev->dev, "function init failed: %li\n",
-			PTR_ERR(fn));
+	fn = ocxl_function_open(dev);
+	if (IS_ERR(fn))
 		return PTR_ERR(fn);
-	}
 
-	for (afu = 0; afu <= fn->config.max_afu_index; afu++) {
-		rc = ocxl_config_check_afu_index(dev, &fn->config, afu);
-		if (rc > 0) {
-			rc = init_afu(dev, fn, afu);
-			if (rc) {
-				dev_err(&dev->dev,
-					"Can't initialize AFU index %d\n", afu);
-				continue;
-			}
-			afu_count++;
-		}
+	pci_set_drvdata(dev, fn);
+
+	afu_list = ocxl_function_afu_list(fn);
+
+	list_for_each_entry_safe(afu, tmp, afu_list, list) {
+		rc = ocxl_file_register_afu(afu);
+		if (rc)
+			continue;
+
+		rc = ocxl_file_make_visible(afu);
+		if (rc)
+			continue;
+
+		// Defer error cleanup until the device is removed
 	}
-	dev_info(&dev->dev, "%d AFU(s) configured\n", afu_count);
+
 	return 0;
 }
 
-static void ocxl_remove(struct pci_dev *dev)
+void ocxl_remove(struct pci_dev *dev)
 {
-	struct ocxl_afu *afu, *tmp;
-	struct ocxl_fn *fn = pci_get_drvdata(dev);
+	struct ocxl_fn *fn;
+	struct ocxl_afu *afu;
+	struct list_head *afu_list;
+
+	fn = pci_get_drvdata(dev);
+	afu_list = ocxl_function_afu_list(fn);
 
-	list_for_each_entry_safe(afu, tmp, &fn->afu_list, list) {
-		remove_afu(afu);
+	list_for_each_entry(afu, afu_list, list) {
+		ocxl_file_make_invisible(afu);
 	}
-	remove_function(fn);
+
+	ocxl_function_close(fn);
 }
 
 struct pci_driver ocxl_pci_driver = {
diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
index 0ab1fd1b2682..a67931f1feec 100644
--- a/drivers/misc/ocxl/sysfs.c
+++ b/drivers/misc/ocxl/sysfs.c
@@ -3,11 +3,18 @@
 #include <linux/sysfs.h>
 #include "ocxl_internal.h"
 
+static inline struct ocxl_afu *to_afu(struct device *device)
+{
+	struct ocxl_file_info *info = container_of(device, struct ocxl_file_info, dev);
+
+	return info->afu;
+}
+
 static ssize_t global_mmio_size_show(struct device *device,
 				struct device_attribute *attr,
 				char *buf)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(device);
+	struct ocxl_afu *afu = to_afu(device);
 
 	return scnprintf(buf, PAGE_SIZE, "%d\n",
 			afu->config.global_mmio_size);
@@ -17,7 +24,7 @@ static ssize_t pp_mmio_size_show(struct device *device,
 				struct device_attribute *attr,
 				char *buf)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(device);
+	struct ocxl_afu *afu = to_afu(device);
 
 	return scnprintf(buf, PAGE_SIZE, "%d\n",
 			afu->config.pp_mmio_stride);
@@ -27,7 +34,7 @@ static ssize_t afu_version_show(struct device *device,
 				struct device_attribute *attr,
 				char *buf)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(device);
+	struct ocxl_afu *afu = to_afu(device);
 
 	return scnprintf(buf, PAGE_SIZE, "%hhu:%hhu\n",
 			afu->config.version_major,
@@ -38,7 +45,7 @@ static ssize_t contexts_show(struct device *device,
 		struct device_attribute *attr,
 		char *buf)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(device);
+	struct ocxl_afu *afu = to_afu(device);
 
 	return scnprintf(buf, PAGE_SIZE, "%d/%d\n",
 			afu->pasid_count, afu->pasid_max);
@@ -55,7 +62,7 @@ static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
 				struct bin_attribute *bin_attr, char *buf,
 				loff_t off, size_t count)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(kobj_to_dev(kobj));
+	struct ocxl_afu *afu = to_afu(kobj_to_dev(kobj));
 
 	if (count == 0 || off < 0 ||
 		off >= afu->config.global_mmio_size)
@@ -86,7 +93,7 @@ static int global_mmio_mmap(struct file *filp, struct kobject *kobj,
 			struct bin_attribute *bin_attr,
 			struct vm_area_struct *vma)
 {
-	struct ocxl_afu *afu = to_ocxl_afu(kobj_to_dev(kobj));
+	struct ocxl_afu *afu = to_afu(kobj_to_dev(kobj));
 
 	if ((vma_pages(vma) + vma->vm_pgoff) >
 		(afu->config.global_mmio_size >> PAGE_SHIFT))
@@ -99,27 +106,29 @@ static int global_mmio_mmap(struct file *filp, struct kobject *kobj,
 	return 0;
 }
 
-int ocxl_sysfs_add_afu(struct ocxl_afu *afu)
+int ocxl_sysfs_register_afu(struct ocxl_afu *afu)
 {
 	int i, rc;
+	struct ocxl_file_info *info = ocxl_afu_get_private(afu);
+
+	if (!info)
+		return -EFAULT; // ocxl_file_register_afu() must be called first
 
 	for (i = 0; i < ARRAY_SIZE(afu_attrs); i++) {
-		rc = device_create_file(&afu->dev, &afu_attrs[i]);
+		rc = device_create_file(&info->dev, &afu_attrs[i]);
 		if (rc)
 			goto err;
 	}
 
-	sysfs_attr_init(&afu->attr_global_mmio.attr);
-	afu->attr_global_mmio.attr.name = "global_mmio_area";
-	afu->attr_global_mmio.attr.mode = 0600;
-	afu->attr_global_mmio.size = afu->config.global_mmio_size;
-	afu->attr_global_mmio.read = global_mmio_read;
-	afu->attr_global_mmio.mmap = global_mmio_mmap;
-	rc = device_create_bin_file(&afu->dev, &afu->attr_global_mmio);
+	sysfs_attr_init(&info->attr_global_mmio.attr);
+	info->attr_global_mmio.attr.name = "global_mmio_area";
+	info->attr_global_mmio.attr.mode = 0600;
+	info->attr_global_mmio.size = afu->config.global_mmio_size;
+	info->attr_global_mmio.read = global_mmio_read;
+	info->attr_global_mmio.mmap = global_mmio_mmap;
+	rc = device_create_bin_file(&info->dev, &info->attr_global_mmio);
 	if (rc) {
-		dev_err(&afu->dev,
-			"Unable to create global mmio attr for afu: %d\n",
-			rc);
+		dev_err(&info->dev, "Unable to create global mmio attr for afu: %d\n", rc);
 		goto err;
 	}
 
@@ -127,15 +136,20 @@ int ocxl_sysfs_add_afu(struct ocxl_afu *afu)
 
 err:
 	for (i--; i >= 0; i--)
-		device_remove_file(&afu->dev, &afu_attrs[i]);
+		device_remove_file(&info->dev, &afu_attrs[i]);
+
 	return rc;
 }
 
-void ocxl_sysfs_remove_afu(struct ocxl_afu *afu)
+void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu)
 {
+	struct ocxl_file_info *info = ocxl_afu_get_private(afu);
 	int i;
 
+	if (!info)
+		return;
+
 	for (i = 0; i < ARRAY_SIZE(afu_attrs); i++)
-		device_remove_file(&afu->dev, &afu_attrs[i]);
-	device_remove_bin_file(&afu->dev, &afu->attr_global_mmio);
+		device_remove_file(&info->dev, &afu_attrs[i]);
+	device_remove_bin_file(&info->dev, &info->attr_global_mmio);
 }
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 9530d3be1b30..7d611ea68a59 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -16,11 +16,7 @@
 
 #define OCXL_AFU_NAME_SZ      (24+1)  /* add 1 for NULL termination */
 
-/*
- * The following 2 structures are a fairly generic way of representing
- * the configuration data for a function and AFU, as read from the
- * configuration space.
- */
+
 struct ocxl_afu_config {
 	u8 idx;
 	int dvsec_afu_control_pos; /* offset of AFU control DVSEC */
@@ -49,12 +45,110 @@ struct ocxl_fn_config {
 	s8 max_afu_index;
 };
 
-/*
- * Read the configuration space of a function and fill in a
- * ocxl_fn_config structure with all the function details
+// These are opaque outside the ocxl driver
+struct ocxl_afu;
+struct ocxl_fn;
+
+// Device detection & initialisation
+
+/**
+ * Open an OpenCAPI function on an OpenCAPI device
+ *
+ * @dev: The PCI device that contains the function
+ *
+ * Returns an opaque pointer to the function, or an error pointer (check with IS_ERR)
  */
-int ocxl_config_read_function(struct pci_dev *dev,
-				struct ocxl_fn_config *fn);
+struct ocxl_fn *ocxl_function_open(struct pci_dev *dev);
+
+/**
+ * Get the list of AFUs associated with a PCI function device
+ *
+ * Returns a list of struct ocxl_afu *
+ *
+ * @fn: The OpenCAPI function containing the AFUs
+ */
+struct list_head *ocxl_function_afu_list(struct ocxl_fn *fn);
+
+/**
+ * Fetch an AFU instance from an OpenCAPI function
+ *
+ * @fn: The OpenCAPI function to get the AFU from
+ * @afu_idx: The index of the AFU to get
+ *
+ * If successful, the AFU should be released with ocxl_afu_put()
+ *
+ * Returns a pointer to the AFU, or NULL on error
+ */
+struct ocxl_afu *ocxl_function_fetch_afu(struct ocxl_fn *fn, u8 afu_idx);
+
+/**
+ * Take a reference to an AFU
+ *
+ * @afu: The AFU to increment the reference count on
+ */
+void ocxl_afu_get(struct ocxl_afu *afu);
+
+/**
+ * Release a reference to an AFU
+ *
+ * @afu: The AFU to decrement the reference count on
+ */
+void ocxl_afu_put(struct ocxl_afu *afu);
+
+
+/**
+ * Get the configuration information for an OpenCAPI function
+ *
+ * @fn: The OpenCAPI function to get the config for
+ *
+ * Returns the function config, or NULL on error
+ */
+const struct ocxl_fn_config *ocxl_function_config(struct ocxl_fn *fn);
+
+/**
+ * Close an OpenCAPI function
+ *
+ * This will free any AFUs previously retrieved from the function, and
+ * detach and associated contexts. The contexts must by freed by the caller.
+ *
+ * @fn: The OpenCAPI function to close
+ *
+ */
+void ocxl_function_close(struct ocxl_fn *fn);
+
+// AFU Metadata
+
+/**
+ * Get a pointer to the config for an AFU
+ *
+ * @afu: a pointer to the AFU to get the config for
+ *
+ * Returns a pointer to the AFU config
+ */
+struct ocxl_afu_config *ocxl_afu_config(struct ocxl_afu *afu);
+
+/**
+ * Assign opaque hardware specific information to an OpenCAPI AFU.
+ *
+ * @dev: The PCI device associated with the OpenCAPI device
+ * @private: the opaque hardware specific information to assign to the driver
+ * @private_release: A callback to free the private data
+ */
+void ocxl_afu_set_private(struct ocxl_afu *afu, void *private,
+			void (*private_release)(void *private));
+
+/**
+ * Fetch the hardware specific information associated with an external OpenCAPI
+ * AFU. This may be consumed by an external OpenCAPI driver.
+ *
+ * @afu: The AFU
+ *
+ * Returns the opaque pointer associated with the device, or NULL if not set
+ */
+void *ocxl_afu_get_private(struct ocxl_afu *dev);
+
+
+// Functions left here are for compatibility with the cxlflash driver
 
 /*
  * Read the configuration space of a function for the AFU specified by
@@ -141,6 +235,13 @@ int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
 int ocxl_config_terminate_pasid(struct pci_dev *dev,
 				int afu_control_offset, int pasid);
 
+/*
+ * Read the configuration space of a function and fill in a
+ * ocxl_fn_config structure with all the function details
+ */
+int ocxl_config_read_function(struct pci_dev *dev,
+				struct ocxl_fn_config *fn);
+
 /*
  * Set up the opencapi link for the function.
  *
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 2/7] ocxl: Don't pass pci_dev around
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

This data is already available in a struct

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/core.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
index b47cfda83e46..2fd0c700e8a0 100644
--- a/drivers/misc/ocxl/core.c
+++ b/drivers/misc/ocxl/core.c
@@ -66,10 +66,11 @@ static int set_afu_device(struct ocxl_afu *afu, const char *location)
 	return rc;
 }
 
-static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
+static int assign_afu_actag(struct ocxl_afu *afu)
 {
 	struct ocxl_fn *fn = afu->fn;
 	int actag_count, actag_offset;
+	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
 
 	/*
 	 * if there were not enough actags for the function, each afu
@@ -79,16 +80,16 @@ static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
 		fn->actag_enabled / fn->actag_supported;
 	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
 	if (actag_offset < 0) {
-		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
+		dev_err(&pci_dev->dev, "Can't allocate %d actags for AFU: %d\n",
 			actag_count, actag_offset);
 		return actag_offset;
 	}
 	afu->actag_base = fn->actag_base + actag_offset;
 	afu->actag_enabled = actag_count;
 
-	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
+	ocxl_config_set_afu_actag(pci_dev, afu->config.dvsec_afu_control_pos,
 				afu->actag_base, afu->actag_enabled);
-	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
+	dev_dbg(&pci_dev->dev, "actag base=%d enabled=%d\n",
 		afu->actag_base, afu->actag_enabled);
 	return 0;
 }
@@ -103,10 +104,11 @@ static void reclaim_afu_actag(struct ocxl_afu *afu)
 	ocxl_actag_afu_free(afu->fn, start_offset, size);
 }
 
-static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
+static int assign_afu_pasid(struct ocxl_afu *afu)
 {
 	struct ocxl_fn *fn = afu->fn;
 	int pasid_count, pasid_offset;
+	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
 
 	/*
 	 * We only support the case where the function configuration
@@ -115,7 +117,7 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
 	pasid_count = 1 << afu->config.pasid_supported_log;
 	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
 	if (pasid_offset < 0) {
-		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
+		dev_err(&pci_dev->dev, "Can't allocate %d PASIDs for AFU: %d\n",
 			pasid_count, pasid_offset);
 		return pasid_offset;
 	}
@@ -123,10 +125,10 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
 	afu->pasid_count = 0;
 	afu->pasid_max = pasid_count;
 
-	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
+	ocxl_config_set_afu_pasid(pci_dev, afu->config.dvsec_afu_control_pos,
 				afu->pasid_base,
 				afu->config.pasid_supported_log);
-	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
+	dev_dbg(&pci_dev->dev, "PASID base=%d, enabled=%d\n",
 		afu->pasid_base, pasid_count);
 	return 0;
 }
@@ -172,9 +174,10 @@ static void release_fn_bar(struct ocxl_fn *fn, int bar)
 	WARN_ON(fn->bar_used[idx] < 0);
 }
 
-static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
+static int map_mmio_areas(struct ocxl_afu *afu)
 {
 	int rc;
+	struct pci_dev *pci_dev = to_pci_dev(afu->fn->dev.parent);
 
 	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
 	if (rc)
@@ -187,10 +190,10 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
 	}
 
 	afu->global_mmio_start =
-		pci_resource_start(dev, afu->config.global_mmio_bar) +
+		pci_resource_start(pci_dev, afu->config.global_mmio_bar) +
 		afu->config.global_mmio_offset;
 	afu->pp_mmio_start =
-		pci_resource_start(dev, afu->config.pp_mmio_bar) +
+		pci_resource_start(pci_dev, afu->config.pp_mmio_bar) +
 		afu->config.pp_mmio_offset;
 
 	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
@@ -198,7 +201,7 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
 	if (!afu->global_mmio_ptr) {
 		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
 		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
-		dev_err(&dev->dev, "Error mapping global mmio area\n");
+		dev_err(&pci_dev->dev, "Error mapping global mmio area\n");
 		return -ENOMEM;
 	}
 
@@ -234,17 +237,17 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
 	if (rc)
 		return rc;
 
-	rc = assign_afu_actag(afu, dev);
+	rc = assign_afu_actag(afu);
 	if (rc)
 		return rc;
 
-	rc = assign_afu_pasid(afu, dev);
+	rc = assign_afu_pasid(afu);
 	if (rc) {
 		reclaim_afu_actag(afu);
 		return rc;
 	}
 
-	rc = map_mmio_areas(afu, dev);
+	rc = map_mmio_areas(afu);
 	if (rc) {
 		reclaim_afu_pasid(afu);
 		reclaim_afu_actag(afu);
@@ -331,7 +334,7 @@ void remove_afu(struct ocxl_afu *afu)
 	device_unregister(&afu->dev);
 }
 
-static struct ocxl_fn *alloc_function(struct pci_dev *dev)
+static struct ocxl_fn *alloc_function(void)
 {
 	struct ocxl_fn *fn;
 
@@ -342,6 +345,7 @@ static struct ocxl_fn *alloc_function(struct pci_dev *dev)
 	INIT_LIST_HEAD(&fn->afu_list);
 	INIT_LIST_HEAD(&fn->pasid_list);
 	INIT_LIST_HEAD(&fn->actag_list);
+
 	return fn;
 }
 
@@ -491,7 +495,7 @@ struct ocxl_fn *init_function(struct pci_dev *dev)
 	struct ocxl_fn *fn;
 	int rc;
 
-	fn = alloc_function(dev);
+	fn = alloc_function();
 	if (!fn)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 1/7] ocxl: Split pci.c
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

In preparation for making core code available for external drivers,
move the core code out of pci.c and into core.c

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/Makefile        |   1 +
 drivers/misc/ocxl/core.c          | 517 ++++++++++++++++++++++++++++++
 drivers/misc/ocxl/ocxl_internal.h |   5 +
 drivers/misc/ocxl/pci.c           | 517 ------------------------------
 4 files changed, 523 insertions(+), 517 deletions(-)
 create mode 100644 drivers/misc/ocxl/core.c

diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
index 5229dcda8297..bc4e39bfda7b 100644
--- a/drivers/misc/ocxl/Makefile
+++ b/drivers/misc/ocxl/Makefile
@@ -3,6 +3,7 @@ ccflags-$(CONFIG_PPC_WERROR)	+= -Werror
 
 ocxl-y				+= main.o pci.o config.o file.o pasid.o
 ocxl-y				+= link.o context.o afu_irq.o sysfs.o trace.o
+ocxl-y				+= core.o
 obj-$(CONFIG_OCXL)		+= ocxl.o
 
 # For tracepoints to include our trace.h from tracepoint infrastructure:
diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
new file mode 100644
index 000000000000..b47cfda83e46
--- /dev/null
+++ b/drivers/misc/ocxl/core.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2017 IBM Corp.
+#include <linux/idr.h>
+#include "ocxl_internal.h"
+
+static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
+{
+	return (get_device(&fn->dev) == NULL) ? NULL : fn;
+}
+
+static void ocxl_fn_put(struct ocxl_fn *fn)
+{
+	put_device(&fn->dev);
+}
+
+struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
+{
+	return (get_device(&afu->dev) == NULL) ? NULL : afu;
+}
+
+void ocxl_afu_put(struct ocxl_afu *afu)
+{
+	put_device(&afu->dev);
+}
+
+static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
+{
+	struct ocxl_afu *afu;
+
+	afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
+	if (!afu)
+		return NULL;
+
+	mutex_init(&afu->contexts_lock);
+	mutex_init(&afu->afu_control_lock);
+	idr_init(&afu->contexts_idr);
+	afu->fn = fn;
+	ocxl_fn_get(fn);
+	return afu;
+}
+
+static void free_afu(struct ocxl_afu *afu)
+{
+	idr_destroy(&afu->contexts_idr);
+	ocxl_fn_put(afu->fn);
+	kfree(afu);
+}
+
+static void free_afu_dev(struct device *dev)
+{
+	struct ocxl_afu *afu = to_ocxl_afu(dev);
+
+	ocxl_unregister_afu(afu);
+	free_afu(afu);
+}
+
+static int set_afu_device(struct ocxl_afu *afu, const char *location)
+{
+	struct ocxl_fn *fn = afu->fn;
+	int rc;
+
+	afu->dev.parent = &fn->dev;
+	afu->dev.release = free_afu_dev;
+	rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
+		afu->config.idx);
+	return rc;
+}
+
+static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
+{
+	struct ocxl_fn *fn = afu->fn;
+	int actag_count, actag_offset;
+
+	/*
+	 * if there were not enough actags for the function, each afu
+	 * reduces its count as well
+	 */
+	actag_count = afu->config.actag_supported *
+		fn->actag_enabled / fn->actag_supported;
+	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
+	if (actag_offset < 0) {
+		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
+			actag_count, actag_offset);
+		return actag_offset;
+	}
+	afu->actag_base = fn->actag_base + actag_offset;
+	afu->actag_enabled = actag_count;
+
+	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
+				afu->actag_base, afu->actag_enabled);
+	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
+		afu->actag_base, afu->actag_enabled);
+	return 0;
+}
+
+static void reclaim_afu_actag(struct ocxl_afu *afu)
+{
+	struct ocxl_fn *fn = afu->fn;
+	int start_offset, size;
+
+	start_offset = afu->actag_base - fn->actag_base;
+	size = afu->actag_enabled;
+	ocxl_actag_afu_free(afu->fn, start_offset, size);
+}
+
+static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
+{
+	struct ocxl_fn *fn = afu->fn;
+	int pasid_count, pasid_offset;
+
+	/*
+	 * We only support the case where the function configuration
+	 * requested enough PASIDs to cover all AFUs.
+	 */
+	pasid_count = 1 << afu->config.pasid_supported_log;
+	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
+	if (pasid_offset < 0) {
+		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
+			pasid_count, pasid_offset);
+		return pasid_offset;
+	}
+	afu->pasid_base = fn->pasid_base + pasid_offset;
+	afu->pasid_count = 0;
+	afu->pasid_max = pasid_count;
+
+	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
+				afu->pasid_base,
+				afu->config.pasid_supported_log);
+	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
+		afu->pasid_base, pasid_count);
+	return 0;
+}
+
+static void reclaim_afu_pasid(struct ocxl_afu *afu)
+{
+	struct ocxl_fn *fn = afu->fn;
+	int start_offset, size;
+
+	start_offset = afu->pasid_base - fn->pasid_base;
+	size = 1 << afu->config.pasid_supported_log;
+	ocxl_pasid_afu_free(afu->fn, start_offset, size);
+}
+
+static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
+{
+	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
+	int rc, idx;
+
+	if (bar != 0 && bar != 2 && bar != 4)
+		return -EINVAL;
+
+	idx = bar >> 1;
+	if (fn->bar_used[idx]++ == 0) {
+		rc = pci_request_region(dev, bar, "ocxl");
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+
+static void release_fn_bar(struct ocxl_fn *fn, int bar)
+{
+	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
+	int idx;
+
+	if (bar != 0 && bar != 2 && bar != 4)
+		return;
+
+	idx = bar >> 1;
+	if (--fn->bar_used[idx] == 0)
+		pci_release_region(dev, bar);
+	WARN_ON(fn->bar_used[idx] < 0);
+}
+
+static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
+{
+	int rc;
+
+	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
+	if (rc)
+		return rc;
+
+	rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
+	if (rc) {
+		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
+		return rc;
+	}
+
+	afu->global_mmio_start =
+		pci_resource_start(dev, afu->config.global_mmio_bar) +
+		afu->config.global_mmio_offset;
+	afu->pp_mmio_start =
+		pci_resource_start(dev, afu->config.pp_mmio_bar) +
+		afu->config.pp_mmio_offset;
+
+	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
+				afu->config.global_mmio_size);
+	if (!afu->global_mmio_ptr) {
+		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
+		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
+		dev_err(&dev->dev, "Error mapping global mmio area\n");
+		return -ENOMEM;
+	}
+
+	/*
+	 * Leave an empty page between the per-process mmio area and
+	 * the AFU interrupt mappings
+	 */
+	afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
+	return 0;
+}
+
+static void unmap_mmio_areas(struct ocxl_afu *afu)
+{
+	if (afu->global_mmio_ptr) {
+		iounmap(afu->global_mmio_ptr);
+		afu->global_mmio_ptr = NULL;
+	}
+	afu->global_mmio_start = 0;
+	afu->pp_mmio_start = 0;
+	release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
+	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
+}
+
+static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
+{
+	int rc;
+
+	rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
+	if (rc)
+		return rc;
+
+	rc = set_afu_device(afu, dev_name(&dev->dev));
+	if (rc)
+		return rc;
+
+	rc = assign_afu_actag(afu, dev);
+	if (rc)
+		return rc;
+
+	rc = assign_afu_pasid(afu, dev);
+	if (rc) {
+		reclaim_afu_actag(afu);
+		return rc;
+	}
+
+	rc = map_mmio_areas(afu, dev);
+	if (rc) {
+		reclaim_afu_pasid(afu);
+		reclaim_afu_actag(afu);
+		return rc;
+	}
+	return 0;
+}
+
+static void deconfigure_afu(struct ocxl_afu *afu)
+{
+	unmap_mmio_areas(afu);
+	reclaim_afu_pasid(afu);
+	reclaim_afu_actag(afu);
+}
+
+static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
+{
+	int rc;
+
+	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
+	/*
+	 * Char device creation is the last step, as processes can
+	 * call our driver immediately, so all our inits must be finished.
+	 */
+	rc = ocxl_create_cdev(afu);
+	if (rc)
+		return rc;
+	return 0;
+}
+
+static void deactivate_afu(struct ocxl_afu *afu)
+{
+	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
+
+	ocxl_destroy_cdev(afu);
+	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
+}
+
+int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
+{
+	int rc;
+	struct ocxl_afu *afu;
+
+	afu = alloc_afu(fn);
+	if (!afu)
+		return -ENOMEM;
+
+	rc = configure_afu(afu, afu_idx, dev);
+	if (rc) {
+		free_afu(afu);
+		return rc;
+	}
+
+	rc = ocxl_register_afu(afu);
+	if (rc)
+		goto err;
+
+	rc = ocxl_sysfs_add_afu(afu);
+	if (rc)
+		goto err;
+
+	rc = activate_afu(dev, afu);
+	if (rc)
+		goto err_sys;
+
+	list_add_tail(&afu->list, &fn->afu_list);
+	return 0;
+
+err_sys:
+	ocxl_sysfs_remove_afu(afu);
+err:
+	deconfigure_afu(afu);
+	device_unregister(&afu->dev);
+	return rc;
+}
+
+void remove_afu(struct ocxl_afu *afu)
+{
+	list_del(&afu->list);
+	ocxl_context_detach_all(afu);
+	deactivate_afu(afu);
+	ocxl_sysfs_remove_afu(afu);
+	deconfigure_afu(afu);
+	device_unregister(&afu->dev);
+}
+
+static struct ocxl_fn *alloc_function(struct pci_dev *dev)
+{
+	struct ocxl_fn *fn;
+
+	fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
+	if (!fn)
+		return NULL;
+
+	INIT_LIST_HEAD(&fn->afu_list);
+	INIT_LIST_HEAD(&fn->pasid_list);
+	INIT_LIST_HEAD(&fn->actag_list);
+	return fn;
+}
+
+static void free_function(struct ocxl_fn *fn)
+{
+	WARN_ON(!list_empty(&fn->afu_list));
+	WARN_ON(!list_empty(&fn->pasid_list));
+	kfree(fn);
+}
+
+static void free_function_dev(struct device *dev)
+{
+	struct ocxl_fn *fn = to_ocxl_function(dev);
+
+	free_function(fn);
+}
+
+static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
+{
+	int rc;
+
+	fn->dev.parent = &dev->dev;
+	fn->dev.release = free_function_dev;
+	rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
+	if (rc)
+		return rc;
+	pci_set_drvdata(dev, fn);
+	return 0;
+}
+
+static int assign_function_actag(struct ocxl_fn *fn)
+{
+	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
+	u16 base, enabled, supported;
+	int rc;
+
+	rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
+	if (rc)
+		return rc;
+
+	fn->actag_base = base;
+	fn->actag_enabled = enabled;
+	fn->actag_supported = supported;
+
+	ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
+			fn->actag_base,	fn->actag_enabled);
+	dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
+		fn->actag_base, fn->actag_enabled);
+	return 0;
+}
+
+static int set_function_pasid(struct ocxl_fn *fn)
+{
+	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
+	int rc, desired_count, max_count;
+
+	/* A function may not require any PASID */
+	if (fn->config.max_pasid_log < 0)
+		return 0;
+
+	rc = ocxl_config_get_pasid_info(dev, &max_count);
+	if (rc)
+		return rc;
+
+	desired_count = 1 << fn->config.max_pasid_log;
+
+	if (desired_count > max_count) {
+		dev_err(&fn->dev,
+			"Function requires more PASIDs than is available (%d vs. %d)\n",
+			desired_count, max_count);
+		return -ENOSPC;
+	}
+
+	fn->pasid_base = 0;
+	return 0;
+}
+
+static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
+{
+	int rc;
+
+	rc = pci_enable_device(dev);
+	if (rc) {
+		dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
+		return rc;
+	}
+
+	/*
+	 * Once it has been confirmed to work on our hardware, we
+	 * should reset the function, to force the adapter to restart
+	 * from scratch.
+	 * A function reset would also reset all its AFUs.
+	 *
+	 * Some hints for implementation:
+	 *
+	 * - there's not status bit to know when the reset is done. We
+	 *   should try reading the config space to know when it's
+	 *   done.
+	 * - probably something like:
+	 *	Reset
+	 *	wait 100ms
+	 *	issue config read
+	 *	allow device up to 1 sec to return success on config
+	 *	read before declaring it broken
+	 *
+	 * Some shared logic on the card (CFG, TLX) won't be reset, so
+	 * there's no guarantee that it will be enough.
+	 */
+	rc = ocxl_config_read_function(dev, &fn->config);
+	if (rc)
+		return rc;
+
+	rc = set_function_device(fn, dev);
+	if (rc)
+		return rc;
+
+	rc = assign_function_actag(fn);
+	if (rc)
+		return rc;
+
+	rc = set_function_pasid(fn);
+	if (rc)
+		return rc;
+
+	rc = ocxl_link_setup(dev, 0, &fn->link);
+	if (rc)
+		return rc;
+
+	rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
+	if (rc) {
+		ocxl_link_release(dev, fn->link);
+		return rc;
+	}
+	return 0;
+}
+
+static void deconfigure_function(struct ocxl_fn *fn)
+{
+	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
+
+	ocxl_link_release(dev, fn->link);
+	pci_disable_device(dev);
+}
+
+struct ocxl_fn *init_function(struct pci_dev *dev)
+{
+	struct ocxl_fn *fn;
+	int rc;
+
+	fn = alloc_function(dev);
+	if (!fn)
+		return ERR_PTR(-ENOMEM);
+
+	rc = configure_function(fn, dev);
+	if (rc) {
+		free_function(fn);
+		return ERR_PTR(rc);
+	}
+
+	rc = device_register(&fn->dev);
+	if (rc) {
+		deconfigure_function(fn);
+		put_device(&fn->dev);
+		return ERR_PTR(rc);
+	}
+	return fn;
+}
+
+void remove_function(struct ocxl_fn *fn)
+{
+	deconfigure_function(fn);
+	device_unregister(&fn->dev);
+}
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 06fd98c989c8..81086534dab5 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -150,4 +150,9 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
 			int eventfd);
 u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
 
+struct ocxl_fn *init_function(struct pci_dev *dev);
+void remove_function(struct ocxl_fn *fn);
+int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx);
+void remove_afu(struct ocxl_afu *afu);
+
 #endif /* _OCXL_INTERNAL_H_ */
diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
index 21f425472a82..5926c863716c 100644
--- a/drivers/misc/ocxl/pci.c
+++ b/drivers/misc/ocxl/pci.c
@@ -1,9 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 // Copyright 2017 IBM Corp.
 #include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/idr.h>
-#include <asm/pnv-ocxl.h>
 #include "ocxl_internal.h"
 
 /*
@@ -17,520 +14,6 @@ static const struct pci_device_id ocxl_pci_tbl[] = {
 };
 MODULE_DEVICE_TABLE(pci, ocxl_pci_tbl);
 
-
-static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
-{
-	return (get_device(&fn->dev) == NULL) ? NULL : fn;
-}
-
-static void ocxl_fn_put(struct ocxl_fn *fn)
-{
-	put_device(&fn->dev);
-}
-
-struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
-{
-	return (get_device(&afu->dev) == NULL) ? NULL : afu;
-}
-
-void ocxl_afu_put(struct ocxl_afu *afu)
-{
-	put_device(&afu->dev);
-}
-
-static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
-{
-	struct ocxl_afu *afu;
-
-	afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
-	if (!afu)
-		return NULL;
-
-	mutex_init(&afu->contexts_lock);
-	mutex_init(&afu->afu_control_lock);
-	idr_init(&afu->contexts_idr);
-	afu->fn = fn;
-	ocxl_fn_get(fn);
-	return afu;
-}
-
-static void free_afu(struct ocxl_afu *afu)
-{
-	idr_destroy(&afu->contexts_idr);
-	ocxl_fn_put(afu->fn);
-	kfree(afu);
-}
-
-static void free_afu_dev(struct device *dev)
-{
-	struct ocxl_afu *afu = to_ocxl_afu(dev);
-
-	ocxl_unregister_afu(afu);
-	free_afu(afu);
-}
-
-static int set_afu_device(struct ocxl_afu *afu, const char *location)
-{
-	struct ocxl_fn *fn = afu->fn;
-	int rc;
-
-	afu->dev.parent = &fn->dev;
-	afu->dev.release = free_afu_dev;
-	rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
-		afu->config.idx);
-	return rc;
-}
-
-static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
-{
-	struct ocxl_fn *fn = afu->fn;
-	int actag_count, actag_offset;
-
-	/*
-	 * if there were not enough actags for the function, each afu
-	 * reduces its count as well
-	 */
-	actag_count = afu->config.actag_supported *
-		fn->actag_enabled / fn->actag_supported;
-	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
-	if (actag_offset < 0) {
-		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
-			actag_count, actag_offset);
-		return actag_offset;
-	}
-	afu->actag_base = fn->actag_base + actag_offset;
-	afu->actag_enabled = actag_count;
-
-	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
-				afu->actag_base, afu->actag_enabled);
-	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
-		afu->actag_base, afu->actag_enabled);
-	return 0;
-}
-
-static void reclaim_afu_actag(struct ocxl_afu *afu)
-{
-	struct ocxl_fn *fn = afu->fn;
-	int start_offset, size;
-
-	start_offset = afu->actag_base - fn->actag_base;
-	size = afu->actag_enabled;
-	ocxl_actag_afu_free(afu->fn, start_offset, size);
-}
-
-static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
-{
-	struct ocxl_fn *fn = afu->fn;
-	int pasid_count, pasid_offset;
-
-	/*
-	 * We only support the case where the function configuration
-	 * requested enough PASIDs to cover all AFUs.
-	 */
-	pasid_count = 1 << afu->config.pasid_supported_log;
-	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
-	if (pasid_offset < 0) {
-		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
-			pasid_count, pasid_offset);
-		return pasid_offset;
-	}
-	afu->pasid_base = fn->pasid_base + pasid_offset;
-	afu->pasid_count = 0;
-	afu->pasid_max = pasid_count;
-
-	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
-				afu->pasid_base,
-				afu->config.pasid_supported_log);
-	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
-		afu->pasid_base, pasid_count);
-	return 0;
-}
-
-static void reclaim_afu_pasid(struct ocxl_afu *afu)
-{
-	struct ocxl_fn *fn = afu->fn;
-	int start_offset, size;
-
-	start_offset = afu->pasid_base - fn->pasid_base;
-	size = 1 << afu->config.pasid_supported_log;
-	ocxl_pasid_afu_free(afu->fn, start_offset, size);
-}
-
-static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
-{
-	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
-	int rc, idx;
-
-	if (bar != 0 && bar != 2 && bar != 4)
-		return -EINVAL;
-
-	idx = bar >> 1;
-	if (fn->bar_used[idx]++ == 0) {
-		rc = pci_request_region(dev, bar, "ocxl");
-		if (rc)
-			return rc;
-	}
-	return 0;
-}
-
-static void release_fn_bar(struct ocxl_fn *fn, int bar)
-{
-	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
-	int idx;
-
-	if (bar != 0 && bar != 2 && bar != 4)
-		return;
-
-	idx = bar >> 1;
-	if (--fn->bar_used[idx] == 0)
-		pci_release_region(dev, bar);
-	WARN_ON(fn->bar_used[idx] < 0);
-}
-
-static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
-{
-	int rc;
-
-	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
-	if (rc)
-		return rc;
-
-	rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
-	if (rc) {
-		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
-		return rc;
-	}
-
-	afu->global_mmio_start =
-		pci_resource_start(dev, afu->config.global_mmio_bar) +
-		afu->config.global_mmio_offset;
-	afu->pp_mmio_start =
-		pci_resource_start(dev, afu->config.pp_mmio_bar) +
-		afu->config.pp_mmio_offset;
-
-	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
-				afu->config.global_mmio_size);
-	if (!afu->global_mmio_ptr) {
-		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
-		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
-		dev_err(&dev->dev, "Error mapping global mmio area\n");
-		return -ENOMEM;
-	}
-
-	/*
-	 * Leave an empty page between the per-process mmio area and
-	 * the AFU interrupt mappings
-	 */
-	afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
-	return 0;
-}
-
-static void unmap_mmio_areas(struct ocxl_afu *afu)
-{
-	if (afu->global_mmio_ptr) {
-		iounmap(afu->global_mmio_ptr);
-		afu->global_mmio_ptr = NULL;
-	}
-	afu->global_mmio_start = 0;
-	afu->pp_mmio_start = 0;
-	release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
-	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
-}
-
-static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
-{
-	int rc;
-
-	rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
-	if (rc)
-		return rc;
-
-	rc = set_afu_device(afu, dev_name(&dev->dev));
-	if (rc)
-		return rc;
-
-	rc = assign_afu_actag(afu, dev);
-	if (rc)
-		return rc;
-
-	rc = assign_afu_pasid(afu, dev);
-	if (rc) {
-		reclaim_afu_actag(afu);
-		return rc;
-	}
-
-	rc = map_mmio_areas(afu, dev);
-	if (rc) {
-		reclaim_afu_pasid(afu);
-		reclaim_afu_actag(afu);
-		return rc;
-	}
-	return 0;
-}
-
-static void deconfigure_afu(struct ocxl_afu *afu)
-{
-	unmap_mmio_areas(afu);
-	reclaim_afu_pasid(afu);
-	reclaim_afu_actag(afu);
-}
-
-static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
-{
-	int rc;
-
-	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
-	/*
-	 * Char device creation is the last step, as processes can
-	 * call our driver immediately, so all our inits must be finished.
-	 */
-	rc = ocxl_create_cdev(afu);
-	if (rc)
-		return rc;
-	return 0;
-}
-
-static void deactivate_afu(struct ocxl_afu *afu)
-{
-	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
-
-	ocxl_destroy_cdev(afu);
-	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
-}
-
-static int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
-{
-	int rc;
-	struct ocxl_afu *afu;
-
-	afu = alloc_afu(fn);
-	if (!afu)
-		return -ENOMEM;
-
-	rc = configure_afu(afu, afu_idx, dev);
-	if (rc) {
-		free_afu(afu);
-		return rc;
-	}
-
-	rc = ocxl_register_afu(afu);
-	if (rc)
-		goto err;
-
-	rc = ocxl_sysfs_add_afu(afu);
-	if (rc)
-		goto err;
-
-	rc = activate_afu(dev, afu);
-	if (rc)
-		goto err_sys;
-
-	list_add_tail(&afu->list, &fn->afu_list);
-	return 0;
-
-err_sys:
-	ocxl_sysfs_remove_afu(afu);
-err:
-	deconfigure_afu(afu);
-	device_unregister(&afu->dev);
-	return rc;
-}
-
-static void remove_afu(struct ocxl_afu *afu)
-{
-	list_del(&afu->list);
-	ocxl_context_detach_all(afu);
-	deactivate_afu(afu);
-	ocxl_sysfs_remove_afu(afu);
-	deconfigure_afu(afu);
-	device_unregister(&afu->dev);
-}
-
-static struct ocxl_fn *alloc_function(struct pci_dev *dev)
-{
-	struct ocxl_fn *fn;
-
-	fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
-	if (!fn)
-		return NULL;
-
-	INIT_LIST_HEAD(&fn->afu_list);
-	INIT_LIST_HEAD(&fn->pasid_list);
-	INIT_LIST_HEAD(&fn->actag_list);
-	return fn;
-}
-
-static void free_function(struct ocxl_fn *fn)
-{
-	WARN_ON(!list_empty(&fn->afu_list));
-	WARN_ON(!list_empty(&fn->pasid_list));
-	kfree(fn);
-}
-
-static void free_function_dev(struct device *dev)
-{
-	struct ocxl_fn *fn = to_ocxl_function(dev);
-
-	free_function(fn);
-}
-
-static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
-{
-	int rc;
-
-	fn->dev.parent = &dev->dev;
-	fn->dev.release = free_function_dev;
-	rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
-	if (rc)
-		return rc;
-	pci_set_drvdata(dev, fn);
-	return 0;
-}
-
-static int assign_function_actag(struct ocxl_fn *fn)
-{
-	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
-	u16 base, enabled, supported;
-	int rc;
-
-	rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
-	if (rc)
-		return rc;
-
-	fn->actag_base = base;
-	fn->actag_enabled = enabled;
-	fn->actag_supported = supported;
-
-	ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
-			fn->actag_base,	fn->actag_enabled);
-	dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
-		fn->actag_base, fn->actag_enabled);
-	return 0;
-}
-
-static int set_function_pasid(struct ocxl_fn *fn)
-{
-	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
-	int rc, desired_count, max_count;
-
-	/* A function may not require any PASID */
-	if (fn->config.max_pasid_log < 0)
-		return 0;
-
-	rc = ocxl_config_get_pasid_info(dev, &max_count);
-	if (rc)
-		return rc;
-
-	desired_count = 1 << fn->config.max_pasid_log;
-
-	if (desired_count > max_count) {
-		dev_err(&fn->dev,
-			"Function requires more PASIDs than is available (%d vs. %d)\n",
-			desired_count, max_count);
-		return -ENOSPC;
-	}
-
-	fn->pasid_base = 0;
-	return 0;
-}
-
-static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
-{
-	int rc;
-
-	rc = pci_enable_device(dev);
-	if (rc) {
-		dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
-		return rc;
-	}
-
-	/*
-	 * Once it has been confirmed to work on our hardware, we
-	 * should reset the function, to force the adapter to restart
-	 * from scratch.
-	 * A function reset would also reset all its AFUs.
-	 *
-	 * Some hints for implementation:
-	 *
-	 * - there's not status bit to know when the reset is done. We
-	 *   should try reading the config space to know when it's
-	 *   done.
-	 * - probably something like:
-	 *	Reset
-	 *	wait 100ms
-	 *	issue config read
-	 *	allow device up to 1 sec to return success on config
-	 *	read before declaring it broken
-	 *
-	 * Some shared logic on the card (CFG, TLX) won't be reset, so
-	 * there's no guarantee that it will be enough.
-	 */
-	rc = ocxl_config_read_function(dev, &fn->config);
-	if (rc)
-		return rc;
-
-	rc = set_function_device(fn, dev);
-	if (rc)
-		return rc;
-
-	rc = assign_function_actag(fn);
-	if (rc)
-		return rc;
-
-	rc = set_function_pasid(fn);
-	if (rc)
-		return rc;
-
-	rc = ocxl_link_setup(dev, 0, &fn->link);
-	if (rc)
-		return rc;
-
-	rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
-	if (rc) {
-		ocxl_link_release(dev, fn->link);
-		return rc;
-	}
-	return 0;
-}
-
-static void deconfigure_function(struct ocxl_fn *fn)
-{
-	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
-
-	ocxl_link_release(dev, fn->link);
-	pci_disable_device(dev);
-}
-
-static struct ocxl_fn *init_function(struct pci_dev *dev)
-{
-	struct ocxl_fn *fn;
-	int rc;
-
-	fn = alloc_function(dev);
-	if (!fn)
-		return ERR_PTR(-ENOMEM);
-
-	rc = configure_function(fn, dev);
-	if (rc) {
-		free_function(fn);
-		return ERR_PTR(rc);
-	}
-
-	rc = device_register(&fn->dev);
-	if (rc) {
-		deconfigure_function(fn);
-		put_device(&fn->dev);
-		return ERR_PTR(rc);
-	}
-	return fn;
-}
-
-static void remove_function(struct ocxl_fn *fn)
-{
-	deconfigure_function(fn);
-	device_unregister(&fn->dev);
-}
-
 static int ocxl_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int rc, afu_count = 0;
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 0/7] Refactor OCXL driver to allow external drivers to use it
From: Alastair D'Silva @ 2019-03-20  5:08 UTC (permalink / raw)
  To: alastair
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-1-alastair@au1.ibm.com>

From: Alastair D'Silva <alastair@d-silva.org>

This series reworks the OpenCAPI driver to split frontend
(driver interactions) from backend (hardware interactions).

This allows external drivers to utilise the core of the
generic OpenCAPI driver to communicate with specific
OpenCAPI hardware.

Changelog:
V2:
  - Reorder patches as some required structs that were only available later
  - Add dev.release implementation for ocxl_file_info to address warning on
    driver unload (ocxl: Create a clear delineation...)
  - Set output var irq_id in ocxl_afu_irq_alloc (ocxl: afu_irq only deals...)
  - Bump copyright year (ocxl: Provide global MMIO accessors...,
	ocxl: Split pci.c)

Alastair D'Silva (7):
  ocxl: Split pci.c
  ocxl: Don't pass pci_dev around
  ocxl: Create a clear delineation between ocxl backend & frontend
  ocxl: Allow external drivers to use OpenCAPI contexts
  ocxl: afu_irq only deals with IRQ IDs, not offsets
  ocxl: move event_fd handling to frontend
  ocxl: Provide global MMIO accessors for external drivers

 drivers/misc/ocxl/Makefile        |   3 +-
 drivers/misc/ocxl/afu_irq.c       |  97 ++---
 drivers/misc/ocxl/context.c       |  18 +-
 drivers/misc/ocxl/core.c          | 578 ++++++++++++++++++++++++++++++
 drivers/misc/ocxl/file.c          | 160 ++++++---
 drivers/misc/ocxl/mmio.c          | 234 ++++++++++++
 drivers/misc/ocxl/ocxl_internal.h |  49 +--
 drivers/misc/ocxl/pci.c           | 562 ++---------------------------
 drivers/misc/ocxl/sysfs.c         |  58 +--
 drivers/misc/ocxl/trace.h         |  12 +-
 include/misc/ocxl.h               | 322 ++++++++++++++++-
 11 files changed, 1390 insertions(+), 703 deletions(-)
 create mode 100644 drivers/misc/ocxl/core.c
 create mode 100644 drivers/misc/ocxl/mmio.c

-- 
2.20.1


^ permalink raw reply

* [PATCH v3 3/3] Documentation/vmcoreinfo: Add documentation for 'MAX_PHYSMEM_BITS'
From: Bhupesh Sharma @ 2019-03-20  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kazuhito Hagio, bhsharma, x86, Will Deacon, linuxppc-dev, kexec,
	James Morse, linux-arm-kernel, Boris Petkov, Thomas Gleixner,
	bhupesh.linux, Dave Anderson, Ingo Molnar, Paul Mackerras
In-Reply-To: <1553058574-18606-1-git-send-email-bhsharma@redhat.com>

Add documentation for 'MAX_PHYSMEM_BITS' variable being added to
vmcoreinfo.

'MAX_PHYSMEM_BITS' defines the maximum supported physical address
space memory.

Cc: Boris Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: kexec@lists.infradead.org
Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 Documentation/kdump/vmcoreinfo.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/kdump/vmcoreinfo.txt b/Documentation/kdump/vmcoreinfo.txt
index bb94a4bd597a..f5a11388dc49 100644
--- a/Documentation/kdump/vmcoreinfo.txt
+++ b/Documentation/kdump/vmcoreinfo.txt
@@ -95,6 +95,11 @@ It exists in the sparse memory mapping model, and it is also somewhat
 similar to the mem_map variable, both of them are used to translate an
 address.
 
+MAX_PHYSMEM_BITS
+----------------
+
+Defines the maximum supported physical address space memory.
+
 page
 ----
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 2/3] crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
From: Bhupesh Sharma @ 2019-03-20  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kazuhito Hagio, bhsharma, x86, Will Deacon, linuxppc-dev, kexec,
	James Morse, linux-arm-kernel, Boris Petkov, Thomas Gleixner,
	bhupesh.linux, Dave Anderson, Ingo Molnar, Paul Mackerras
In-Reply-To: <1553058574-18606-1-git-send-email-bhsharma@redhat.com>

Right now user-space tools like 'makedumpfile' and 'crash' need to rely
on a best-guess method of determining value of 'MAX_PHYSMEM_BITS'
supported by underlying kernel.

This value is used in user-space code to calculate the bit-space
required to store a section for SPARESMEM (similar to the existing
calculation method used in the kernel implementation):

  #define SECTIONS_SHIFT    (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)

Now, regressions have been reported in user-space utilities
like 'makedumpfile' and 'crash' on arm64, with the recently added
kernel support for 52-bit physical address space, as there is
no clear method of determining this value in user-space
(other than reading kernel CONFIG flags).

As per suggestion from makedumpfile maintainer (Kazu), it makes more
sense to append 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself
rather than in arch-specific code, so that the user-space code for other
archs can also benefit from this addition to the vmcoreinfo and use it
as a standard way of determining 'SECTIONS_SHIFT' value in user-land.

A reference 'makedumpfile' implementation which reads the
'MAX_PHYSMEM_BITS' value from vmcoreinfo in a arch-independent fashion
is available here:

[0]. https://github.com/bhupesh-sharma/makedumpfile/blob/remove-max-phys-mem-bit-v1/arch/ppc64.c#L471

Cc: Boris Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: kexec@lists.infradead.org
Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 kernel/crash_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 093c9f917ed0..495f09084696 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -415,6 +415,7 @@ static int __init crash_save_vmcoreinfo_init(void)
 	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
 	VMCOREINFO_STRUCT_SIZE(mem_section);
 	VMCOREINFO_OFFSET(mem_section, section_mem_map);
+	VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
 #endif
 	VMCOREINFO_STRUCT_SIZE(page);
 	VMCOREINFO_STRUCT_SIZE(pglist_data);
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 0/3] Append new variables to vmcoreinfo (PTRS_PER_PGD for arm64 and MAX_PHYSMEM_BITS for all archs)
From: Bhupesh Sharma @ 2019-03-20  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, Kazuhito Hagio, bhsharma, x86, Will Deacon,
	linuxppc-dev, kexec, James Morse, linux-arm-kernel, Boris Petkov,
	Thomas Gleixner, bhupesh.linux, Dave Anderson, Ingo Molnar,
	Paul Mackerras

Changes since v2:
----------------
- v2 can be seen here:
  http://lists.infradead.org/pipermail/kexec/2019-March/022531.html
- Protected 'MAX_PHYSMEM_BITS' vmcoreinfo variable under CONFIG_SPARSEMEM
  ifdef sections, as suggested by Kazu.
- Updated vmcoreinfo documentation to add description about
  'MAX_PHYSMEM_BITS' variable (via [PATCH 3/3]).

Changes since v1:
----------------
- v1 was sent out as a single patch which can be seen here:
  http://lists.infradead.org/pipermail/kexec/2019-February/022411.html

- v2 breaks the single patch into two independent patches:
  [PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
  [PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)

This patchset primarily fixes the regression reported in user-space
utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
with the availability of 52-bit address space feature in underlying
kernel. These regressions have been reported both on CPUs which don't
support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
and also on prototype platforms (like ARMv8 FVP simulator model) which
support ARMv8.2 extensions and are running newer kernels.

The reason for these regressions is that right now user-space tools
have no direct access to these values (since these are not exported
from the kernel) and hence need to rely on a best-guess method of
determining value of 'PTRS_PER_PGD' and 'MAX_PHYSMEM_BITS' supported
by underlying kernel.

Exporting these values via vmcoreinfo will help user-land in such cases.
In addition, as per suggestion from makedumpfile maintainer (Kazu),
it makes more sense to append 'MAX_PHYSMEM_BITS' to
vmcoreinfo in the core code itself rather than in arm64 arch-specific
code, so that the user-space code for other archs can also benefit from
this addition to the vmcoreinfo and use it as a standard way of
determining 'SECTIONS_SHIFT' value in user-land.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Boris Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: kexec@lists.infradead.org

Bhupesh Sharma (3):
  arm64, vmcoreinfo : Append 'PTRS_PER_PGD' to vmcoreinfo
  crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
  Documentation/vmcoreinfo: Add documentation for 'MAX_PHYSMEM_BITS'

 Documentation/kdump/vmcoreinfo.txt | 5 +++++
 arch/arm64/kernel/crash_core.c     | 1 +
 kernel/crash_core.c                | 1 +
 3 files changed, 7 insertions(+)

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH v3 06/17] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: David Gibson @ 2019-03-20  3:44 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <3b59a5be-ebde-8b1b-36af-5336cfb35cb3@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 16966 bytes --]

On Tue, Mar 19, 2019 at 04:47:20PM +0100, Cédric Le Goater wrote:
> On 3/19/19 5:54 AM, David Gibson wrote:
> > On Mon, Mar 18, 2019 at 03:12:10PM +0100, Cédric Le Goater wrote:
> >> On 3/18/19 4:23 AM, David Gibson wrote:
> >>> On Fri, Mar 15, 2019 at 01:05:58PM +0100, Cédric Le Goater wrote:
> >>>> These controls will be used by the H_INT_SET_QUEUE_CONFIG and
> >>>> H_INT_GET_QUEUE_CONFIG hcalls from QEMU to configure the underlying
> >>>> Event Queue in the XIVE IC. They will also be used to restore the
> >>>> configuration of the XIVE EQs and to capture the internal run-time
> >>>> state of the EQs. Both 'get' and 'set' rely on an OPAL call to access
> >>>> the EQ toggle bit and EQ index which are updated by the XIVE IC when
> >>>> event notifications are enqueued in the EQ.
> >>>>
> >>>> The value of the guest physical address of the event queue is saved in
> >>>> the XIVE internal xive_q structure for later use. That is when
> >>>> migration needs to mark the EQ pages dirty to capture a consistent
> >>>> memory state of the VM.
> >>>>
> >>>> To be noted that H_INT_SET_QUEUE_CONFIG does not require the extra
> >>>> OPAL call setting the EQ toggle bit and EQ index to configure the EQ,
> >>>> but restoring the EQ state will.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> ---
> >>>>
> >>>>  Changes since v2 :
> >>>>  
> >>>>  - fixed comments on the KVM device attribute definitions
> >>>>  - fixed check on supported EQ size to restrict to 64K pages
> >>>>  - checked kvm_eq.flags that need to be zero
> >>>>  - removed the OPAL call when EQ qtoggle bit and index are zero. 
> >>>>
> >>>>  arch/powerpc/include/asm/xive.h            |   2 +
> >>>>  arch/powerpc/include/uapi/asm/kvm.h        |  21 ++
> >>>>  arch/powerpc/kvm/book3s_xive.h             |   2 +
> >>>>  arch/powerpc/kvm/book3s_xive.c             |  15 +-
> >>>>  arch/powerpc/kvm/book3s_xive_native.c      | 232 +++++++++++++++++++++
> >>>>  Documentation/virtual/kvm/devices/xive.txt |  31 +++
> >>>>  6 files changed, 297 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> >>>> index b579a943407b..46891f321606 100644
> >>>> --- a/arch/powerpc/include/asm/xive.h
> >>>> +++ b/arch/powerpc/include/asm/xive.h
> >>>> @@ -73,6 +73,8 @@ struct xive_q {
> >>>>  	u32			esc_irq;
> >>>>  	atomic_t		count;
> >>>>  	atomic_t		pending_count;
> >>>> +	u64			guest_qpage;
> >>>> +	u32			guest_qsize;
> >>>>  };
> >>>>  
> >>>>  /* Global enable flags for the XIVE support */
> >>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> index 12bb01baf0ae..1cd728c87d7c 100644
> >>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> @@ -679,6 +679,7 @@ struct kvm_ppc_cpu_char {
> >>>>  #define KVM_DEV_XIVE_GRP_CTRL		1
> >>>>  #define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
> >>>>  #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
> >>>> +#define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
> >>>>  
> >>>>  /* Layout of 64-bit XIVE source attribute values */
> >>>>  #define KVM_XIVE_LEVEL_SENSITIVE	(1ULL << 0)
> >>>> @@ -694,4 +695,24 @@ struct kvm_ppc_cpu_char {
> >>>>  #define KVM_XIVE_SOURCE_EISN_SHIFT	33
> >>>>  #define KVM_XIVE_SOURCE_EISN_MASK	0xfffffffe00000000ULL
> >>>>  
> >>>> +/* Layout of 64-bit EQ identifier */
> >>>> +#define KVM_XIVE_EQ_PRIORITY_SHIFT	0
> >>>> +#define KVM_XIVE_EQ_PRIORITY_MASK	0x7
> >>>> +#define KVM_XIVE_EQ_SERVER_SHIFT	3
> >>>> +#define KVM_XIVE_EQ_SERVER_MASK		0xfffffff8ULL
> >>>> +
> >>>> +/* Layout of EQ configuration values (64 bytes) */
> >>>> +struct kvm_ppc_xive_eq {
> >>>> +	__u32 flags;
> >>>> +	__u32 qsize;
> >>>> +	__u64 qpage;
> >>>> +	__u32 qtoggle;
> >>>> +	__u32 qindex;
> >>>> +	__u8  pad[40];
> >>>> +};
> >>>> +
> >>>> +#define KVM_XIVE_EQ_FLAG_ENABLED	0x00000001
> >>>> +#define KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY	0x00000002
> >>>> +#define KVM_XIVE_EQ_FLAG_ESCALATE	0x00000004
> >>>> +
> >>>>  #endif /* __LINUX_KVM_POWERPC_H */
> >>>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
> >>>> index ae26fe653d98..622f594d93e1 100644
> >>>> --- a/arch/powerpc/kvm/book3s_xive.h
> >>>> +++ b/arch/powerpc/kvm/book3s_xive.h
> >>>> @@ -272,6 +272,8 @@ struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
> >>>>  	struct kvmppc_xive *xive, int irq);
> >>>>  void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
> >>>>  int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio);
> >>>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> >>>> +				  bool single_escalation);
> >>>>  
> >>>>  #endif /* CONFIG_KVM_XICS */
> >>>>  #endif /* _KVM_PPC_BOOK3S_XICS_H */
> >>>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> >>>> index e09f3addffe5..c1b7aa7dbc28 100644
> >>>> --- a/arch/powerpc/kvm/book3s_xive.c
> >>>> +++ b/arch/powerpc/kvm/book3s_xive.c
> >>>> @@ -166,7 +166,8 @@ static irqreturn_t xive_esc_irq(int irq, void *data)
> >>>>  	return IRQ_HANDLED;
> >>>>  }
> >>>>  
> >>>> -static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
> >>>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> >>>> +				  bool single_escalation)
> >>>>  {
> >>>>  	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> >>>>  	struct xive_q *q = &xc->queues[prio];
> >>>> @@ -185,7 +186,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
> >>>>  		return -EIO;
> >>>>  	}
> >>>>  
> >>>> -	if (xc->xive->single_escalation)
> >>>> +	if (single_escalation)
> >>>>  		name = kasprintf(GFP_KERNEL, "kvm-%d-%d",
> >>>>  				 vcpu->kvm->arch.lpid, xc->server_num);
> >>>>  	else
> >>>> @@ -217,7 +218,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
> >>>>  	 * interrupt, thus leaving it effectively masked after
> >>>>  	 * it fires once.
> >>>>  	 */
> >>>> -	if (xc->xive->single_escalation) {
> >>>> +	if (single_escalation) {
> >>>>  		struct irq_data *d = irq_get_irq_data(xc->esc_virq[prio]);
> >>>>  		struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
> >>>>  
> >>>> @@ -291,7 +292,8 @@ static int xive_check_provisioning(struct kvm *kvm, u8 prio)
> >>>>  			continue;
> >>>>  		rc = xive_provision_queue(vcpu, prio);
> >>>>  		if (rc == 0 && !xive->single_escalation)
> >>>> -			xive_attach_escalation(vcpu, prio);
> >>>> +			kvmppc_xive_attach_escalation(vcpu, prio,
> >>>> +						      xive->single_escalation);
> >>>>  		if (rc)
> >>>>  			return rc;
> >>>>  	}
> >>>> @@ -1214,7 +1216,8 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> >>>>  		if (xive->qmap & (1 << i)) {
> >>>>  			r = xive_provision_queue(vcpu, i);
> >>>>  			if (r == 0 && !xive->single_escalation)
> >>>> -				xive_attach_escalation(vcpu, i);
> >>>> +				kvmppc_xive_attach_escalation(
> >>>> +					vcpu, i, xive->single_escalation);
> >>>>  			if (r)
> >>>>  				goto bail;
> >>>>  		} else {
> >>>> @@ -1229,7 +1232,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> >>>>  	}
> >>>>  
> >>>>  	/* If not done above, attach priority 0 escalation */
> >>>> -	r = xive_attach_escalation(vcpu, 0);
> >>>> +	r = kvmppc_xive_attach_escalation(vcpu, 0, xive->single_escalation);
> >>>>  	if (r)
> >>>>  		goto bail;
> >>>>  
> >>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> >>>> index b841d339f674..42e824658a30 100644
> >>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
> >>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> >>>> @@ -340,6 +340,226 @@ static int kvmppc_xive_native_set_source_config(struct kvmppc_xive *xive,
> >>>>  						       priority, masked, eisn);
> >>>>  }
> >>>>  
> >>>> +static int xive_native_validate_queue_size(u32 qsize)
> >>>> +{
> >>>> +	/*
> >>>> +	 * We only support 64K pages for the moment. This is also
> >>>> +	 * advertised in the DT property "ibm,xive-eq-sizes"
> >>>
> >>> IIUC, that won't work properly if you had a guest using 4kiB pages.
> >>
> >>> That's fine, but do we have somewhere that checks for that case and
> >>> throws a suitable error?
> >>
> >> Not in the device. 
> >>
> >> So, we should check the current page_size of the guest ? Is there a way 
> >> to do that simply from KVM ?
> > 
> > Not really.  But I think I know where to make the necessary test, see
> > comment below..
> > 
> >>>> +	 */
> >>>> +	switch (qsize) {
> >>>> +	case 0: /* EQ reset */
> >>>> +	case 16:
> >>>> +		return 0;
> >>>> +	case 12:
> >>>> +	case 21:
> >>>> +	case 24:
> >>>> +	default:
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>> +}
> >>>> +
> >>>> +static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
> >>>> +					       long eq_idx, u64 addr)
> >>>> +{
> >>>> +	struct kvm *kvm = xive->kvm;
> >>>> +	struct kvm_vcpu *vcpu;
> >>>> +	struct kvmppc_xive_vcpu *xc;
> >>>> +	void __user *ubufp = (u64 __user *) addr;
> >>>
> >>> Nit: that should be (void __user *) on the right, shouldn't it?
> >>
> >> yes.
> >>
> >>>
> >>>> +	u32 server;
> >>>> +	u8 priority;
> >>>> +	struct kvm_ppc_xive_eq kvm_eq;
> >>>> +	int rc;
> >>>> +	__be32 *qaddr = 0;
> >>>> +	struct page *page;
> >>>> +	struct xive_q *q;
> >>>> +
> >>>> +	/*
> >>>> +	 * Demangle priority/server tuple from the EQ identifier
> >>>> +	 */
> >>>> +	priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
> >>>> +		KVM_XIVE_EQ_PRIORITY_SHIFT;
> >>>> +	server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
> >>>> +		KVM_XIVE_EQ_SERVER_SHIFT;
> >>>> +
> >>>> +	if (copy_from_user(&kvm_eq, ubufp, sizeof(kvm_eq)))
> >>>> +		return -EFAULT;
> >>>> +
> >>>> +	vcpu = kvmppc_xive_find_server(kvm, server);
> >>>> +	if (!vcpu) {
> >>>> +		pr_err("Can't find server %d\n", server);
> >>>> +		return -ENOENT;
> >>>> +	}
> >>>> +	xc = vcpu->arch.xive_vcpu;
> >>>> +
> >>>> +	if (priority != xive_prio_from_guest(priority)) {
> >>>> +		pr_err("Trying to restore invalid queue %d for VCPU %d\n",
> >>>> +		       priority, server);
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>> +	q = &xc->queues[priority];
> >>>> +
> >>>> +	pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
> >>>> +		 __func__, server, priority, kvm_eq.flags,
> >>>> +		 kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
> >>>> +
> >>>> +	/*
> >>>> +	 * We can not tune the EQ configuration from user space. All
> >>>> +	 * is done in OPAL.
> >>>> +	 */
> >>>> +	if (kvm_eq.flags != 0) {
> >>>> +		pr_err("invalid flags %d\n", kvm_eq.flags);
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>> +
> >>>> +	rc = xive_native_validate_queue_size(kvm_eq.qsize);
> >>>> +	if (rc) {
> >>>> +		pr_err("invalid queue size %d\n", kvm_eq.qsize);
> >>>> +		return rc;
> >>>> +	}
> >>>> +
> >>>> +	/* reset queue and disable queueing */
> >>>> +	if (!kvm_eq.qsize) {
> >>>> +		q->guest_qpage = 0;
> >>>> +		q->guest_qsize = 0;
> >>>> +
> >>>> +		rc = xive_native_configure_queue(xc->vp_id, q, priority,
> >>>> +						 NULL, 0, true);
> >>>> +		if (rc) {
> >>>> +			pr_err("Failed to reset queue %d for VCPU %d: %d\n",
> >>>> +			       priority, xc->server_num, rc);
> >>>> +			return rc;
> >>>> +		}
> >>>> +
> >>>> +		if (q->qpage) {
> >>>> +			put_page(virt_to_page(q->qpage));
> >>>> +			q->qpage = NULL;
> >>>> +		}
> >>>> +
> >>>> +		return 0;
> >>>> +	}
> >>>> +
> >>>> +
> >>>> +	page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
> >>>> +	if (is_error_page(page)) {
> >>>> +		pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>
> >>> Yeah.. for the case of a 4kiB page host (these days weird, but not
> >>> actually prohibited, AFAIK) you need to check that the qsize selected
> >>> actually fits within the page.
> >>
> >> Ah yes. sure.
> > 
> > I think the pagesize test belongs here.  Rather than thinking about
> > the pagesize of the guest overall, you can check that this specific
> > page (possibly compound) is large enough to take the requested queue
> > size.
> 
> OK. It think kvm_host_page_size() is what we need. It returns the page
> size of the underlying VMA of the memblock holding the gfn. So I am going 
> to add :

Yes, that sounds good.

> +	page_size = kvm_host_page_size(kvm, gfn);
> +	if (1ull << kvm_eq.qshift > page_size) {
> +		pr_warn("Incompatible host page size %lx!\n", page_size);
> +		return -EINVAL;
> +	}
> +
> 
> Also I am renaming 'qsize' in 'qshift' and renaming 'qpage' to 'qaddr'.
> 
> > That should be enough to protect the host - it ensures that userspace
> > owns a suitable contiguous chunk of memory for the XIVE to write the
> > queue into.
> > 
> > It's possible there are weirder edge cases with a large page that's
> > not fully mapped into the guest - if necessary we can add tests for
> > that on the qemu side.
> > 
> > Oh.. it occurs to me that we might need to pin the queue page to make
> > sure it doesn't get swapped out or page-migrated while the XIVE holds
> > a pointer to it
> 
> That is what gfn_to_page() ends up doing by calling hva_to_pfn().

Ah, ok.

> >>>> +	/*
> >>>> +	 * Return some information on the EQ configuration in
> >>>> +	 * OPAL. This is purely informative for now as we can't really
> >>>> +	 * tune the EQ configuration from user space.
> >>>> +	 */
> >>>> +	kvm_eq.flags = 0;
> >>>> +	if (qflags & OPAL_XIVE_EQ_ENABLED)
> >>>> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ENABLED;
> >>>> +	if (qflags & OPAL_XIVE_EQ_ALWAYS_NOTIFY)
> >>>> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY;
> >>>> +	if (qflags & OPAL_XIVE_EQ_ESCALATE)
> >>>> +		kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ESCALATE;
> >>>
> >>> If there's not really anything it can do about it, does it make sense
> >>> to even expose this info to userspace?
> >>
> >> Hmm, good question. 
> >>
> >>  - KVM_XIVE_EQ_FLAG_ENABLED		
> >> 	may be uselessly obvious.
> > 
> > What's it controlled by?
> 
> OPAL only. It's equivalent to the VALID bit in the XIVE END structure. 
> We can drop this one. 

Ok.

> >>  - KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY 	
> >> 	means we do not use the END ESBs to coalesce the events at the END
> >> 	level. This flag is reflected by the XIVE_EQ_ALWAYS_NOTIFY option
> >> 	in the sPAPR specs. We don't support the END ESBs but we might one
> >> 	day.
> > 
> > Since the guest isn't currently permitted to set this, it should never
> > be set here either, no?
> 
> The OS does not support the END ESBs so this flag is always set in the 
> Linux XIVE driver. END ESBs are supported in the emulated device but not 
> in KVM/OPAL.

Ok, it's reasonable to keep this then.

> >>  - KVM_XIVE_EQ_FLAG_ESCALATE
> >> 	means the EQ is an escalation. QEMU doesn't really care for now 
> >> 	but it's an important information I think.
> > 
> > Likewise this one, yes?
> 
> That is an hypervisor information on the nature of the EQ, so it is 
> for QEMU and not the guest. I am not sure it is important today as
> we don't support escalation in QEMU. May be drop ? 

Yes, I think drop it.  We can add something later if we have a
concrete use case for it.

> >> I tried not to add too many of the END flags, only the relevant ones which
> >> could have an impact in the future modeling.  
> >>
> >> I think KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY is important. I was setting it from
> >> QEMU in the hcall but as OPAL does the same blindly I removed it in
> >> v3.
> > 
> > So, I might have misinterpreted this a bit the first time around.  Am
> > I correct in thinking that these bits all correspond to defined
> > options in the PAPR hcall
> 
> Yes for KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY which is the only flag defined 
> in sPAPR. 
> 
> > - but that for now we don't allow guests to
> > set them (because we haven't implemented support so far).
> 
> It's a bit more complex.
> 
> KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY is a "required" flag as the OS does not 
> support END ESBs. END ESBs are not supported in KVM/OPAL but they are 
> in the QEMU device. 

So, the (current) guest needs this behaviour, but I'm guessing PAPR
doesn't require it.  Is this behaviour configurable in the PAPR interface?

> I think it would be good for consistency to set KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY
> when calling kvmppc_xive_native_get_queue_config() from QEMU, as I did 
> in v2. But as OPAL forces the same behavior without any flag, it is not 
> really needed at the KVM level ...
> 
> Please tell me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH 7/8] powerpc/eeh: EEH for pSeries hot plug
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

On PowerNV and pSeries, devices currently acquire EEH support from
several different places: Boot-time devices from eeh_probe_devices()
and eeh_addr_cache_build(), Virtual Function devices from the pcibios
bus add device hooks and hot plugged devices from pci_hp_add_devices()
(with other platforms using other methods as well).  Unfortunately,
pSeries machines currently discover hot plugged devices using
pci_rescan_bus(), not pci_hp_add_devices(), and so those devices do
not receive EEH support.

Rather than adding another case for pci_rescan_bus(), this change
widens the scope of the pcibios bus add device hooks so that they can
handle all devices. As a side effect this also supports devices
discovered after manually rescanning via /sys/bus/pci/rescan.

Note that on PowerNV, this change allows the EEH subsystem to become
enabled after boot as long as it has not been forced off, which was
not previously possible (it was already possible on pSeries).

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c |  8 ++-
 arch/powerpc/platforms/pseries/eeh_pseries.c | 54 ++++++++++----------
 4 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 7a406d58d2c0..217e14bb1fb6 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	struct pci_dn *pdn;
 	struct eeh_dev *edev;
 
-	if (!dev || !eeh_enabled())
+	if (!dev)
 		return;
 
 	pr_debug("EEH: Adding device %s\n", pci_name(dev));
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index becaec990140..d5818e9c4069 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -86,7 +86,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
 	pcibios_claim_one_bus(phb->bus);
 
 	/* Finish EEH setup */
-	eeh_add_device_tree_late(phb->bus);
+	if (!eeh_has_flag(EEH_FORCE_DISABLED))
+		eeh_add_device_tree_late(phb->bus);
 
 	/* Add probed PCI devices to the device model */
 	pci_bus_add_devices(phb->bus);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 51c5b6bb9b0e..81b0923cc55f 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 
-	if (!pdev->is_virtfn)
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
 	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
@@ -479,7 +479,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	 * Enable EEH explicitly so that we will do EEH check
 	 * while accessing I/O stuff
 	 */
-	eeh_add_flag(EEH_ENABLED);
+	if (!eeh_has_flag(EEH_ENABLED)) {
+		enable_irq(eeh_event_irq);
+		eeh_add_flag(EEH_PHB_ENABLED);
+		eeh_add_flag(EEH_ENABLED);
+	}
 
 	/* Save memory bars */
 	eeh_save_bars(edev);
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ae06878fbdea..e68c79164974 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -55,44 +55,44 @@ static int ibm_get_config_addr_info;
 static int ibm_get_config_addr_info2;
 static int ibm_configure_pe;
 
-#ifdef CONFIG_PCI_IOV
 void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
-	struct pci_dn *physfn_pdn;
-	struct eeh_dev *edev;
 
-	if (!pdev->is_virtfn)
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
 	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+#ifdef CONFIG_PCI_IOV
+	if (pdev->is_virtfn) {
+		struct pci_dn *physfn_pdn;
 
-	pdn->device_id  =  pdev->device;
-	pdn->vendor_id  =  pdev->vendor;
-	pdn->class_code =  pdev->class;
-	/*
-	 * Last allow unfreeze return code used for retrieval
-	 * by user space in eeh-sysfs to show the last command
-	 * completion from platform.
-	 */
-	pdn->last_allow_rc =  0;
-	physfn_pdn      =  pci_get_pdn(pdev->physfn);
-	pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
-	edev = pdn_to_eeh_dev(pdn);
-
-	/*
-	 * The following operations will fail if VF's sysfs files
-	 * aren't created or its resources aren't finalized.
-	 */
+		pdn->device_id  =  pdev->device;
+		pdn->vendor_id  =  pdev->vendor;
+		pdn->class_code =  pdev->class;
+		/*
+		 * Last allow unfreeze return code used for retrieval
+		 * by user space in eeh-sysfs to show the last command
+		 * completion from platform.
+		 */
+		pdn->last_allow_rc =  0;
+		physfn_pdn      =  pci_get_pdn(pdev->physfn);
+		pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
+	}
+#endif
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
-	edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
-	eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
-	eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
-	eeh_sysfs_add_device(pdev);
+#ifdef CONFIG_PCI_IOV
+	if (pdev->is_virtfn) {
+		struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
 
-}
+		edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
+		eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
+		eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
+	}
 #endif
+	eeh_sysfs_add_device(pdev);
+}
 
 /*
  * Buffer for reporting slot-error-detail rtas calls. Its here
@@ -159,10 +159,8 @@ static int pseries_eeh_init(void)
 	/* Set EEH probe mode */
 	eeh_add_flag(EEH_PROBE_MODE_DEVTREE | EEH_ENABLE_IO_FOR_LOG);
 
-#ifdef CONFIG_PCI_IOV
 	/* Set EEH machine dependent code */
 	ppc_md.pcibios_bus_add_device = pseries_pcibios_bus_add_device;
-#endif
 
 	return 0;
 }
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 8/8] powerpc/eeh: Remove eeh_probe_devices() and eeh_addr_cache_build()
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

Now that EEH support for all devices (on PowerNV and pSeries) is
provided by the pcibios bus add device hooks, eeh_probe_devices() and
eeh_addr_cache_build() are redundant and can be removed.

Note that previously on pSeries, useless EEH sysfs files were created
for some devices that did not have EEH support and this change
prevents them from being created.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               |  6 ----
 arch/powerpc/kernel/eeh.c                    | 13 --------
 arch/powerpc/kernel/eeh_cache.c              | 32 --------------------
 arch/powerpc/platforms/powernv/eeh-powernv.c |  5 ++-
 arch/powerpc/platforms/pseries/pci.c         |  3 +-
 5 files changed, 3 insertions(+), 56 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 791b9e6fcc45..f1eca1757cbc 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -290,13 +290,11 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
 struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
 void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
 void eeh_show_enabled(void);
-void eeh_probe_devices(void);
 int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
 void eeh_addr_cache_init(void);
-void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);
 void eeh_add_device_late(struct pci_dev *);
@@ -347,8 +345,6 @@ static inline bool eeh_phb_enabled(void)
 	return false;
 }
 
-static inline void eeh_probe_devices(void) { }
-
 static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
 {
 	return NULL;
@@ -365,8 +361,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 static inline void eeh_addr_cache_init(void) { }
 
-static inline void eeh_addr_cache_build(void) { }
-
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }
 
 static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 217e14bb1fb6..cd2abbe41497 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1166,19 +1166,6 @@ static struct notifier_block eeh_reboot_nb = {
 	.notifier_call = eeh_reboot_notifier,
 };
 
-void eeh_probe_devices(void)
-{
-	struct pci_controller *hose, *tmp;
-	struct pci_dn *pdn;
-
-	/* Enable EEH for all adapters */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
-		pdn = hose->pci_data;
-		traverse_pci_dn(pdn, eeh_ops->probe, NULL);
-	}
-	eeh_show_enabled();
-}
-
 /**
  * eeh_init - EEH initialization
  *
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index f93dd5cf6a39..c40078d036af 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -278,38 +278,6 @@ void eeh_addr_cache_init(void)
 	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
 }
 
-/**
- * eeh_addr_cache_build - Build a cache of I/O addresses
- *
- * Build a cache of pci i/o addresses.  This cache will be used to
- * find the pci device that corresponds to a given address.
- * This routine scans all pci busses to build the cache.
- * Must be run late in boot process, after the pci controllers
- * have been scanned for devices (after all device resources are known).
- */
-void eeh_addr_cache_build(void)
-{
-	struct pci_dn *pdn;
-	struct eeh_dev *edev;
-	struct pci_dev *dev = NULL;
-
-	for_each_pci_dev(dev) {
-		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
-		if (!pdn)
-			continue;
-
-		edev = pdn_to_eeh_dev(pdn);
-		if (!edev)
-			continue;
-
-		dev->dev.archdata.edev = edev;
-		edev->pdev = dev;
-
-		eeh_addr_cache_insert_dev(dev);
-		eeh_sysfs_add_device(dev);
-	}
-}
-
 static int eeh_addr_cache_show(struct seq_file *s, void *v)
 {
 	struct pci_io_addr_range *piar;
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 81b0923cc55f..6a08f4fab255 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -240,9 +240,7 @@ int pnv_eeh_post_init(void)
 	struct pnv_phb *phb;
 	int ret = 0;
 
-	/* Probe devices & build address cache */
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 
 	/* Register OPAL event notifier */
 	eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
@@ -360,6 +358,7 @@ static int pnv_eeh_find_ecap(struct pci_dn *pdn, int cap)
 	return 0;
 }
 
+
 /**
  * pnv_eeh_probe - Do probe on PCI device
  * @pdn: PCI device node
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 7be80882c08d..7c781b04fb15 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -242,8 +242,7 @@ void __init pSeries_final_fixup(void)
 
 	pSeries_request_regions();
 
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 #ifdef CONFIG_EEH
 	if (eeh_enabled())
 		eeh_add_flag(EEH_PHB_ENABLED);
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 2/8] powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

The EEH_DEV_NO_HANDLER flag is used by the EEH system to prevent the
use of driver callbacks in drivers that have been bound part way
through the recovery process. This is necessary to prevent later stage
handlers from being called when the earlier stage handlers haven't,
which can be confusing for drivers.

However, the flag is set for all devices that are added after boot
time and only cleared at the end of the EEH recovery process. This
results in hot plugged devices erroneously having the flag set during
the first recovery after they are added (causing their driver's
handlers to be incorrectly ignored).

To remedy this, clear the flag at the beginning of recovery
processing. The flag is still cleared at the end of recovery
processing, although it is no longer really necessary.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 6f3ee30565dd..4c34b9901f15 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -819,6 +819,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 		result = PCI_ERS_RESULT_DISCONNECT;
 	}
 
+	eeh_for_each_pe(pe, tmp_pe)
+		eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+			edev->mode &= ~EEH_DEV_NO_HANDLER;
+
 	/* Walk the various device drivers attached to this slot through
 	 * a reset sequence, giving each an opportunity to do what it needs
 	 * to accomplish the reset.  Each child gets a report of the
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 4/8] powerpc/eeh: Improve debug messages around device addition
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

Also remove useless comment.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 8d3c36a1f194..b14d89547895 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 	edev = pdn_to_eeh_dev(pdn);
 	if (edev->pdev == dev) {
-		pr_debug("EEH: Already referenced !\n");
+		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
 		return;
 	}
 
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index f0a95f663810..51c5b6bb9b0e 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
-	/*
-	 * The following operations will fail if VF's sysfs files
-	 * aren't created or its resources aren't finalized.
-	 */
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
 	eeh_sysfs_add_device(pdev);
@@ -389,6 +386,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	int ret;
 	int config_addr = (pdn->busno << 8) | (pdn->devfn);
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, hose->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/*
 	 * When probing the root bridge, which doesn't have any
 	 * subordinate PCI devices. We don't have OF node for
@@ -483,6 +484,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	/* Save memory bars */
 	eeh_save_bars(edev);
 
+	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
+		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
+		edev->pe->addr);
+
 	return NULL;
 }
 
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 7aa50258dd42..ae06878fbdea 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+
 	pdn->device_id  =  pdev->device;
 	pdn->vendor_id  =  pdev->vendor;
 	pdn->class_code =  pdev->class;
@@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 	int enable = 0;
 	int ret;
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, pdn->phb->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/* Retrieve OF node and eeh device */
 	edev = pdn_to_eeh_dev(pdn);
 	if (!edev || edev->pe)
@@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 
 	/* Enable EEH on the device */
 	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
-	if (!ret) {
+	if (ret) {
+		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
+	} else {
 		/* Retrieve PE address */
 		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
 		pe.addr = edev->pe_config_addr;
@@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 		if (enable) {
 			eeh_add_flag(EEH_ENABLED);
 			eeh_add_to_parent_pe(edev);
-
-			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
-				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
-				PCI_FUNC(pdn->devfn), pe.phb->global_number,
-				pe.addr);
 		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
 			   (pdn_to_eeh_dev(pdn->parent))->pe) {
 			/* This device doesn't support EEH, but it may have an
@@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
 			eeh_add_to_parent_pe(edev);
 		}
+		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, (enable ? "enabled" : "unsupported"),
+			pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
 	}
 
 	/* Save memory bars */
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 0/8]
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev

Hi all,

This patch set adds support for EEH recovery of hot plugged devices on pSeries
machines. Specifically, devices discovered by PCI rescanning using
/sys/bus/pci/rescan, which includes devices hotplugged by QEMU's device_add
command. (pSeries doesn't currently use slot power control for hotplugging.)

As a side effect this also provides EEH support for devices removed by
/sys/bus/pci/devices/*/remove and re-discovered by writing to /sys/bus/pci/rescan,
on all platforms.

The approach I've taken is to use the fact that the existing
pcibios_bus_add_device() platform hooks (which are used to set up EEH on
Virtual Function devices (VFs)) are actually called for all devices, so I've
widened their scope and made other adjustments necessary to allow them to work
for hotplugged and boot-time devices as well.

Because some of the changes are in generic PowerPC code, it's
possible that I've disturbed something for another PowerPC platform. I've tried
to minimize this by leaving that code alone as much as possible and so there
are a few cases where eeh_add_device_{early,late}() or eeh_add_sysfs_files() is
called more than once. I think these can be looked at later, as duplicate calls
are not harmful.

The patch "Convert PNV_PHB_FLAG_EEH" isn't strictly necessary and I'm not sure
if it's better to keep it, because it simplifies the code or drop it, because
we may need a separate flag per PHB later on. Thoughts anyone?

The first patch is a rework of the pcibios_init reordering patch I posted
earlier, which I've included here because it's necessary for this set.

I have done some testing for PowerNV on Power9 using a modified pnv_php module
and some testing on pSeries with slot power control using a modified rpaphp
module, and the EEH-related parts seem to work.

Cheers,
Sam.

Sam Bobroff (8):
  powerpc/64: Adjust order in pcibios_init()
  powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
  powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag
  powerpc/eeh: Improve debug messages around device addition
  powerpc/eeh: Add eeh_show_enabled()
  powerpc/eeh: Initialize EEH address cache earlier
  powerpc/eeh: EEH for pSeries hot plug
  powerpc/eeh: Remove eeh_probe_devices() and eeh_addr_cache_build()

 arch/powerpc/include/asm/eeh.h               | 19 +++--
 arch/powerpc/kernel/eeh.c                    | 33 ++++-----
 arch/powerpc/kernel/eeh_cache.c              | 29 +-------
 arch/powerpc/kernel/eeh_driver.c             |  4 ++
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/kernel/pci-common.c             |  4 --
 arch/powerpc/kernel/pci_32.c                 |  4 ++
 arch/powerpc/kernel/pci_64.c                 | 12 +++-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 41 +++++------
 arch/powerpc/platforms/powernv/pci.c         |  7 +-
 arch/powerpc/platforms/powernv/pci.h         |  2 -
 arch/powerpc/platforms/pseries/eeh_pseries.c | 75 +++++++++++---------
 arch/powerpc/platforms/pseries/pci.c         |  7 +-
 13 files changed, 122 insertions(+), 118 deletions(-)

-- 
2.19.0.2.gcad72f5712


^ permalink raw reply

* [PATCH 3/8] powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on
each PHB once the EEH subsystem is ready. It is the only use of the
flags member of the phb struct.

However there is no need to store this separately on each PHB, so
convert it to a global flag. For symmetry, the flag is now also set
for pSeries; although it is currently unused it may be useful in the
future.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               | 11 +++++++++++
 arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++-----------
 arch/powerpc/platforms/powernv/pci.c         |  7 +++----
 arch/powerpc/platforms/powernv/pci.h         |  2 --
 arch/powerpc/platforms/pseries/pci.c         |  4 ++++
 5 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 3613a56281f2..fe4cf7208890 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -43,6 +43,7 @@ struct pci_dn;
 #define EEH_VALID_PE_ZERO	0x10	/* PE#0 is valid		     */
 #define EEH_ENABLE_IO_FOR_LOG	0x20	/* Enable IO for log		     */
 #define EEH_EARLY_DUMP_LOG	0x40	/* Dump log immediately		     */
+#define EEH_PHB_ENABLED		0x80	/* PHB recovery uses EEH	     */
 
 /*
  * Delay for PE reset, all in ms
@@ -245,6 +246,11 @@ static inline bool eeh_enabled(void)
 	return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED);
 }
 
+static inline bool eeh_phb_enabled(void)
+{
+	return eeh_has_flag(EEH_PHB_ENABLED);
+}
+
 static inline void eeh_serialize_lock(unsigned long *flags)
 {
 	raw_spin_lock_irqsave(&confirm_error_lock, *flags);
@@ -332,6 +338,11 @@ static inline bool eeh_enabled(void)
         return false;
 }
 
+static inline bool eeh_phb_enabled(void)
+{
+	return false;
+}
+
 static inline void eeh_probe_devices(void) { }
 
 static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 6fc1a463b796..f0a95f663810 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -264,22 +264,14 @@ int pnv_eeh_post_init(void)
 		return ret;
 	}
 
-	if (!eeh_enabled())
+	if (eeh_enabled())
+		eeh_add_flag(EEH_PHB_ENABLED);
+	else
 		disable_irq(eeh_event_irq);
 
 	list_for_each_entry(hose, &hose_list, list_node) {
 		phb = hose->private_data;
 
-		/*
-		 * If EEH is enabled, we're going to rely on that.
-		 * Otherwise, we restore to conventional mechanism
-		 * to clear frozen PE during PCI config access.
-		 */
-		if (eeh_enabled())
-			phb->flags |= PNV_PHB_FLAG_EEH;
-		else
-			phb->flags &= ~PNV_PHB_FLAG_EEH;
-
 		/* Create debugfs entries */
 #ifdef CONFIG_DEBUG_FS
 		if (phb->has_dbgfs || !phb->dbgfs)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 307181fd8a17..d2b50f3bf6b1 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = NULL;
-	struct pnv_phb *phb = pdn->phb->private_data;
 
 	/* EEH not enabled ? */
-	if (!(phb->flags & PNV_PHB_FLAG_EEH))
+	if (!eeh_phb_enabled())
 		return true;
 
 	/* PE reset or device removed ? */
@@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 
 	ret = pnv_pci_cfg_read(pdn, where, size, val);
 	phb = pdn->phb->private_data;
-	if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) {
+	if (eeh_phb_enabled() && pdn->edev) {
 		if (*val == EEH_IO_ERROR_VALUE(size) &&
 		    eeh_dev_check_failure(pdn->edev))
                         return PCIBIOS_DEVICE_NOT_FOUND;
@@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus,
 
 	ret = pnv_pci_cfg_write(pdn, where, size, val);
 	phb = pdn->phb->private_data;
-	if (!(phb->flags & PNV_PHB_FLAG_EEH))
+	if (!eeh_phb_enabled())
 		pnv_pci_config_check_eeh(pdn);
 
 	return ret;
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 8e36da379252..eb0add61397b 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -85,8 +85,6 @@ struct pnv_ioda_pe {
 	struct list_head	list;
 };
 
-#define PNV_PHB_FLAG_EEH	(1 << 0)
-
 struct pnv_phb {
 	struct pci_controller	*hose;
 	enum pnv_phb_type	type;
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 37a77e57893e..7be80882c08d 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void)
 
 	eeh_probe_devices();
 	eeh_addr_cache_build();
+#ifdef CONFIG_EEH
+	if (eeh_enabled())
+		eeh_add_flag(EEH_PHB_ENABLED);
+#endif
 
 #ifdef CONFIG_PCI_IOV
 	ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 5/8] powerpc/eeh: Add eeh_show_enabled()
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

Move the EEH enabled message into it's own function so that future
work can call it from multiple places.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/include/asm/eeh.h |  3 +++
 arch/powerpc/kernel/eeh.c      | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index fe4cf7208890..e217ccda55d0 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -289,6 +289,7 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
 
 struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
 void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
+void eeh_show_enabled(void);
 void eeh_probe_devices(void);
 int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
@@ -338,6 +339,8 @@ static inline bool eeh_enabled(void)
         return false;
 }
 
+static inline void eeh_show_enabled(void) { }
+
 static inline bool eeh_phb_enabled(void)
 {
 	return false;
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index b14d89547895..3dcff29cb9b3 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
 }
 __setup("eeh=", eeh_setup);
 
+void eeh_show_enabled(void)
+{
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
+		pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (by eeh=off)\n");
+	else if (eeh_enabled())
+		pr_info("EEH: PCI Enhanced I/O Error Handling ENABLED (capable adapter found)\n");
+	else
+		pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (no capable adapter found)\n");
+}
+
 /*
  * This routine captures assorted PCI configuration space data
  * for the indicated PCI device, and puts them into a buffer
@@ -1166,11 +1176,7 @@ void eeh_probe_devices(void)
 		pdn = hose->pci_data;
 		traverse_pci_dn(pdn, eeh_ops->probe, NULL);
 	}
-	if (eeh_enabled())
-		pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
-	else
-		pr_info("EEH: No capable adapters found\n");
-
+	eeh_show_enabled();
 }
 
 /**
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 6/8] powerpc/eeh: Initialize EEH address cache earlier
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

The EEH address cache is currently initialized and populated by a
single function: eeh_addr_cache_build().  While the initial population
of the cache can only be done once resources are allocated,
initialization (just setting up a spinlock) could be done much
earlier.

So move the initialization step into a separate function and call it
from a core_initcall (rather than a subsys initcall).

This will allow future work to make use of the cache during boot time
PCI scanning.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/include/asm/eeh.h  |  3 +++
 arch/powerpc/kernel/eeh.c       |  2 ++
 arch/powerpc/kernel/eeh_cache.c | 13 +++++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index e217ccda55d0..791b9e6fcc45 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -295,6 +295,7 @@ int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
+void eeh_addr_cache_init(void);
 void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);
@@ -362,6 +363,8 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 #define eeh_dev_check_failure(x) (0)
 
+static inline void eeh_addr_cache_init(void) { }
+
 static inline void eeh_addr_cache_build(void) { }
 
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 3dcff29cb9b3..7a406d58d2c0 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1219,6 +1219,8 @@ static int eeh_init(void)
 	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		eeh_dev_phb_init_dynamic(hose);
 
+	eeh_addr_cache_init();
+
 	/* Initialize EEH event */
 	return eeh_event_init();
 }
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index 9c68f0837385..f93dd5cf6a39 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -267,6 +267,17 @@ void eeh_addr_cache_rmv_dev(struct pci_dev *dev)
 	spin_unlock_irqrestore(&pci_io_addr_cache_root.piar_lock, flags);
 }
 
+/**
+ * eeh_addr_cache_init - Initialize a cache of I/O addresses
+ *
+ * Initialize a cache of pci i/o addresses.  This cache will be used to
+ * find the pci device that corresponds to a given address.
+ */
+void eeh_addr_cache_init(void)
+{
+	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
+}
+
 /**
  * eeh_addr_cache_build - Build a cache of I/O addresses
  *
@@ -282,8 +293,6 @@ void eeh_addr_cache_build(void)
 	struct eeh_dev *edev;
 	struct pci_dev *dev = NULL;
 
-	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
-
 	for_each_pci_dev(dev) {
 		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 		if (!pdn)
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* [PATCH 1/8] powerpc/64: Adjust order in pcibios_init()
From: Sam Bobroff @ 2019-03-20  2:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1553050609.git.sbobroff@linux.ibm.com>

The pcibios_init() function for 64 bit PowerPC currently calls
pci_bus_add_devices() before pcibios_resource_survey(), which seems
incorrect because it adds devices and attempts to bind their drivers
before allocating their resources (although no problems seem to be
apparent).

So move the call to pci_bus_add_devices() to after
pcibios_resource_survey(), while extracting call to the
pcibios_fixup() hook so that it remains in the same location.

This will also allow the ppc_md.pcibios_bus_add_device() hooks to
perform actions that depend on PCI resources, both during rescanning
(where this is already the case) and at boot time, to support future
work.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/kernel/pci-common.c |  4 ----
 arch/powerpc/kernel/pci_32.c     |  4 ++++
 arch/powerpc/kernel/pci_64.c     | 12 +++++++++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ff4b7539cbdf..3146eb73e3b3 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1383,10 +1383,6 @@ void __init pcibios_resource_survey(void)
 		pr_debug("PCI: Assigning unassigned resources...\n");
 		pci_assign_unassigned_resources();
 	}
-
-	/* Call machine dependent fixup */
-	if (ppc_md.pcibios_fixup)
-		ppc_md.pcibios_fixup();
 }
 
 /* This is used by the PCI hotplug driver to allocate resource
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index d3f04f2d8249..40aaa1a6e193 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -259,6 +259,10 @@ static int __init pcibios_init(void)
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	/* Call machine dependent post-init code */
 	if (ppc_md.pcibios_after_init)
 		ppc_md.pcibios_after_init();
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 9d8c10d55407..6f16f30031d7 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -58,14 +58,20 @@ static int __init pcibios_init(void)
 	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
 
 	/* Scan all of the recorded PCI controllers.  */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		pcibios_scan_phb(hose);
-		pci_bus_add_devices(hose->bus);
-	}
 
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Add devices. */
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
+		pci_bus_add_devices(hose->bus);
+
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
 
 	return 0;
-- 
2.19.0.2.gcad72f5712


^ permalink raw reply related

* Re: [PATCH kernel RFC 2/2] vfio-pci-nvlink2: Implement interconnect isolation
From: Alexey Kardashevskiy @ 2019-03-20  1:57 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Jose Ricardo Ziviani, Daniel Henrique Barboza, kvm-ppc,
	Piotr Jaroszynski, Leonardo Augusto Guimarães Garcia,
	linuxppc-dev, David Gibson
In-Reply-To: <20190319103619.6534c7df@x1.home>



On 20/03/2019 03:36, Alex Williamson wrote:
> On Fri, 15 Mar 2019 19:18:35 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> The NVIDIA V100 SXM2 GPUs are connected to the CPU via PCIe links and
>> (on POWER9) NVLinks. In addition to that, GPUs themselves have direct
>> peer to peer NVLinks in groups of 2 to 4 GPUs. At the moment the POWERNV
>> platform puts all interconnected GPUs to the same IOMMU group.
>>
>> However the user may want to pass individual GPUs to the userspace so
>> in order to do so we need to put them into separate IOMMU groups and
>> cut off the interconnects.
>>
>> Thankfully V100 GPUs implement an interface to do by programming link
>> disabling mask to BAR0 of a GPU. Once a link is disabled in a GPU using
>> this interface, it cannot be re-enabled until the secondary bus reset is
>> issued to the GPU.
>>
>> This defines a reset_done() handler for V100 NVlink2 device which
>> determines what links need to be disabled. This relies on presence
>> of the new "ibm,nvlink-peers" device tree property of a GPU telling which
>> PCI peers it is connected to (which includes NVLink bridges or peer GPUs).
>>
>> This does not change the existing behaviour and instead adds
>> a new "isolate_nvlink" kernel parameter to allow such isolation.
>>
>> The alternative approaches would be:
>>
>> 1. do this in the system firmware (skiboot) but for that we would need
>> to tell skiboot via an additional OPAL call whether or not we want this
>> isolation - skiboot is unaware of IOMMU groups.
>>
>> 2. do this in the secondary bus reset handler in the POWERNV platform -
>> the problem with that is at that point the device is not enabled, i.e.
>> config space is not restored so we need to enable the device (i.e. MMIO
>> bit in CMD register + program valid address to BAR0) in order to disable
>> links and then perhaps undo all this initialization to bring the device
>> back to the state where pci_try_reset_function() expects it to be.
> 
> The trouble seems to be that this approach only maintains the isolation
> exposed by the IOMMU group when vfio-pci is the active driver for the
> device.  IOMMU groups can be used by any driver and the IOMMU core is
> incorporating groups in various ways.  So, if there's a device specific
> way to configure the isolation reported in the group, which requires
> some sort of active management against things like secondary bus
> resets, then I think we need to manage it above the attached endpoint
> driver.

Fair point. So for now I'll go for 2) then.

> Ideally I'd see this as a set of PCI quirks so that we might
> leverage it beyond POWER platforms.  I'm not sure how we get past the
> reliance on device tree properties that we won't have on other
> platforms though, if only NVIDIA could at least open a spec addressing
> the discovery and configuration of NVLink registers on their
> devices :-\  Thanks,

This would be nice, yes...


-- 
Alexey

^ permalink raw reply

* Re: [PATCH 6/7] ocxl: afu_irq only deals with IRQ IDs, not offsets
From: Alastair D'Silva @ 2019-03-20  0:28 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <20190315145639.4ae9e257@bahia.lab.toulouse-stg.fr.ibm.com>

On Fri, 2019-03-15 at 14:56 +0100, Greg Kurz wrote:
> On Wed, 13 Mar 2019 15:15:21 +1100
> "Alastair D'Silva" <alastair@au1.ibm.com> wrote:
> 
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > The use of offsets is required only in the frontend, so alter
> > the IRQ API to only work with IRQ IDs in the backend.
> > 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >  drivers/misc/ocxl/afu_irq.c       | 31 +++++++++++++------------
> > ------
> >  drivers/misc/ocxl/context.c       |  7 +++++--
> >  drivers/misc/ocxl/file.c          | 13 ++++++++-----
> >  drivers/misc/ocxl/ocxl_internal.h | 10 ++++++----
> >  drivers/misc/ocxl/trace.h         | 12 ++++--------
> >  5 files changed, 36 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/misc/ocxl/afu_irq.c
> > b/drivers/misc/ocxl/afu_irq.c
> > index 11ab996657a2..1885c472df58 100644
> > --- a/drivers/misc/ocxl/afu_irq.c
> > +++ b/drivers/misc/ocxl/afu_irq.c
> > @@ -14,14 +14,14 @@ struct afu_irq {
> >  	struct eventfd_ctx *ev_ctx;
> >  };
> >  
> > -static int irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
> > +int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
> >  {
> >  	return (offset - ctx->afu->irq_base_offset) >> PAGE_SHIFT;
> >  }
> >  
> > -static u64 irq_id_to_offset(struct ocxl_context *ctx, int id)
> > +u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
> >  {
> > -	return ctx->afu->irq_base_offset + (id << PAGE_SHIFT);
> > +	return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
> >  }
> >  
> >  static irqreturn_t afu_irq_handler(int virq, void *data)
> > @@ -69,7 +69,7 @@ static void release_afu_irq(struct afu_irq *irq)
> >  	kfree(irq->name);
> >  }
> >  
> > -int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
> > +int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
> >  {
> >  	struct afu_irq *irq;
> >  	int rc;
> > @@ -101,10 +101,7 @@ int ocxl_afu_irq_alloc(struct ocxl_context
> > *ctx, u64 *irq_offset)
> >  	if (rc)
> >  		goto err_alloc;
> >  
> > -	*irq_offset = irq_id_to_offset(ctx, irq->id);
> 
> This should be replaced by:
> 
> 	*irq_id = irq->id;
> 

Whoops, good catch, thanks :)

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* [PATCH 4/4] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2019-03-19 23:24 UTC (permalink / raw)
  To: Andrew Morton, Christophe Leroy, Michael Ellerman, Rob Herring,
	xe-linux-external, linuxppc-dev
  Cc: Daniel Walker, Paul Mackerras, linux-kernel

This is a mass convert of the config files to use the new
generic cmdline.

The command used to convert is as follows,

sed -i 's/CONFIG_CMDLINE=/CONFIG_CMDLINE_PREPEND=/g' <config>

Cc: xe-linux-external@cisco.com
Cc: Daniel Walker <dwalker@fifo99.com>
Signed-off-by: Daniel Walker <danielwa@cisco.com>

Change-Id: Idf7cae45ef5f8afebcb1ee7e025aafcb6541ad35
Signed-off-by: Daniel Walker <danielwa@cisco.com>
---
 arch/powerpc/configs/44x/fsp2_defconfig       | 2 +-
 arch/powerpc/configs/44x/iss476-smp_defconfig | 2 +-
 arch/powerpc/configs/44x/warp_defconfig       | 2 +-
 arch/powerpc/configs/holly_defconfig          | 2 +-
 arch/powerpc/configs/mvme5100_defconfig       | 2 +-
 arch/powerpc/configs/skiroot_defconfig        | 2 +-
 arch/powerpc/configs/storcenter_defconfig     | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/configs/44x/fsp2_defconfig b/arch/powerpc/configs/44x/fsp2_defconfig
index bae6b26bcfba..1e9b1e7e281f 100644
--- a/arch/powerpc/configs/44x/fsp2_defconfig
+++ b/arch/powerpc/configs/44x/fsp2_defconfig
@@ -29,7 +29,7 @@ CONFIG_SWIOTLB=y
 CONFIG_KEXEC=y
 CONFIG_CRASH_DUMP=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="ip=on rw"
+CONFIG_CMDLINE_PREPEND="ip=on rw"
 # CONFIG_SUSPEND is not set
 # CONFIG_PCI is not set
 CONFIG_NET=y
diff --git a/arch/powerpc/configs/44x/iss476-smp_defconfig b/arch/powerpc/configs/44x/iss476-smp_defconfig
index d24bfa6ecd62..ed234c2b1956 100644
--- a/arch/powerpc/configs/44x/iss476-smp_defconfig
+++ b/arch/powerpc/configs/44x/iss476-smp_defconfig
@@ -18,7 +18,7 @@ CONFIG_HZ_100=y
 CONFIG_MATH_EMULATION=y
 CONFIG_IRQ_ALL_CPUS=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="root=/dev/issblk0"
+CONFIG_CMDLINE_PREPEND="root=/dev/issblk0"
 # CONFIG_PCI is not set
 CONFIG_ADVANCED_OPTIONS=y
 CONFIG_DYNAMIC_MEMSTART=y
diff --git a/arch/powerpc/configs/44x/warp_defconfig b/arch/powerpc/configs/44x/warp_defconfig
index 6c02f53271cd..ddb395840bb9 100644
--- a/arch/powerpc/configs/44x/warp_defconfig
+++ b/arch/powerpc/configs/44x/warp_defconfig
@@ -15,7 +15,7 @@ CONFIG_WARP=y
 CONFIG_PPC4xx_GPIO=y
 CONFIG_HZ_1000=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="ip=on"
+CONFIG_CMDLINE_PREPEND="ip=on"
 # CONFIG_PCI is not set
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/holly_defconfig b/arch/powerpc/configs/holly_defconfig
index 71d8d2430b6c..14945562d193 100644
--- a/arch/powerpc/configs/holly_defconfig
+++ b/arch/powerpc/configs/holly_defconfig
@@ -14,7 +14,7 @@ CONFIG_PPC_HOLLY=y
 CONFIG_GEN_RTC=y
 CONFIG_BINFMT_MISC=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="console=ttyS0,115200"
+CONFIG_CMDLINE_PREPEND="console=ttyS0,115200"
 # CONFIG_SECCOMP is not set
 CONFIG_NET=y
 CONFIG_PACKET=y
diff --git a/arch/powerpc/configs/mvme5100_defconfig b/arch/powerpc/configs/mvme5100_defconfig
index 63e38c7220f1..07a68ebb3713 100644
--- a/arch/powerpc/configs/mvme5100_defconfig
+++ b/arch/powerpc/configs/mvme5100_defconfig
@@ -23,7 +23,7 @@ CONFIG_HZ_100=y
 # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
 # CONFIG_COMPACTION is not set
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="console=ttyS0,9600 ip=dhcp root=/dev/nfs"
+CONFIG_CMDLINE_PREPEND="console=ttyS0,9600 ip=dhcp root=/dev/nfs"
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_UNIX=y
diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index cfdd08897a06..a79984208224 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -53,7 +53,7 @@ CONFIG_NUMA=y
 CONFIG_PPC_64K_PAGES=y
 CONFIG_SCHED_SMT=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="console=tty0 console=hvc0 ipr.fast_reboot=1 quiet"
+CONFIG_CMDLINE_PREPEND="console=tty0 console=hvc0 ipr.fast_reboot=1 quiet"
 # CONFIG_SECCOMP is not set
 # CONFIG_PPC_MEM_KEYS is not set
 CONFIG_NET=y
diff --git a/arch/powerpc/configs/storcenter_defconfig b/arch/powerpc/configs/storcenter_defconfig
index 74bca2eccd0f..83b3b92176a0 100644
--- a/arch/powerpc/configs/storcenter_defconfig
+++ b/arch/powerpc/configs/storcenter_defconfig
@@ -13,7 +13,7 @@ CONFIG_STORCENTER=y
 CONFIG_HZ_100=y
 CONFIG_BINFMT_MISC=y
 CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="console=ttyS0,115200"
+CONFIG_CMDLINE_PREPEND="console=ttyS0,115200"
 # CONFIG_SECCOMP is not set
 CONFIG_NET=y
 CONFIG_PACKET=m
-- 
2.19.1


^ permalink raw reply related

* [PATCH 1/4] add generic builtin command line
From: Daniel Walker @ 2019-03-19 23:24 UTC (permalink / raw)
  To: Andrew Morton, Christophe Leroy, Michael Ellerman, Rob Herring,
	xe-linux-external, linuxppc-dev
  Cc: Daniel Walker, linux-kernel, Maksym Kokhan

This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.

The code in this commit unifies the mips and x86 code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.

[maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 include/linux/cmdline.h | 69 +++++++++++++++++++++++++++++++++++++++++
 init/Kconfig            | 69 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+)
 create mode 100644 include/linux/cmdline.h

diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index 000000000000..4a16ee134585
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2015. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/*
+ * This function will append or prepend a builtin command line to the command
+ * line provided by the bootloader. Kconfig options can be used to alter
+ * the behavior of this builtin command line.
+ * @dest: The destination of the final appended/prepended string
+ * @src: The starting string or NULL if there isn't one.
+ * @tmp: temporary space used for prepending
+ * @length: the maximum length of the strings above.
+ */
+static inline void
+_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
+{
+	if (src != dest && src != NULL) {
+		strlcpy(dest, " ", length);
+		strlcat(dest, src, length);
+	}
+
+	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
+		strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
+
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
+		strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
+		strlcat(tmp, dest, length);
+		strlcpy(dest, tmp, length);
+	}
+}
+
+#define cmdline_add_builtin_section(dest, src, length, section) 	    \
+{									    \
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
+		static char cmdline_tmp_space[length] section;	            \
+		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
+	} else {							    \
+		_cmdline_add_builtin(dest, src, NULL, length);		    \
+	}								    \
+}
+#else
+#define cmdline_add_builtin_section(dest, src, length, section) 	   \
+{									   \
+	strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
+		length);						   \
+}
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+
+#else
+#define cmdline_add_builtin_section(dest, src, length, section) {          \
+	if (src != NULL)						   \
+		strlcpy(dest, src, length);				   \
+}
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+#define cmdline_add_builtin(dest, src, length) \
+	cmdline_add_builtin_section(dest, src, length, __initdata)
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index d47cb77a220e..b9b9e7702ea3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1778,6 +1778,75 @@ config PROFILING
 config TRACEPOINTS
 	bool
 
+config GENERIC_CMDLINE
+	bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+	bool "Built-in kernel command line"
+	help
+	  Allow for specifying boot arguments to the kernel at
+	  build time.  On some systems (e.g. embedded ones), it is
+	  necessary or convenient to provide some or all of the
+	  kernel boot arguments with the kernel itself (that is,
+	  to not rely on the boot loader to provide them.)
+
+	  To compile command line arguments into the kernel,
+	  set this option to 'Y', then fill in the
+	  the boot arguments in CONFIG_CMDLINE.
+
+	  Systems with fully functional boot loaders (i.e. non-embedded)
+	  should leave this option set to 'N'.
+
+config CMDLINE_APPEND
+	string "Built-in kernel command string append"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is appended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_PREPEND
+	string "Built-in kernel command string prepend"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is prepended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_OVERRIDE
+	bool "Built-in command line overrides boot loader arguments"
+	depends on CMDLINE_BOOL
+	help
+	  Set this option to 'Y' to have the kernel ignore the boot loader
+	  command line, and use ONLY the built-in command line. In this case
+	  append and prepend strings are concatenated to form the full
+	  command line.
+
+	  This is used to work around broken boot loaders.  This should
+	  be set to 'N' under normal conditions.
+
+endif
+
 endmenu		# General setup
 
 source "arch/Kconfig"
-- 
2.19.1


^ permalink raw reply related

* [PATCH 2/4] drivers: of: generic command line support
From: Daniel Walker @ 2019-03-19 23:24 UTC (permalink / raw)
  To: Andrew Morton, Christophe Leroy, Michael Ellerman, Rob Herring,
	xe-linux-external, linuxppc-dev, Frank Rowand
  Cc: devicetree, linux-kernel

This adds support for the generic command line implementation into the
device tree code. This allows some platforms to use the original
CONFIG_CMDLINE implementation, but powerpc platforms can used the newer
generic command line code. As platforms support the generic command line
code they can simply add "select GENERIC_CMDLINE" and delete their
Kconfig options for the current CMDLINE.

Change-Id: Ief473a5ffac01a999b0aba7619f5b63bc4b36ac4
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Daniel Walker <danielwa@cisco.com>
---
 drivers/of/fdt.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 7099c652c6a5..9dc5550697c2 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -24,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/serial_core.h>
 #include <linux/sysfs.h>
+#include <linux/cmdline.h>
 
 #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
 #include <asm/page.h>
@@ -1102,7 +1103,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
 	 * is set in which case we override whatever was found earlier.
 	 */
-#ifdef CONFIG_CMDLINE
+#if defined(CONFIG_CMDLINE) && !defined(CONFIG_GENERIC_CMDLINE)
 #if defined(CONFIG_CMDLINE_EXTEND)
 	strlcat(data, " ", COMMAND_LINE_SIZE);
 	strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
@@ -1113,7 +1114,12 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	if (!((char *)data)[0])
 		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif
-#endif /* CONFIG_CMDLINE */
+#endif /* CONFIG_CMDLINE  && !CONFIG_GENERIC_CMDLINE */
+
+	/* append and prepend any arguments built into the kernel via
+	 * generic cmdline.
+	 */
+	cmdline_add_builtin(data, NULL, COMMAND_LINE_SIZE);
 
 	pr_debug("Command line is: %s\n", (char*)data);
 
-- 
2.19.1


^ permalink raw reply related

* [PATCH 3/4] powerpc: convert to generic builtin command line
From: Daniel Walker @ 2019-03-19 23:24 UTC (permalink / raw)
  To: Andrew Morton, Christophe Leroy, Michael Ellerman, Rob Herring,
	xe-linux-external, linuxppc-dev, Benjamin Herrenschmidt,
	Paul Mackerras
  Cc: Daniel Walker, linux-kernel, Maksym Kokhan

This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
option.

[maksym.kokhan@globallogic.com: add strlcat to prom_init_check.sh
whitelist]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 arch/powerpc/Kconfig                   | 23 +----------------------
 arch/powerpc/kernel/prom_init.c        |  8 ++++----
 arch/powerpc/kernel/prom_init_check.sh |  2 +-
 3 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index def41a06377b..385120ff0236 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -173,6 +173,7 @@ config PPC
 	select GENERIC_STRNCPY_FROM_USER
 	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
+	select GENERIC_CMDLINE
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KGDB
@@ -780,28 +781,6 @@ config PPC_DENORMALISATION
 	  Add support for handling denormalisation of single precision
 	  values.  Useful for bare metal only.  If unsure say Y here.
 
-config CMDLINE_BOOL
-	bool "Default bootloader kernel arguments"
-
-config CMDLINE
-	string "Initial kernel command string"
-	depends on CMDLINE_BOOL
-	default "console=ttyS0,9600 console=tty0 root=/dev/sda2"
-	help
-	  On some platforms, there is currently no way for the boot loader to
-	  pass arguments to the kernel. For these platforms, you can supply
-	  some command-line options at build time by entering them here.  In
-	  most cases you will need to specify the root device here.
-
-config CMDLINE_FORCE
-	bool "Always use the default kernel command string"
-	depends on CMDLINE_BOOL
-	help
-	  Always use the default kernel command string, even if the boot
-	  loader passes other arguments to the kernel.
-	  This is useful if you cannot or don't want to change the
-	  command-line options your boot loader passes to the kernel.
-
 config EXTRA_TARGETS
 	string "Additional default image types"
 	help
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f33ff4163a51..e8e9fca22470 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -30,6 +30,7 @@
 #include <linux/delay.h>
 #include <linux/initrd.h>
 #include <linux/bitops.h>
+#include <linux/cmdline.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/page.h>
@@ -637,11 +638,10 @@ static void __init early_cmdline_parse(void)
 	p = prom_cmd_line;
 	if ((long)prom.chosen > 0)
 		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
-#ifdef CONFIG_CMDLINE
+
 	if (l <= 0 || p[0] == '\0') /* dbl check */
-		strlcpy(prom_cmd_line,
-			CONFIG_CMDLINE, sizeof(prom_cmd_line));
-#endif /* CONFIG_CMDLINE */
+		cmdline_add_builtin_section(prom_cmd_line, NULL, sizeof(prom_cmd_line), __prombss);
+
 	prom_printf("command line: %s\n", prom_cmd_line);
 
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 667df97d2595..ab2acc8d8b5a 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -18,7 +18,7 @@
 
 WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
 _end enter_prom memcpy memset reloc_offset __secondary_hold
-__secondary_hold_acknowledge __secondary_hold_spinloop __start
+__secondary_hold_acknowledge __secondary_hold_spinloop __start strlcat
 strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
 reloc_got2 kernstart_addr memstart_addr linux_banner _stext
 __prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
-- 
2.19.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox