LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 06/16] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: David Gibson @ 2019-03-13  4:03 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <9e732140-b2c0-dfb7-d753-ba0ec7f3b930@kaod.org>

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

On Tue, Mar 12, 2019 at 06:00:38PM +0100, Cédric Le Goater wrote:
> On 2/25/19 3:39 AM, David Gibson wrote:
> > On Fri, Feb 22, 2019 at 12:28:30PM +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. They will also be used to
> >> restore the configuration of the XIVE EQs in the KVM device and to
> >> capture the internal runtime state of the EQs. Both 'get' and 'set'
> >> rely on an OPAL call to access from the XIVE interrupt controller the
> >> EQ toggle bit and EQ index which are updated by the HW 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.
> 
> I think we need to add some kind of flags to differentiate the hcall
> H_INT_SET_QUEUE_CONFIG from the restore of the EQ. The hcall does
> not need OPAL support call and this could help in the code
> transition.

Hrm.  What's the actual difference in the semantics between the two
cases.  The guest shouldn't have awareness of whether or not OPAL is
involved.

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

* Re: [PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE
From: David Gibson @ 2019-03-13  4:05 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <2f029721-0b20-2706-6627-0bac36afd03e@kaod.org>

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

On Tue, Mar 12, 2019 at 03:03:25PM +0100, Cédric Le Goater wrote:
> On 2/25/19 1:35 AM, David Gibson wrote:
> > On Fri, Feb 22, 2019 at 12:28:27PM +0100, Cédric Le Goater wrote:
[snip]
> >> +int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> >> +				    struct kvm_vcpu *vcpu, u32 cpu)
> >> +{
> >> +	struct kvmppc_xive *xive = dev->private;
> >> +	struct kvmppc_xive_vcpu *xc;
> >> +	int rc;
> >> +
> >> +	pr_devel("native_connect_vcpu(cpu=%d)\n", cpu);
> >> +
> >> +	if (dev->ops != &kvm_xive_native_ops) {
> >> +		pr_devel("Wrong ops !\n");
> >> +		return -EPERM;
> >> +	}
> >> +	if (xive->kvm != vcpu->kvm)
> >> +		return -EPERM;
> >> +	if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT)
> >> +		return -EBUSY;
> >> +	if (kvmppc_xive_find_server(vcpu->kvm, cpu)) {
> > 
> > You haven't taken the kvm->lock yet, so couldn't a race mean a
> > duplicate server gets inserted after you make this check?
> > 
> >> +		pr_devel("Duplicate !\n");
> >> +		return -EEXIST;
> >> +	}
> >> +	if (cpu >= KVM_MAX_VCPUS) {
> >> +		pr_devel("Out of bounds !\n");
> >> +		return -EINVAL;
> >> +	}
> >> +	xc = kzalloc(sizeof(*xc), GFP_KERNEL);
> >> +	if (!xc)
> >> +		return -ENOMEM;
> >> +
> >> +	mutex_lock(&vcpu->kvm->lock);
> >> +	vcpu->arch.xive_vcpu = xc;
> > 
> > Similarly you don't verify this is NULL after taking the lock, so
> > couldn't another thread race and make a connect which gets clobbered
> > here?
> 
> Yes. this is not very safe ... We need to clean up all the KVM device 
> methods doing the connection of the presenter to the vCPU AFAICT. 
> I will fix the XIVE native one for now. 
> 
> And also, this CPU parameter is useless. There is no reason to connect 
> a vCPU from another vCPU.

Hmm.. I thought the point of the 'cpu' parameter (not a great name) is
that it lets userspace chose the guest visible irq server ID.  I think
that's preferable to tying it to an existing cpu id, if possible.

-- 
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/7] ocxl: move event_fd handling to frontend
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-1-alastair@au1.ibm.com>

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

Event_fd is only used in the driver frontend, so it does not
need to exist in the backend code. Relocate it to the frontend
and provide an opaque mechanism for consumers instead.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/afu_irq.c       | 69 +++++++++++++++++--------------
 drivers/misc/ocxl/file.c          | 22 +++++++++-
 drivers/misc/ocxl/ocxl_internal.h |  5 ---
 include/misc/ocxl.h               | 44 ++++++++++++++++++++
 4 files changed, 102 insertions(+), 38 deletions(-)

diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index 1885c472df58..bccb89085a29 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 // Copyright 2017 IBM Corp.
 #include <linux/interrupt.h>
-#include <linux/eventfd.h>
+#include <asm/pnv-ocxl.h>
 #include "ocxl_internal.h"
 #include "trace.h"
 
@@ -11,7 +11,9 @@ struct afu_irq {
 	unsigned int virq;
 	char *name;
 	u64 trigger_page;
-	struct eventfd_ctx *ev_ctx;
+	irqreturn_t (*handler)(void *private);
+	void (*free_private)(void *private);
+	void *private;
 };
 
 int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
@@ -24,14 +26,42 @@ u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
 	return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
 }
 
+int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
+		irqreturn_t (*handler)(void *private),
+		void (*free_private)(void *private),
+		void *private)
+{
+	struct afu_irq *irq;
+	int rc;
+
+	mutex_lock(&ctx->irq_lock);
+	irq = idr_find(&ctx->irq_idr, irq_id);
+	if (!irq) {
+		rc = -EINVAL;
+		goto unlock;
+	}
+
+	irq->handler = handler;
+	irq->private = private;
+
+	rc = 0;
+	goto unlock;
+
+unlock:
+	mutex_unlock(&ctx->irq_lock);
+	return rc;
+}
+
 static irqreturn_t afu_irq_handler(int virq, void *data)
 {
 	struct afu_irq *irq = (struct afu_irq *) data;
 
 	trace_ocxl_afu_irq_receive(virq);
-	if (irq->ev_ctx)
-		eventfd_signal(irq->ev_ctx, 1);
-	return IRQ_HANDLED;
+
+	if (irq->handler)
+		return irq->handler(irq->private);
+
+	return IRQ_HANDLED; // Just drop it on the ground
 }
 
 static int setup_afu_irq(struct ocxl_context *ctx, struct afu_irq *irq)
@@ -123,8 +153,8 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
 				ocxl_irq_id_to_offset(ctx, irq->id),
 				1 << PAGE_SHIFT, 1);
 	release_afu_irq(irq);
-	if (irq->ev_ctx)
-		eventfd_ctx_put(irq->ev_ctx);
+	if (irq->free_private)
+		irq->free_private(irq->private);
 	ocxl_link_free_irq(ctx->afu->fn->link, irq->hw_irq);
 	kfree(irq);
 }
@@ -157,31 +187,6 @@ 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, int irq_id, int eventfd)
-{
-	struct afu_irq *irq;
-	struct eventfd_ctx *ev_ctx;
-	int rc = 0;
-
-	mutex_lock(&ctx->irq_lock);
-	irq = idr_find(&ctx->irq_idr, irq_id);
-	if (!irq) {
-		rc = -EINVAL;
-		goto unlock;
-	}
-
-	ev_ctx = eventfd_ctx_fdget(eventfd);
-	if (IS_ERR(ev_ctx)) {
-		rc = -EINVAL;
-		goto unlock;
-	}
-
-	irq->ev_ctx = ev_ctx;
-unlock:
-	mutex_unlock(&ctx->irq_lock);
-	return rc;
-}
-
 u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
 {
 	struct afu_irq *irq;
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d28618c161de..16833a5d47f0 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -3,6 +3,7 @@
 #include <linux/fs.h>
 #include <linux/poll.h>
 #include <linux/sched/signal.h>
+#include <linux/eventfd.h>
 #include <linux/uaccess.h>
 #include <uapi/misc/ocxl.h>
 #include <asm/reg.h>
@@ -192,11 +193,27 @@ static long afu_ioctl_get_features(struct ocxl_context *ctx,
 			x == OCXL_IOCTL_GET_FEATURES ? "GET_FEATURES" :	\
 			"UNKNOWN")
 
+static irqreturn_t irq_handler(void *private)
+{
+	struct eventfd_ctx *ev_ctx = private;
+
+	eventfd_signal(ev_ctx, 1);
+	return IRQ_HANDLED;
+}
+
+static void irq_free(void *private)
+{
+	struct eventfd_ctx *ev_ctx = private;
+
+	eventfd_ctx_put(ev_ctx);
+}
+
 static long afu_ioctl(struct file *file, unsigned int cmd,
 		unsigned long args)
 {
 	struct ocxl_context *ctx = file->private_data;
 	struct ocxl_ioctl_irq_fd irq_fd;
+	struct eventfd_ctx *ev_ctx;
 	int irq_id;
 	u64 irq_offset;
 	long rc;
@@ -248,7 +265,10 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
 		if (irq_fd.reserved)
 			return -EINVAL;
 		irq_id = ocxl_irq_offset_to_id(ctx, irq_fd.irq_offset);
-		rc = ocxl_afu_irq_set_fd(ctx, irq_id, irq_fd.eventfd);
+		ev_ctx = eventfd_ctx_fdget(irq_fd.eventfd);
+		if (!ev_ctx)
+			return -EFAULT;
+		rc = ocxl_irq_set_handler(ctx, irq_id, irq_handler, irq_free, ev_ctx);
 		break;
 
 	case OCXL_IOCTL_GET_METADATA:
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 871155b464da..f3775223f4b1 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -142,12 +142,7 @@ void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
 
 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, int irq_id,
-			int eventfd);
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
 
 /**
  * Free an AFU
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 54f8bafa3caa..3099c858093c 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -169,6 +169,50 @@ int ocxl_context_detach(struct ocxl_context *ctx);
 
 // OpenCAPI MMIO accesses
 
+/**
+ * Allocate an IRQ associated with an AFU context
+ * @ctx: the AFU context
+ * @irq_id: out, the IRQ ID
+ *
+ * Returns 0 on success, negative on failure
+ */
+extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
+
+/**
+ * Frees an IRQ associated with an AFU context
+ * @ctx: the AFU context
+ * @irq_id: the IRQ ID
+ *
+ * Returns 0 on success, negative on failure
+ */
+extern int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
+
+/**
+ * Gets the address of the trigger page for an IRQ
+ * This can then be provided to an AFU which will write to that
+ * page to trigger the IRQ.
+ * @ctx: The AFU context that the IRQ is associated with
+ * @irq_id: The IRQ ID
+ *
+ * returns the trigger page address, or 0 if the IRQ is not valid
+ */
+extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
+
+/**
+ * Provide a callback to be called when an IRQ is triggered
+ * @ctx: The AFU context that the IRQ is associated with
+ * @irq_id: The IRQ ID
+ * @handler: the callback to be called when the IRQ is triggered
+ * @free_private: the callback to be called when the IRQ is freed
+ * @private: Private data to be passed to the callbacks
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
+		irqreturn_t (*handler)(void *private),
+		void (*free_private)(void *private),
+		void *private);
+
 /**
  * Read a 32 bit value from global MMIO
  *
-- 
2.20.1


^ permalink raw reply related

* [PATCH 6/7] ocxl: afu_irq only deals with IRQ IDs, not offsets
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-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       | 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);
-
-	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);
 	return 0;
 
@@ -123,7 +120,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 +129,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 +157,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 +182,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 424bb0b40afb..d28618c161de 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -197,6 +197,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;
@@ -218,12 +219,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;
 			}
 		}
@@ -234,7 +236,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:
@@ -244,8 +247,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 5/7] ocxl: Create a clear delineation between ocxl backend & frontend
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-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          | 122 +++++++++++-------
 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, 411 insertions(+), 197 deletions(-)

diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 1534b56f1db1..9a37e9632cd9 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -241,7 +241,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 865b3d176431..424bb0b40afb 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;
 }
 
@@ -205,11 +199,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) {
@@ -469,39 +468,76 @@ 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);
 
-void ocxl_destroy_cdev(struct ocxl_afu *afu)
-{
-	cdev_del(&afu->cdev);
+	free_minor(info);
+	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;
 
-	minor = allocate_afu_minor(afu);
-	if (minor < 0)
+	info->afu = afu;
+
+	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;
+
+	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);
+}
+
+int ocxl_file_make_visible(struct ocxl_afu *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_unregister_afu(struct ocxl_afu *afu)
+void ocxl_file_make_invisible(struct ocxl_afu *afu)
 {
-	free_afu_minor(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 1319406ce5c6..4fc7e9597ede 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);
@@ -134,8 +137,8 @@ int ocxl_context_mmap(struct ocxl_context *ctx,
 			struct vm_area_struct *vma);
 void ocxl_context_detach_all(struct ocxl_afu *afu);
 
-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);
@@ -144,9 +147,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 ebbfe83cd5a6..54f8bafa3caa 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 */
@@ -51,6 +47,7 @@ struct ocxl_fn_config {
 
 // These are opaque outside the ocxl driver
 struct ocxl_afu;
+struct ocxl_fn;
 struct ocxl_context;
 
 enum ocxl_endian {
@@ -59,6 +56,74 @@ enum ocxl_endian {
 	OCXL_HOST_ENDIAN = 2,   /**< AFU data is the same endianness as the host */
 };
 
+// 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)
+ */
+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);
+
+// OpenCAPI contexts
 /**
  * Allocate space for a new OpenCAPI context
  *
@@ -102,6 +167,8 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
  */
 int ocxl_context_detach(struct ocxl_context *ctx);
 
+// OpenCAPI MMIO accesses
+
 /**
  * Read a 32 bit value from global MMIO
  *
@@ -206,12 +273,39 @@ int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
 int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
 				enum ocxl_endian endian, u64 mask);
 
-/*
- * Read the configuration space of a function and fill in a
- * ocxl_fn_config structure with all the function details
+// 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
  */
-int ocxl_config_read_function(struct pci_dev *dev,
-				struct ocxl_fn_config *fn);
+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
@@ -298,6 +392,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 4/7] ocxl: Don't pass pci_dev around
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-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 3/7] ocxl: Split pci.c
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-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 922e47cd4f0d..d07d1bb8e8d4 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 mmio.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 779d15ef60b5..1319406ce5c6 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -144,4 +144,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 2/7] ocxl: Allow external drivers to use OpenCAPI contexts
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
	Andrew Donnellan, Alastair D'Silva, Frederic Barrat,
	linuxppc-dev
In-Reply-To: <20190313041524.14644-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               | 44 +++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 3498a0199bde..1534b56f1db1 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 16eb8a60d5c7..865b3d176431 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -100,7 +100,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 06fd98c989c8..779d15ef60b5 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -130,15 +130,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_add_afu(struct ocxl_afu *afu);
 void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 3b320c39f0af..ebbfe83cd5a6 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -51,6 +51,7 @@ struct ocxl_fn_config {
 
 // These are opaque outside the ocxl driver
 struct ocxl_afu;
+struct ocxl_context;
 
 enum ocxl_endian {
 	OCXL_BIG_ENDIAN = 0,    /**< AFU data is big-endian */
@@ -58,6 +59,49 @@ enum ocxl_endian {
 	OCXL_HOST_ENDIAN = 2,   /**< AFU data is the same endianness as the host */
 };
 
+/**
+ * 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);
+
 /**
  * Read a 32 bit value from global MMIO
  *
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/7] ocxl: Provide global MMIO accessors for external drivers
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
	Andrew Donnellan, Alastair D'Silva, Frederic Barrat,
	linuxppc-dev
In-Reply-To: <20190313041524.14644-1-alastair@au1.ibm.com>

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

External drivers that communicate via OpenCAPI will need to make
MMIO calls to interact with the devices.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 drivers/misc/ocxl/Makefile |   2 +-
 drivers/misc/ocxl/mmio.c   | 234 +++++++++++++++++++++++++++++++++++++
 include/misc/ocxl.h        | 113 ++++++++++++++++++
 3 files changed, 348 insertions(+), 1 deletion(-)
 create mode 100644 drivers/misc/ocxl/mmio.c

diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
index 5229dcda8297..922e47cd4f0d 100644
--- a/drivers/misc/ocxl/Makefile
+++ b/drivers/misc/ocxl/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0+
 ccflags-$(CONFIG_PPC_WERROR)	+= -Werror
 
-ocxl-y				+= main.o pci.o config.o file.o pasid.o
+ocxl-y				+= main.o pci.o config.o file.o pasid.o mmio.o
 ocxl-y				+= link.o context.o afu_irq.o sysfs.o trace.o
 obj-$(CONFIG_OCXL)		+= ocxl.o
 
diff --git a/drivers/misc/ocxl/mmio.c b/drivers/misc/ocxl/mmio.c
new file mode 100644
index 000000000000..7f6ebae1c6c7
--- /dev/null
+++ b/drivers/misc/ocxl/mmio.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2017 IBM Corp.
+#include <linux/sched/mm.h>
+#include "trace.h"
+#include "ocxl_internal.h"
+
+int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 *val)
+{
+	if (offset > afu->config.global_mmio_size - 4)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		*val = readl_be((char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		*val = readl((char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_read32);
+
+int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 *val)
+{
+	if (offset > afu->config.global_mmio_size - 8)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		*val = readq_be((char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		*val = readq((char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_read64);
+
+int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 val)
+{
+	if (offset > afu->config.global_mmio_size - 4)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		writel_be(val, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		writel(val, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_write32);
+
+int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 val)
+{
+	if (offset > afu->config.global_mmio_size - 8)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		writeq_be(val, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		writeq(val, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_write64);
+
+int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 mask)
+{
+	u32 tmp;
+
+	if (offset > afu->config.global_mmio_size - 4)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		tmp = readl_be((char *)afu->global_mmio_ptr + offset);
+		tmp |= mask;
+		writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		tmp = readl((char *)afu->global_mmio_ptr + offset);
+		tmp |= mask;
+		writel(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_set32);
+
+int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 mask)
+{
+	u64 tmp;
+
+	if (offset > afu->config.global_mmio_size - 8)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		tmp = readq_be((char *)afu->global_mmio_ptr + offset);
+		tmp |= mask;
+		writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		tmp = readq((char *)afu->global_mmio_ptr + offset);
+		tmp |= mask;
+		writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_set64);
+
+int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 mask)
+{
+	u32 tmp;
+
+	if (offset > afu->config.global_mmio_size - 4)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		tmp = readl_be((char *)afu->global_mmio_ptr + offset);
+		tmp &= ~mask;
+		writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		tmp = readl((char *)afu->global_mmio_ptr + offset);
+		tmp &= ~mask;
+		writel(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear32);
+
+int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 mask)
+{
+	u64 tmp;
+
+	if (offset > afu->config.global_mmio_size - 8)
+		return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+	if (endian == OCXL_HOST_ENDIAN)
+		endian = OCXL_BIG_ENDIAN;
+#endif
+
+	switch (endian) {
+	case OCXL_BIG_ENDIAN:
+		tmp = readq_be((char *)afu->global_mmio_ptr + offset);
+		tmp &= ~mask;
+		writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+
+	default:
+		tmp = readq((char *)afu->global_mmio_ptr + offset);
+		tmp &= ~mask;
+		writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+		break;
+	}
+
+	writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear64);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 9530d3be1b30..3b320c39f0af 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -49,6 +49,119 @@ struct ocxl_fn_config {
 	s8 max_afu_index;
 };
 
+// These are opaque outside the ocxl driver
+struct ocxl_afu;
+
+enum ocxl_endian {
+	OCXL_BIG_ENDIAN = 0,    /**< AFU data is big-endian */
+	OCXL_LITTLE_ENDIAN = 1, /**< AFU data is little-endian */
+	OCXL_HOST_ENDIAN = 2,   /**< AFU data is the same endianness as the host */
+};
+
+/**
+ * Read a 32 bit value from global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: returns the value
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 *val);
+
+/**
+ * Read a 64 bit value from global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: returns the value
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 *val);
+
+/**
+ * Write a 32 bit value to global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: The value to write
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 val);
+
+/**
+ * Write a 64 bit value to global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: The value to write
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 val);
+
+/**
+ * Set bits in a 32 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 mask);
+
+/**
+ * Set bits in a 64 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 mask);
+
+/**
+ * Set bits in a 32 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u32 mask);
+
+/**
+ * Set bits in a 64 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
+				enum ocxl_endian endian, u64 mask);
+
 /*
  * Read the configuration space of a function and fill in a
  * ocxl_fn_config structure with all the function details
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/7] Refactor OCXL driver to allow external drivers to use it
From: Alastair D'Silva @ 2019-03-13  4:15 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev

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.

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

 drivers/misc/ocxl/Makefile        |   3 +-
 drivers/misc/ocxl/afu_irq.c       |  94 ++---
 drivers/misc/ocxl/context.c       |  18 +-
 drivers/misc/ocxl/core.c          | 578 ++++++++++++++++++++++++++++++
 drivers/misc/ocxl/file.c          | 157 +++++---
 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, 1382 insertions(+), 705 deletions(-)
 create mode 100644 drivers/misc/ocxl/core.c
 create mode 100644 drivers/misc/ocxl/mmio.c

-- 
2.20.1


^ permalink raw reply

* [PATCH 4/5] ocxl: Remove superfluous 'extern' from headers
From: Alastair D'Silva @ 2019-03-13  4:07 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>

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

The 'extern' keyword adds no value here.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/ocxl_internal.h | 54 +++++++++++++++----------------
 include/misc/ocxl.h               | 36 ++++++++++-----------
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index a32f2151029f..321b29e77f45 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -16,7 +16,6 @@
 
 extern struct pci_driver ocxl_pci_driver;
 
-
 struct ocxl_fn {
 	struct device dev;
 	int bar_used[3];
@@ -92,41 +91,40 @@ 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);
 
-extern struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
-extern void ocxl_afu_put(struct ocxl_afu *afu);
-
-extern int ocxl_create_cdev(struct ocxl_afu *afu);
-extern void ocxl_destroy_cdev(struct ocxl_afu *afu);
-extern int ocxl_register_afu(struct ocxl_afu *afu);
-extern void ocxl_unregister_afu(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);
 
-extern int ocxl_file_init(void);
-extern void ocxl_file_exit(void);
+int ocxl_file_init(void);
+void ocxl_file_exit(void);
 
-extern int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
-extern int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
 
-extern struct ocxl_context *ocxl_context_alloc(void);
-extern int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
+struct ocxl_context *ocxl_context_alloc(void);
+int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
 			struct address_space *mapping);
-extern int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
-extern int ocxl_context_mmap(struct ocxl_context *ctx,
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
+int ocxl_context_mmap(struct ocxl_context *ctx,
 			struct vm_area_struct *vma);
-extern int ocxl_context_detach(struct ocxl_context *ctx);
-extern void ocxl_context_detach_all(struct ocxl_afu *afu);
-extern void ocxl_context_free(struct ocxl_context *ctx);
+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);
 
-extern int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
-extern void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
+int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
+void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
 
-extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
-extern int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
-extern void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-extern int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
+void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
 			int eventfd);
-extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
 
 #endif /* _OCXL_INTERNAL_H_ */
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 9ff6ddc28e22..4544573cc93c 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -53,7 +53,7 @@ struct ocxl_fn_config {
  * Read the configuration space of a function and fill in a
  * ocxl_fn_config structure with all the function details
  */
-extern int ocxl_config_read_function(struct pci_dev *dev,
+int ocxl_config_read_function(struct pci_dev *dev,
 				struct ocxl_fn_config *fn);
 
 /*
@@ -62,14 +62,14 @@ extern int ocxl_config_read_function(struct pci_dev *dev,
  * AFU indexes can be sparse, so a driver should check all indexes up
  * to the maximum found in the function description
  */
-extern int ocxl_config_check_afu_index(struct pci_dev *dev,
+int ocxl_config_check_afu_index(struct pci_dev *dev,
 				struct ocxl_fn_config *fn, int afu_idx);
 
 /*
  * Read the configuration space of a function for the AFU specified by
  * the index 'afu_idx'. Fills in a ocxl_afu_config structure
  */
-extern int ocxl_config_read_afu(struct pci_dev *dev,
+int ocxl_config_read_afu(struct pci_dev *dev,
 				struct ocxl_fn_config *fn,
 				struct ocxl_afu_config *afu,
 				u8 afu_idx);
@@ -77,7 +77,7 @@ extern int ocxl_config_read_afu(struct pci_dev *dev,
 /*
  * Get the max PASID value that can be used by the function
  */
-extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
 
 /*
  * Tell an AFU, by writing in the configuration space, the PASIDs that
@@ -87,7 +87,7 @@ extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
  * 'afu_control_offset' is the offset of the AFU control DVSEC which
  * can be found in the function configuration
  */
-extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
+void ocxl_config_set_afu_pasid(struct pci_dev *dev,
 				int afu_control_offset,
 				int pasid_base, u32 pasid_count_log);
 
@@ -98,7 +98,7 @@ extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
  * 'supported' is the total number of actags desired by all the AFUs
  *             of the function.
  */
-extern int ocxl_config_get_actag_info(struct pci_dev *dev,
+int ocxl_config_get_actag_info(struct pci_dev *dev,
 				u16 *base, u16 *enabled, u16 *supported);
 
 /*
@@ -108,7 +108,7 @@ extern int ocxl_config_get_actag_info(struct pci_dev *dev,
  * 'func_offset' is the offset of the Function DVSEC that can found in
  * the function configuration
  */
-extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
+void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
 				u32 actag_base, u32 actag_count);
 
 /*
@@ -118,7 +118,7 @@ extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
  * 'afu_control_offset' is the offset of the AFU control DVSEC for the
  * desired AFU. It can be found in the AFU configuration
  */
-extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
+void ocxl_config_set_afu_actag(struct pci_dev *dev,
 				int afu_control_offset,
 				int actag_base, int actag_count);
 
@@ -128,7 +128,7 @@ extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
  * 'afu_control_offset' is the offset of the AFU control DVSEC for the
  * desired AFU. It can be found in the AFU configuration
  */
-extern void ocxl_config_set_afu_state(struct pci_dev *dev,
+void ocxl_config_set_afu_state(struct pci_dev *dev,
 				int afu_control_offset, int enable);
 
 /*
@@ -139,7 +139,7 @@ extern void ocxl_config_set_afu_state(struct pci_dev *dev,
  * between the host and device, and set the Transaction Layer on both
  * accordingly.
  */
-extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
+int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
 
 /*
  * Request an AFU to terminate a PASID.
@@ -152,7 +152,7 @@ extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
  * 'afu_control_offset' is the offset of the AFU control DVSEC for the
  * desired AFU. It can be found in the AFU configuration
  */
-extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
+int ocxl_config_terminate_pasid(struct pci_dev *dev,
 				int afu_control_offset, int pasid);
 
 /*
@@ -165,13 +165,13 @@ extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
  * Returns a 'link handle' that should be used for further calls for
  * the link
  */
-extern int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
+int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
 			void **link_handle);
 
 /*
  * Remove the association between the function and its link.
  */
-extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
+void ocxl_link_release(struct pci_dev *dev, void *link_handle);
 
 /*
  * Add a Process Element to the Shared Process Area for a link.
@@ -183,7 +183,7 @@ extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
  * 'xsl_err_data' is an argument passed to the above callback, if
  * defined
  */
-extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
+int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
 		u64 amr, struct mm_struct *mm,
 		void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
 		void *xsl_err_data);
@@ -195,12 +195,12 @@ extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
  * pasid: the PASID for the AFU context
  * tid: the new thread id for the process element
  */
-extern int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
 
 /*
  * Remove a Process Element from the Shared Process Area for a link
  */
-extern int ocxl_link_remove_pe(void *link_handle, int pasid);
+int ocxl_link_remove_pe(void *link_handle, int pasid);
 
 /*
  * Allocate an AFU interrupt associated to the link.
@@ -212,12 +212,12 @@ extern int ocxl_link_remove_pe(void *link_handle, int pasid);
  * interrupt. It is an MMIO address which needs to be remapped (one
  * page).
  */
-extern int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
+int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
 			u64 *obj_handle);
 
 /*
  * Free a previously allocated AFU interrupt
  */
-extern void ocxl_link_free_irq(void *link_handle, int hw_irq);
+void ocxl_link_free_irq(void *link_handle, int hw_irq);
 
 #endif /* _MISC_OCXL_H_ */
-- 
2.20.1


^ permalink raw reply related

* [PATCH 5/5] ocxl: Remove some unused exported symbols
From: Alastair D'Silva @ 2019-03-13  4:07 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>

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

Remove some unused exported symbols.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/config.c        |  2 --
 drivers/misc/ocxl/ocxl_internal.h | 23 +++++++++++++++++++++++
 include/misc/ocxl.h               | 23 -----------------------
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 026ac2ac4f9c..c90c2e4875bf 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
 	}
 	return 1;
 }
-EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
 
 static int read_afu_name(struct pci_dev *dev, struct ocxl_fn_config *fn,
 			struct ocxl_afu_config *afu)
@@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count)
 {
 	return pnv_ocxl_get_pasid_count(dev, count);
 }
-EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
 
 void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int pasid_base,
 			u32 pasid_count_log)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 321b29e77f45..06fd98c989c8 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -107,6 +107,29 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
 int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
 void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
 
+/*
+ * Get the max PASID value that can be used by the function
+ */
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+
+/*
+ * Check if an AFU index is valid for the given function.
+ *
+ * AFU indexes can be sparse, so a driver should check all indexes up
+ * to the maximum found in the function description
+ */
+int ocxl_config_check_afu_index(struct pci_dev *dev,
+				struct ocxl_fn_config *fn, int afu_idx);
+
+/**
+ * Update values within a Process Element
+ *
+ * link_handle: the link handle associated with the process element
+ * pasid: the PASID for the AFU context
+ * tid: the new thread id for the process element
+ */
+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);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 4544573cc93c..9530d3be1b30 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -56,15 +56,6 @@ struct ocxl_fn_config {
 int ocxl_config_read_function(struct pci_dev *dev,
 				struct ocxl_fn_config *fn);
 
-/*
- * Check if an AFU index is valid for the given function.
- *
- * AFU indexes can be sparse, so a driver should check all indexes up
- * to the maximum found in the function description
- */
-int ocxl_config_check_afu_index(struct pci_dev *dev,
-				struct ocxl_fn_config *fn, int afu_idx);
-
 /*
  * Read the configuration space of a function for the AFU specified by
  * the index 'afu_idx'. Fills in a ocxl_afu_config structure
@@ -74,11 +65,6 @@ int ocxl_config_read_afu(struct pci_dev *dev,
 				struct ocxl_afu_config *afu,
 				u8 afu_idx);
 
-/*
- * Get the max PASID value that can be used by the function
- */
-int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
-
 /*
  * Tell an AFU, by writing in the configuration space, the PASIDs that
  * it can use. Range starts at 'pasid_base' and its size is a multiple
@@ -188,15 +174,6 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
 		void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
 		void *xsl_err_data);
 
-/**
- * Update values within a Process Element
- *
- * link_handle: the link handle associated with the process element
- * pasid: the PASID for the AFU context
- * tid: the new thread id for the process element
- */
-int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
-
 /*
  * Remove a Process Element from the Shared Process Area for a link
  */
-- 
2.20.1


^ permalink raw reply related

* [PATCH 3/5] ocxl: read_pasid never returns an error, so make it void
From: Alastair D'Silva @ 2019-03-13  4:06 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
	Andrew Donnellan, Alastair D'Silva, Frederic Barrat,
	linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>

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

No need for a return value in read_pasid as it only returns 0.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 drivers/misc/ocxl/config.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 0ee7856b033d..026ac2ac4f9c 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -68,7 +68,7 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
 	return 0;
 }
 
-static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
+static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
 {
 	u16 val;
 	int pos;
@@ -89,7 +89,6 @@ static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
 out:
 	dev_dbg(&dev->dev, "PASID capability:\n");
 	dev_dbg(&dev->dev, "  Max PASID log = %d\n", fn->max_pasid_log);
-	return 0;
 }
 
 static int read_dvsec_tl(struct pci_dev *dev, struct ocxl_fn_config *fn)
@@ -205,11 +204,7 @@ int ocxl_config_read_function(struct pci_dev *dev, struct ocxl_fn_config *fn)
 {
 	int rc;
 
-	rc = read_pasid(dev, fn);
-	if (rc) {
-		dev_err(&dev->dev, "Invalid PASID configuration: %d\n", rc);
-		return -ENODEV;
-	}
+	read_pasid(dev, fn);
 
 	rc = read_dvsec_tl(dev, fn);
 	if (rc) {
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 0/5] ocxl: OpenCAPI Cleanup
From: Alastair D'Silva @ 2019-03-13  4:06 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>

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

Some minor cleanups for the OpenCAPI driver as a prerequisite
for an ocxl driver refactoring to allow the driver core to
be utilised by external drivers.

Changelog:
V2:
  - remove intermediate assignment of 'link' var in
		'Rename struct link to ocxl_link'
  - Don't shift definition of ocxl_context_attach in
		'Remove some unused exported symbols'

Alastair D'Silva (5):
  ocxl: Rename struct link to ocxl_link
  ocxl: Clean up printf formats
  ocxl: read_pasid never returns an error, so make it void
  ocxl: Remove superfluous 'extern' from headers
  ocxl: Remove some unused exported symbols

 drivers/misc/ocxl/config.c        | 17 ++----
 drivers/misc/ocxl/context.c       |  2 +-
 drivers/misc/ocxl/file.c          |  2 +-
 drivers/misc/ocxl/link.c          | 36 ++++++-------
 drivers/misc/ocxl/ocxl_internal.h | 86 +++++++++++++++++++------------
 drivers/misc/ocxl/trace.h         | 10 ++--
 include/misc/ocxl.h               | 53 ++++++-------------
 7 files changed, 99 insertions(+), 107 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 2/5] ocxl: Clean up printf formats
From: Alastair D'Silva @ 2019-03-13  4:06 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>

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

Use %# instead of using a literal '0x'

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/config.c  |  6 +++---
 drivers/misc/ocxl/context.c |  2 +-
 drivers/misc/ocxl/trace.h   | 10 +++++-----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 8f2c5d8bd2ee..0ee7856b033d 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -178,9 +178,9 @@ static int read_dvsec_vendor(struct pci_dev *dev)
 	pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx);
 
 	dev_dbg(&dev->dev, "Vendor specific DVSEC:\n");
-	dev_dbg(&dev->dev, "  CFG version = 0x%x\n", cfg);
-	dev_dbg(&dev->dev, "  TLX version = 0x%x\n", tlx);
-	dev_dbg(&dev->dev, "  DLX version = 0x%x\n", dlx);
+	dev_dbg(&dev->dev, "  CFG version = %#x\n", cfg);
+	dev_dbg(&dev->dev, "  TLX version = %#x\n", tlx);
+	dev_dbg(&dev->dev, "  DLX version = %#x\n", dlx);
 	return 0;
 }
 
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index c10a940e3b38..3498a0199bde 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -134,7 +134,7 @@ static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
 	vm_fault_t ret;
 
 	offset = vmf->pgoff << PAGE_SHIFT;
-	pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
+	pr_debug("%s: pasid %d address %#lx offset %#llx\n", __func__,
 		ctx->pasid, vmf->address, offset);
 
 	if (offset < ctx->afu->irq_base_offset)
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index bcb7ff330c1e..68bf2f173a1a 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -28,7 +28,7 @@ DECLARE_EVENT_CLASS(ocxl_context,
 		__entry->tidr = tidr;
 	),
 
-	TP_printk("linux pid=%d spa=0x%p pasid=0x%x pidr=0x%x tidr=0x%x",
+	TP_printk("linux pid=%d spa=%p pasid=%#x pidr=%#x tidr=%#x",
 		__entry->pid,
 		__entry->spa,
 		__entry->pasid,
@@ -61,7 +61,7 @@ TRACE_EVENT(ocxl_terminate_pasid,
 		__entry->rc = rc;
 	),
 
-	TP_printk("pasid=0x%x rc=%d",
+	TP_printk("pasid=%#x rc=%d",
 		__entry->pasid,
 		__entry->rc
 	)
@@ -87,7 +87,7 @@ DECLARE_EVENT_CLASS(ocxl_fault_handler,
 		__entry->tfc = tfc;
 	),
 
-	TP_printk("spa=%p pe=0x%llx dsisr=0x%llx dar=0x%llx tfc=0x%llx",
+	TP_printk("spa=%p pe=%#llx dsisr=%#llx dar=%#llx tfc=%#llx",
 		__entry->spa,
 		__entry->pe,
 		__entry->dsisr,
@@ -127,7 +127,7 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
 		__entry->irq_offset = irq_offset;
 	),
 
-	TP_printk("pasid=0x%x irq_id=%d virq=%u hw_irq=%d irq_offset=0x%llx",
+	TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d irq_offset=%#llx",
 		__entry->pasid,
 		__entry->irq_id,
 		__entry->virq,
@@ -150,7 +150,7 @@ TRACE_EVENT(ocxl_afu_irq_free,
 		__entry->irq_id = irq_id;
 	),
 
-	TP_printk("pasid=0x%x irq_id=%d",
+	TP_printk("pasid=%#x irq_id=%d",
 		__entry->pasid,
 		__entry->irq_id
 	)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Alastair D'Silva @ 2019-03-13  4:06 UTC (permalink / raw)
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
	Andrew Donnellan, Alastair D'Silva, Frederic Barrat,
	linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>

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

The term 'link' is ambiguous (especially when the struct is used for a
list), so rename it for clarity.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 drivers/misc/ocxl/file.c |  5 ++---
 drivers/misc/ocxl/link.c | 36 ++++++++++++++++++------------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index e6a607488f8a..009e09b7ded5 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -151,10 +151,9 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
 		mutex_unlock(&ctx->status_mutex);
 
 		if (status == ATTACHED) {
-			int rc;
-			struct link *link = ctx->afu->fn->link;
+			int rc = ocxl_link_update_pe(ctx->afu->fn->link,
+				ctx->pasid, ctx->tidr);
 
-			rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
 			if (rc)
 				return rc;
 		}
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index d50b861d7e57..8d2690a1a9de 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -76,7 +76,7 @@ struct spa {
  * limited number of opencapi slots on a system and lookup is only
  * done when the device is probed
  */
-struct link {
+struct ocxl_link {
 	struct list_head list;
 	struct kref ref;
 	int domain;
@@ -179,7 +179,7 @@ static void xsl_fault_handler_bh(struct work_struct *fault_work)
 
 static irqreturn_t xsl_fault_handler(int irq, void *data)
 {
-	struct link *link = (struct link *) data;
+	struct ocxl_link *link = (struct ocxl_link *) data;
 	struct spa *spa = link->spa;
 	u64 dsisr, dar, pe_handle;
 	struct pe_data *pe_data;
@@ -256,7 +256,7 @@ static int map_irq_registers(struct pci_dev *dev, struct spa *spa)
 				&spa->reg_tfc, &spa->reg_pe_handle);
 }
 
-static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
+static int setup_xsl_irq(struct pci_dev *dev, struct ocxl_link *link)
 {
 	struct spa *spa = link->spa;
 	int rc;
@@ -311,7 +311,7 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
 	return rc;
 }
 
-static void release_xsl_irq(struct link *link)
+static void release_xsl_irq(struct ocxl_link *link)
 {
 	struct spa *spa = link->spa;
 
@@ -323,7 +323,7 @@ static void release_xsl_irq(struct link *link)
 	unmap_irq_registers(spa);
 }
 
-static int alloc_spa(struct pci_dev *dev, struct link *link)
+static int alloc_spa(struct pci_dev *dev, struct ocxl_link *link)
 {
 	struct spa *spa;
 
@@ -350,7 +350,7 @@ static int alloc_spa(struct pci_dev *dev, struct link *link)
 	return 0;
 }
 
-static void free_spa(struct link *link)
+static void free_spa(struct ocxl_link *link)
 {
 	struct spa *spa = link->spa;
 
@@ -364,12 +364,12 @@ static void free_spa(struct link *link)
 	}
 }
 
-static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
+static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_link)
 {
-	struct link *link;
+	struct ocxl_link *link;
 	int rc;
 
-	link = kzalloc(sizeof(struct link), GFP_KERNEL);
+	link = kzalloc(sizeof(struct ocxl_link), GFP_KERNEL);
 	if (!link)
 		return -ENOMEM;
 
@@ -405,7 +405,7 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
 	return rc;
 }
 
-static void free_link(struct link *link)
+static void free_link(struct ocxl_link *link)
 {
 	release_xsl_irq(link);
 	free_spa(link);
@@ -415,7 +415,7 @@ static void free_link(struct link *link)
 int ocxl_link_setup(struct pci_dev *dev, int PE_mask, void **link_handle)
 {
 	int rc = 0;
-	struct link *link;
+	struct ocxl_link *link;
 
 	mutex_lock(&links_list_lock);
 	list_for_each_entry(link, &links_list, list) {
@@ -442,7 +442,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_setup);
 
 static void release_xsl(struct kref *ref)
 {
-	struct link *link = container_of(ref, struct link, ref);
+	struct ocxl_link *link = container_of(ref, struct ocxl_link, ref);
 
 	list_del(&link->list);
 	/* call platform code before releasing data */
@@ -452,7 +452,7 @@ static void release_xsl(struct kref *ref)
 
 void ocxl_link_release(struct pci_dev *dev, void *link_handle)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 
 	mutex_lock(&links_list_lock);
 	kref_put(&link->ref, release_xsl);
@@ -488,7 +488,7 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
 		void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
 		void *xsl_err_data)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 	struct spa *spa = link->spa;
 	struct ocxl_process_element *pe;
 	int pe_handle, rc = 0;
@@ -558,7 +558,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
 
 int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 	struct spa *spa = link->spa;
 	struct ocxl_process_element *pe;
 	int pe_handle, rc;
@@ -594,7 +594,7 @@ int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
 
 int ocxl_link_remove_pe(void *link_handle, int pasid)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 	struct spa *spa = link->spa;
 	struct ocxl_process_element *pe;
 	struct pe_data *pe_data;
@@ -666,7 +666,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
 
 int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 	int rc, irq;
 	u64 addr;
 
@@ -687,7 +687,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
 
 void ocxl_link_free_irq(void *link_handle, int hw_irq)
 {
-	struct link *link = (struct link *) link_handle;
+	struct ocxl_link *link = (struct ocxl_link *) link_handle;
 
 	pnv_ocxl_free_xive_irq(hw_irq);
 	atomic_inc(&link->irq_available);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] powerpc/64s: Mark 'dummy_copy_buffer' as used
From: Michael Ellerman @ 2019-03-13  4:05 UTC (permalink / raw)
  To: Christophe Leroy, Mathieu Malaterre
  Cc: linuxppc-dev, Paul Mackerras, linux-kernel, Nicholas Piggin
In-Reply-To: <38359442-e219-18ad-926b-42a6e5ab4e7d@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> On 03/12/2019 08:29 PM, Mathieu Malaterre wrote:
>> In commit 07d2a628bc00 ("powerpc/64s: Avoid cpabort in context switch
>> when possible") a buffer 'dummy_copy_buffer' was introduced. gcc does
>> not see this buffer being used in the inline assembly within function
>> '__switch_to', explicitly marked this variable as being used.
>> 
>> Prefer using '__aligned' to get passed line over 80 characters warning
>> in checkpatch.
>
> Powerpc accepts 90 characters, use arch/powerpc/tools/checkpatch.sh
>
>> 
>> This remove the following warning:
>> 
>>    arch/powerpc/kernel/process.c:1156:17: error: 'dummy_copy_buffer' defined but not used [-Werror=unused-const-variable=]
>
> commit 2bf1071a8d50 ("powerpc/64s: Remove POWER9 DD1 support") has 
> removed the fonction using 'dummy_copy_buffer' so you should remove it 
> completely.

Yes that would be better, thanks.

cheers

^ permalink raw reply

* [PATCH 1/1] arch/powerpc: Rework local_paca to avoid LTO warnings
From: Alastair D'Silva @ 2019-03-13  3:42 UTC (permalink / raw)
  Cc: Michal Hocko, Mahesh Salgaonkar, Alastair D'Silva,
	linux-kernel, Nicholas Piggin, Mike Rapoport, Paul Mackerras,
	Naveen N. Rao, linuxppc-dev, Andrew Morton

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

When building an LTO kernel, the existing code generates warnings:
    ./arch/powerpc/include/asm/paca.h:37:30: warning: register of
        ‘local_paca’ used for multiple global register variables
     register struct paca_struct *local_paca asm("r13");
                              ^
    ./arch/powerpc/include/asm/paca.h:37:30: note: conflicts with
        ‘local_paca’

This patch reworks local_paca into an inline getter & setter function,
which addresses the warning.

Generated ASM from this patch is broadly similar (addresses have
changed and the compiler uses different GPRs in some places).

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/paca.h | 44 +++++++++++++++++++++++----------
 arch/powerpc/kernel/paca.c      |  2 +-
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index e843bc5d1a0f..9c9e2dea0f9b 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -34,19 +34,6 @@
 #include <asm/cpuidle.h>
 #include <asm/atomic.h>
 
-register struct paca_struct *local_paca asm("r13");
-
-#if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
-extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
-/*
- * Add standard checks that preemption cannot occur when using get_paca():
- * otherwise the paca_struct it points to may be the wrong one just after.
- */
-#define get_paca()	((void) debug_smp_processor_id(), local_paca)
-#else
-#define get_paca()	local_paca
-#endif
-
 #ifdef CONFIG_PPC_PSERIES
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #endif
@@ -266,6 +253,37 @@ struct paca_struct {
 #endif
 } ____cacheline_aligned;
 
+#if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
+extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
+#endif
+
+static inline struct paca_struct *get_paca_no_preempt_check(void)
+{
+	register struct paca_struct *paca asm("r13");
+	return paca;
+}
+
+static inline struct paca_struct *get_paca(void)
+{
+#if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
+	/*
+	 * Add standard checks that preemption cannot occur when using get_paca():
+	 * otherwise the paca_struct it points to may be the wrong one just after.
+	 */
+	debug_smp_processor_id();
+#endif
+	return get_paca_no_preempt_check();
+}
+
+#define local_paca	get_paca_no_preempt_check()
+
+static inline void set_paca(struct paca_struct *new)
+{
+	register struct paca_struct *paca asm("r13");
+	paca = new;
+}
+
+
 extern void copy_mm_to_paca(struct mm_struct *mm);
 extern struct paca_struct **paca_ptrs;
 extern void initialise_paca(struct paca_struct *new_paca, int cpu);
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 913bfca09c4f..ae5c243f9d5a 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -172,7 +172,7 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
 void setup_paca(struct paca_struct *new_paca)
 {
 	/* Setup r13 */
-	local_paca = new_paca;
+	set_paca(new_paca);
 
 #ifdef CONFIG_PPC_BOOK3E
 	/* On Book3E, initialize the TLB miss exception frames */
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/1] arch/powerpc: Don't assume start_text & head_end align
From: Alastair D'Silva @ 2019-03-13  3:38 UTC (permalink / raw)
  Cc: Alastair D'Silva, linux-kernel, Paul Mackerras, linuxppc-dev

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

When building LTO kernels, the start_text symbol is not guaranteed to mark
the end of the head section.

Instead, look explicitly for __head_end.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/tools/head_check.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/tools/head_check.sh b/arch/powerpc/tools/head_check.sh
index ad9e57209aa4..1b0f634038c3 100644
--- a/arch/powerpc/tools/head_check.sh
+++ b/arch/powerpc/tools/head_check.sh
@@ -44,7 +44,7 @@ nm="$1"
 vmlinux="$2"
 
 # gcc-4.6-era toolchain make _stext an A (absolute) symbol rather than T
-$nm "$vmlinux" | grep -e " [TA] _stext$" -e " t start_first_256B$" -e " a text_start$" -e " t start_text$" -m4 > .tmp_symbols.txt
+$nm "$vmlinux" | grep -e " [TA] _stext$" -e " t start_first_256B$" -e " a text_start$" -e " T __head_end$" -m4 > .tmp_symbols.txt
 
 
 vma=$(cat .tmp_symbols.txt | grep -e " [TA] _stext$" | cut -d' ' -f1)
@@ -63,12 +63,12 @@ fi
 
 top_vma=$(echo $vma | cut -d'0' -f1)
 
-expected_start_text_addr=$(cat .tmp_symbols.txt | grep " a text_start$" | cut -d' ' -f1 | sed "s/^0/$top_vma/")
+expected_head_end_addr=$(cat .tmp_symbols.txt | grep " a text_start$" | cut -d' ' -f1 | sed "s/^0/$top_vma/")
 
-start_text_addr=$(cat .tmp_symbols.txt | grep " t start_text$" | cut -d' ' -f1)
+head_end_addr=$(cat .tmp_symbols.txt | grep " T __head_end$" | cut -d' ' -f1)
 
-if [ "$start_text_addr" != "$expected_start_text_addr" ]; then
-	echo "ERROR: start_text address is $start_text_addr, should be $expected_start_text_addr"
+if [ "$head_end_addr" != "$expected_head_end_addr" ]; then
+	echo "ERROR: __head_end address is $head_end_addr, should be $expected_head_end_addr"
 	echo "ERROR: try to enable LD_HEAD_STUB_CATCH config option"
 	echo "ERROR: see comments in arch/powerpc/tools/head_check.sh"
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Haibo Xu (Arm Technology China) @ 2019-03-13  1:03 UTC (permalink / raw)
  To: Sudeep Holla, Andy Lutomirski
  Cc: Steve Capper, Catalin Marinas, jdike@addtoit.com, x86@kernel.org,
	Will Deacon, linux-kernel@vger.kernel.org, Oleg Nesterov,
	Richard Weinberger, Ingo Molnar, Paul Mackerras, Borislav Petkov,
	Thomas Gleixner, Bin Lu (Arm Technology China),
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190312120944.GB13825@e107155-lin>

On 2019/3/12 20:09, Sudeep Holla wrote:
> On Mon, Mar 11, 2019 at 08:04:39PM -0700, Andy Lutomirski wrote:
>> On Mon, Mar 11, 2019 at 6:35 PM Haibo Xu (Arm Technology China)
>> <Haibo.Xu@arm.com> wrote:
>>>
>
> [...]
>
>>> For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send
>>> SIGTRAP) at the entry of a system call, no need to report at the exit of a
>>> system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP |
>>> _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special
>>> case(PTRACE_SYSEMU_SINGLESTEP).
>>>
>>> Another way to make sure the logic is fine, you can run some tests with
>>> respect to both logic, and to check whether they have the same behavior.
>>
>> tools/testing/selftests/x86/ptrace_syscall.c has a test intended to
>> exercise this.  Can one of you either confirm that it does exercise it
>> and that it still passes or can you improve the test?
>>
> I did run the tests which didn't flag anything. I haven't looked at the
> details of test implementation, but seem to miss this case. I will see
> what can be improved(if it's possible). Also I think single_step_syscall
> is the one I need to look for this particular one. Both single_step_syscall
> ptrace_syscall reported no errors.
>
> --
> Regards,
> Sudeep
>

Since ptrace() system call do have so many request type, I'm not sure whether the
test cases have covered all of that. But here we'd better make sure the PTRACE_SYSEMU
and PTRACE_SYSEMU_SINGLESTEP requests are work correctly. May be you can verify them with
tests from Bin Lu(bin.lu@arm.com).

Regards,
Haibo
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply

* Re: [PATCH v10 05/18] powerpc/prom_init: don't use string functions from lib/
From: Daniel Axtens @ 2019-03-12 23:38 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <dc2eb0480f2c87316561435cfcf4c78254d2166b.1552428161.git.christophe.leroy@c-s.fr>

Hi Christophe,

In trying to extend my KASAN implementation to Book3S 64bit, I found one
other change needed to prom_init. I don't know if you think it should go
in this patch, the next one, or somewhere else entirely - I will leave
it up to you. Just let me know if you want me to carry it separately.

Thanks again for all your work on this and the integration of my series.

Regards,
Daniel

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 7017156168e8..cebb3fc535ba 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1265,7 +1265,8 @@ static void __init prom_check_platform_support(void)
                                       "ibm,arch-vec-5-platform-support");
 
        /* First copy the architecture vec template */
-       ibm_architecture_vec = ibm_architecture_vec_template;
+       memcpy(&ibm_architecture_vec, &ibm_architecture_vec_template,
+              sizeof(struct ibm_arch_vec));
 
        if (prop_len > 1) {
                int i;

> When KASAN is active, the string functions in lib/ are doing the
> KASAN checks. This is too early for prom_init.
>
> This patch implements dedicated string functions for prom_init,
> which will be compiled in with KASAN disabled.
>
> Size of prom_init before the patch:
>    text	   data	    bss	    dec	    hex	filename
>   12060	    488	   6960	  19508	   4c34	arch/powerpc/kernel/prom_init.o
>
> Size of prom_init after the patch:
>    text	   data	    bss	    dec	    hex	filename
>   12460	    488	   6960	  19908	   4dc4	arch/powerpc/kernel/prom_init.o
>
> This increases the size of prom_init a bit, but as prom_init is
> in __init section, it is freed after boot anyway.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/kernel/prom_init.c        | 211 ++++++++++++++++++++++++++-------
>  arch/powerpc/kernel/prom_init_check.sh |   2 +-
>  2 files changed, 171 insertions(+), 42 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index ecf083c46bdb..7017156168e8 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -224,6 +224,135 @@ static bool  __prombss rtas_has_query_cpu_stopped;
>  #define PHANDLE_VALID(p)	((p) != 0 && (p) != PROM_ERROR)
>  #define IHANDLE_VALID(i)	((i) != 0 && (i) != PROM_ERROR)
>  
> +/* Copied from lib/string.c and lib/kstrtox.c */
> +
> +static int __init prom_strcmp(const char *cs, const char *ct)
> +{
> +	unsigned char c1, c2;
> +
> +	while (1) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +	}
> +	return 0;
> +}
> +
> +static char __init *prom_strcpy(char *dest, const char *src)
> +{
> +	char *tmp = dest;
> +
> +	while ((*dest++ = *src++) != '\0')
> +		/* nothing */;
> +	return tmp;
> +}
> +
> +static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> +{
> +	unsigned char c1, c2;
> +
> +	while (count) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +		count--;
> +	}
> +	return 0;
> +}
> +
> +static size_t __init prom_strlen(const char *s)
> +{
> +	const char *sc;
> +
> +	for (sc = s; *sc != '\0'; ++sc)
> +		/* nothing */;
> +	return sc - s;
> +}
> +
> +static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
> +{
> +	const unsigned char *su1, *su2;
> +	int res = 0;
> +
> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +		if ((res = *su1 - *su2) != 0)
> +			break;
> +	return res;
> +}
> +
> +static char __init *prom_strstr(const char *s1, const char *s2)
> +{
> +	size_t l1, l2;
> +
> +	l2 = prom_strlen(s2);
> +	if (!l2)
> +		return (char *)s1;
> +	l1 = prom_strlen(s1);
> +	while (l1 >= l2) {
> +		l1--;
> +		if (!prom_memcmp(s1, s2, l2))
> +			return (char *)s1;
> +		s1++;
> +	}
> +	return NULL;
> +}
> +
> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
> +{
> +	size_t ret = prom_strlen(src);
> +
> +	if (size) {
> +		size_t len = (ret >= size) ? size - 1 : ret;
> +		memcpy(dest, src, len);
> +		dest[len] = '\0';
> +	}
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PPC_PSERIES
> +static int __init prom_strtobool(const char *s, bool *res)
> +{
> +	if (!s)
> +		return -EINVAL;
> +
> +	switch (s[0]) {
> +	case 'y':
> +	case 'Y':
> +	case '1':
> +		*res = true;
> +		return 0;
> +	case 'n':
> +	case 'N':
> +	case '0':
> +		*res = false;
> +		return 0;
> +	case 'o':
> +	case 'O':
> +		switch (s[1]) {
> +		case 'n':
> +		case 'N':
> +			*res = true;
> +			return 0;
> +		case 'f':
> +		case 'F':
> +			*res = false;
> +			return 0;
> +		default:
> +			break;
> +		}
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +#endif
>  
>  /* This is the one and *ONLY* place where we actually call open
>   * firmware.
> @@ -555,7 +684,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
>  	add_string(&p, tohex((u32)(unsigned long) value));
>  	add_string(&p, tohex(valuelen));
>  	add_string(&p, tohex(ADDR(pname)));
> -	add_string(&p, tohex(strlen(pname)));
> +	add_string(&p, tohex(prom_strlen(pname)));
>  	add_string(&p, "property");
>  	*p = 0;
>  	return call_prom("interpret", 1, 1, (u32)(unsigned long) cmd);
> @@ -638,23 +767,23 @@ static void __init early_cmdline_parse(void)
>  	if ((long)prom.chosen > 0)
>  		l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>  	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl check */
> -		strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
> +		prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, sizeof(prom_cmd_line));
>  	prom_printf("command line: %s\n", prom_cmd_line);
>  
>  #ifdef CONFIG_PPC64
> -	opt = strstr(prom_cmd_line, "iommu=");
> +	opt = prom_strstr(prom_cmd_line, "iommu=");
>  	if (opt) {
>  		prom_printf("iommu opt is: %s\n", opt);
>  		opt += 6;
>  		while (*opt && *opt == ' ')
>  			opt++;
> -		if (!strncmp(opt, "off", 3))
> +		if (!prom_strncmp(opt, "off", 3))
>  			prom_iommu_off = 1;
> -		else if (!strncmp(opt, "force", 5))
> +		else if (!prom_strncmp(opt, "force", 5))
>  			prom_iommu_force_on = 1;
>  	}
>  #endif
> -	opt = strstr(prom_cmd_line, "mem=");
> +	opt = prom_strstr(prom_cmd_line, "mem=");
>  	if (opt) {
>  		opt += 4;
>  		prom_memory_limit = prom_memparse(opt, (const char **)&opt);
> @@ -666,13 +795,13 @@ static void __init early_cmdline_parse(void)
>  
>  #ifdef CONFIG_PPC_PSERIES
>  	prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
> -	opt = strstr(prom_cmd_line, "disable_radix");
> +	opt = prom_strstr(prom_cmd_line, "disable_radix");
>  	if (opt) {
>  		opt += 13;
>  		if (*opt && *opt == '=') {
>  			bool val;
>  
> -			if (kstrtobool(++opt, &val))
> +			if (prom_strtobool(++opt, &val))
>  				prom_radix_disable = false;
>  			else
>  				prom_radix_disable = val;
> @@ -1025,7 +1154,7 @@ static int __init prom_count_smt_threads(void)
>  		type[0] = 0;
>  		prom_getprop(node, "device_type", type, sizeof(type));
>  
> -		if (strcmp(type, "cpu"))
> +		if (prom_strcmp(type, "cpu"))
>  			continue;
>  		/*
>  		 * There is an entry for each smt thread, each entry being
> @@ -1472,7 +1601,7 @@ static void __init prom_init_mem(void)
>  			 */
>  			prom_getprop(node, "name", type, sizeof(type));
>  		}
> -		if (strcmp(type, "memory"))
> +		if (prom_strcmp(type, "memory"))
>  			continue;
>  
>  		plen = prom_getprop(node, "reg", regbuf, sizeof(regbuf));
> @@ -1753,19 +1882,19 @@ static void __init prom_initialize_tce_table(void)
>  		prom_getprop(node, "device_type", type, sizeof(type));
>  		prom_getprop(node, "model", model, sizeof(model));
>  
> -		if ((type[0] == 0) || (strstr(type, "pci") == NULL))
> +		if ((type[0] == 0) || (prom_strstr(type, "pci") == NULL))
>  			continue;
>  
>  		/* Keep the old logic intact to avoid regression. */
>  		if (compatible[0] != 0) {
> -			if ((strstr(compatible, "python") == NULL) &&
> -			    (strstr(compatible, "Speedwagon") == NULL) &&
> -			    (strstr(compatible, "Winnipeg") == NULL))
> +			if ((prom_strstr(compatible, "python") == NULL) &&
> +			    (prom_strstr(compatible, "Speedwagon") == NULL) &&
> +			    (prom_strstr(compatible, "Winnipeg") == NULL))
>  				continue;
>  		} else if (model[0] != 0) {
> -			if ((strstr(model, "ython") == NULL) &&
> -			    (strstr(model, "peedwagon") == NULL) &&
> -			    (strstr(model, "innipeg") == NULL))
> +			if ((prom_strstr(model, "ython") == NULL) &&
> +			    (prom_strstr(model, "peedwagon") == NULL) &&
> +			    (prom_strstr(model, "innipeg") == NULL))
>  				continue;
>  		}
>  
> @@ -1914,12 +2043,12 @@ static void __init prom_hold_cpus(void)
>  
>  		type[0] = 0;
>  		prom_getprop(node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "cpu") != 0)
> +		if (prom_strcmp(type, "cpu") != 0)
>  			continue;
>  
>  		/* Skip non-configured cpus. */
>  		if (prom_getprop(node, "status", type, sizeof(type)) > 0)
> -			if (strcmp(type, "okay") != 0)
> +			if (prom_strcmp(type, "okay") != 0)
>  				continue;
>  
>  		reg = cpu_to_be32(-1); /* make sparse happy */
> @@ -1995,9 +2124,9 @@ static void __init prom_find_mmu(void)
>  		return;
>  	version[sizeof(version) - 1] = 0;
>  	/* XXX might need to add other versions here */
> -	if (strcmp(version, "Open Firmware, 1.0.5") == 0)
> +	if (prom_strcmp(version, "Open Firmware, 1.0.5") == 0)
>  		of_workarounds = OF_WA_CLAIM;
> -	else if (strncmp(version, "FirmWorks,3.", 12) == 0) {
> +	else if (prom_strncmp(version, "FirmWorks,3.", 12) == 0) {
>  		of_workarounds = OF_WA_CLAIM | OF_WA_LONGTRAIL;
>  		call_prom("interpret", 1, 1, "dev /memory 0 to allow-reclaim");
>  	} else
> @@ -2030,7 +2159,7 @@ static void __init prom_init_stdout(void)
>  	call_prom("instance-to-path", 3, 1, prom.stdout, path, 255);
>  	prom_printf("OF stdout device is: %s\n", of_stdout_device);
>  	prom_setprop(prom.chosen, "/chosen", "linux,stdout-path",
> -		     path, strlen(path) + 1);
> +		     path, prom_strlen(path) + 1);
>  
>  	/* instance-to-package fails on PA-Semi */
>  	stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
> @@ -2040,7 +2169,7 @@ static void __init prom_init_stdout(void)
>  		/* If it's a display, note it */
>  		memset(type, 0, sizeof(type));
>  		prom_getprop(stdout_node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "display") == 0)
> +		if (prom_strcmp(type, "display") == 0)
>  			prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
>  	}
>  }
> @@ -2061,19 +2190,19 @@ static int __init prom_find_machine_type(void)
>  		compat[len] = 0;
>  		while (i < len) {
>  			char *p = &compat[i];
> -			int sl = strlen(p);
> +			int sl = prom_strlen(p);
>  			if (sl == 0)
>  				break;
> -			if (strstr(p, "Power Macintosh") ||
> -			    strstr(p, "MacRISC"))
> +			if (prom_strstr(p, "Power Macintosh") ||
> +			    prom_strstr(p, "MacRISC"))
>  				return PLATFORM_POWERMAC;
>  #ifdef CONFIG_PPC64
>  			/* We must make sure we don't detect the IBM Cell
>  			 * blades as pSeries due to some firmware issues,
>  			 * so we do it here.
>  			 */
> -			if (strstr(p, "IBM,CBEA") ||
> -			    strstr(p, "IBM,CPBW-1.0"))
> +			if (prom_strstr(p, "IBM,CBEA") ||
> +			    prom_strstr(p, "IBM,CPBW-1.0"))
>  				return PLATFORM_GENERIC;
>  #endif /* CONFIG_PPC64 */
>  			i += sl + 1;
> @@ -2090,7 +2219,7 @@ static int __init prom_find_machine_type(void)
>  			   compat, sizeof(compat)-1);
>  	if (len <= 0)
>  		return PLATFORM_GENERIC;
> -	if (strcmp(compat, "chrp"))
> +	if (prom_strcmp(compat, "chrp"))
>  		return PLATFORM_GENERIC;
>  
>  	/* Default to pSeries. We need to know if we are running LPAR */
> @@ -2152,7 +2281,7 @@ static void __init prom_check_displays(void)
>  	for (node = 0; prom_next_node(&node); ) {
>  		memset(type, 0, sizeof(type));
>  		prom_getprop(node, "device_type", type, sizeof(type));
> -		if (strcmp(type, "display") != 0)
> +		if (prom_strcmp(type, "display") != 0)
>  			continue;
>  
>  		/* It seems OF doesn't null-terminate the path :-( */
> @@ -2256,9 +2385,9 @@ static unsigned long __init dt_find_string(char *str)
>  	s = os = (char *)dt_string_start;
>  	s += 4;
>  	while (s <  (char *)dt_string_end) {
> -		if (strcmp(s, str) == 0)
> +		if (prom_strcmp(s, str) == 0)
>  			return s - os;
> -		s += strlen(s) + 1;
> +		s += prom_strlen(s) + 1;
>  	}
>  	return 0;
>  }
> @@ -2291,7 +2420,7 @@ static void __init scan_dt_build_strings(phandle node,
>  		}
>  
>   		/* skip "name" */
> - 		if (strcmp(namep, "name") == 0) {
> +		if (prom_strcmp(namep, "name") == 0) {
>   			*mem_start = (unsigned long)namep;
>   			prev_name = "name";
>   			continue;
> @@ -2303,7 +2432,7 @@ static void __init scan_dt_build_strings(phandle node,
>  			namep = sstart + soff;
>  		} else {
>  			/* Trim off some if we can */
> -			*mem_start = (unsigned long)namep + strlen(namep) + 1;
> +			*mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>  			dt_string_end = *mem_start;
>  		}
>  		prev_name = namep;
> @@ -2372,7 +2501,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>  			break;
>  
>   		/* skip "name" */
> - 		if (strcmp(pname, "name") == 0) {
> +		if (prom_strcmp(pname, "name") == 0) {
>   			prev_name = "name";
>   			continue;
>   		}
> @@ -2403,7 +2532,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
>  		call_prom("getprop", 4, 1, node, pname, valp, l);
>  		*mem_start = _ALIGN(*mem_start, 4);
>  
> -		if (!strcmp(pname, "phandle"))
> +		if (!prom_strcmp(pname, "phandle"))
>  			has_phandle = 1;
>  	}
>  
> @@ -2473,8 +2602,8 @@ static void __init flatten_device_tree(void)
>  
>  	/* Add "phandle" in there, we'll need it */
>  	namep = make_room(&mem_start, &mem_end, 16, 1);
> -	strcpy(namep, "phandle");
> -	mem_start = (unsigned long)namep + strlen(namep) + 1;
> +	prom_strcpy(namep, "phandle");
> +	mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>  
>  	/* Build string array */
>  	prom_printf("Building dt strings...\n"); 
> @@ -2796,7 +2925,7 @@ static void __init fixup_device_tree_efika(void)
>  	rv = prom_getprop(node, "model", prop, sizeof(prop));
>  	if (rv == PROM_ERROR)
>  		return;
> -	if (strcmp(prop, "EFIKA5K2"))
> +	if (prom_strcmp(prop, "EFIKA5K2"))
>  		return;
>  
>  	prom_printf("Applying EFIKA device tree fixups\n");
> @@ -2804,13 +2933,13 @@ static void __init fixup_device_tree_efika(void)
>  	/* Claiming to be 'chrp' is death */
>  	node = call_prom("finddevice", 1, 1, ADDR("/"));
>  	rv = prom_getprop(node, "device_type", prop, sizeof(prop));
> -	if (rv != PROM_ERROR && (strcmp(prop, "chrp") == 0))
> +	if (rv != PROM_ERROR && (prom_strcmp(prop, "chrp") == 0))
>  		prom_setprop(node, "/", "device_type", "efika", sizeof("efika"));
>  
>  	/* CODEGEN,description is exposed in /proc/cpuinfo so
>  	   fix that too */
>  	rv = prom_getprop(node, "CODEGEN,description", prop, sizeof(prop));
> -	if (rv != PROM_ERROR && (strstr(prop, "CHRP")))
> +	if (rv != PROM_ERROR && (prom_strstr(prop, "CHRP")))
>  		prom_setprop(node, "/", "CODEGEN,description",
>  			     "Efika 5200B PowerPC System",
>  			     sizeof("Efika 5200B PowerPC System"));
> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 181fd10008ef..4cac45cb5de5 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -27,7 +27,7 @@ fi
>  WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>  _end enter_prom $MEM_FUNCS reloc_offset __secondary_hold
>  __secondary_hold_acknowledge __secondary_hold_spinloop __start
> -strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
> +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.13.3

^ permalink raw reply related

* [PATCH RFC v3 18/18] powerpc: KASAN for 64bit Book3E
From: Christophe Leroy @ 2019-03-12 22:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1552428161.git.christophe.leroy@c-s.fr>

From: Daniel Axtens <dja@axtens.net>

Wire up KASAN. Only outline instrumentation is supported.

The KASAN shadow area is mapped into vmemmap space:
0x8000 0400 0000 0000 to 0x8000 0600 0000 0000.
To do this we require that vmemmap be disabled. (This is the default
in the kernel config that QorIQ provides for the machine in their
SDK anyway - they use flat memory.)

Only the kernel linear mapping (0xc000...) is checked. The vmalloc and
ioremap areas (also in 0x800...) are all mapped to the zero page. As
with the Book3S hash series, this requires overriding the memory <->
shadow mapping.

Also, as with both previous 64-bit series, early instrumentation is not
supported.  It would allow us to drop the check_return_arch_not_ready()
hook in the KASAN core, but it's tricky to get it set up early enough:
we need it setup before the first call to instrumented code like printk().
Perhaps in the future.

Only KASAN_MINIMAL works.

Tested on e6500. KVM, kexec and xmon have not been tested.

The test_kasan module fires warnings as expected, except for the
following tests:

 - Expected/by design:
kasan test: memcg_accounted_kmem_cache allocate memcg accounted object

 - Due to only supporting KASAN_MINIMAL:
kasan test: kasan_stack_oob out-of-bounds on stack
kasan test: kasan_global_oob out-of-bounds global variable
kasan test: kasan_alloca_oob_left out-of-bounds to left on alloca
kasan test: kasan_alloca_oob_right out-of-bounds to right on alloca
kasan test: use_after_scope_test use-after-scope on int
kasan test: use_after_scope_test use-after-scope on array

Thanks to those who have done the heavy lifting over the past several
years:
 - Christophe's 32 bit series: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-February/185379.html
 - Aneesh's Book3S hash series: https://lwn.net/Articles/655642/
 - Balbir's Book3S radix series: https://patchwork.ozlabs.org/patch/795211/

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
[- Removed EXPORT_SYMBOL of the static key
 - Fixed most checkpatch problems
 - Replaced kasan_zero_page[] by kasan_early_shadow_page[]
 - Reduced casting mess by using intermediate locals
 - Fixed build failure on pmac32_defconfig]
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/Kconfig                         |  1 +
 arch/powerpc/Kconfig.debug                   |  2 +-
 arch/powerpc/include/asm/kasan.h             | 71 ++++++++++++++++++++++++++++
 arch/powerpc/mm/Makefile                     |  2 +
 arch/powerpc/mm/kasan/Makefile               |  1 +
 arch/powerpc/mm/kasan/kasan_init_book3e_64.c | 50 ++++++++++++++++++++
 6 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/mm/kasan/kasan_init_book3e_64.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d9364368329b..51ef9fac6c5d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -174,6 +174,7 @@ config PPC
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KASAN			if PPC32
+	select HAVE_ARCH_KASAN			if PPC_BOOK3E_64 && !SPARSEMEM_VMEMMAP
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS	if COMPAT
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 61febbbdd02b..fc1f5fa7554e 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -369,5 +369,5 @@ config PPC_FAST_ENDIAN_SWITCH
 
 config KASAN_SHADOW_OFFSET
 	hex
-	depends on KASAN
+	depends on KASAN && PPC32
 	default 0xe0000000
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
index 296e51c2f066..ae410f0e060d 100644
--- a/arch/powerpc/include/asm/kasan.h
+++ b/arch/powerpc/include/asm/kasan.h
@@ -21,12 +21,15 @@
 #define KASAN_SHADOW_START	(KASAN_SHADOW_OFFSET + \
 				 (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
 
+#ifdef CONFIG_PPC32
 #define KASAN_SHADOW_OFFSET	ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
 
 #define KASAN_SHADOW_END	0UL
 
 #define KASAN_SHADOW_SIZE	(KASAN_SHADOW_END - KASAN_SHADOW_START)
 
+#endif /* CONFIG_PPC32 */
+
 #ifdef CONFIG_KASAN
 void kasan_early_init(void);
 void kasan_mmu_init(void);
@@ -36,5 +39,73 @@ static inline void kasan_init(void) { }
 static inline void kasan_mmu_init(void) { }
 #endif
 
+#ifdef CONFIG_PPC_BOOK3E_64
+#include <asm/pgtable.h>
+#include <linux/jump_label.h>
+
+/*
+ * We don't put this in Kconfig as we only support KASAN_MINIMAL, and
+ * that will be disabled if the symbol is available in Kconfig
+ */
+#define KASAN_SHADOW_OFFSET	ASM_CONST(0x6800040000000000)
+
+#define KASAN_SHADOW_SIZE	(KERN_VIRT_SIZE >> KASAN_SHADOW_SCALE_SHIFT)
+
+extern struct static_key_false powerpc_kasan_enabled_key;
+extern unsigned char kasan_early_shadow_page[];
+
+static inline bool kasan_arch_is_ready_book3e(void)
+{
+	if (static_branch_likely(&powerpc_kasan_enabled_key))
+		return true;
+	return false;
+}
+#define kasan_arch_is_ready kasan_arch_is_ready_book3e
+
+static inline void *kasan_mem_to_shadow_book3e(const void *ptr)
+{
+	unsigned long addr = (unsigned long)ptr;
+
+	if (addr >= KERN_VIRT_START && addr < KERN_VIRT_START + KERN_VIRT_SIZE)
+		return kasan_early_shadow_page;
+
+	return (void *)(addr >> KASAN_SHADOW_SCALE_SHIFT) + KASAN_SHADOW_OFFSET;
+}
+#define kasan_mem_to_shadow kasan_mem_to_shadow_book3e
+
+static inline void *kasan_shadow_to_mem_book3e(const void *shadow_addr)
+{
+	/*
+	 * We map the entire non-linear virtual mapping onto the zero page so if
+	 * we are asked to map the zero page back just pick the beginning of that
+	 * area.
+	 */
+	if (shadow_addr >= (void *)kasan_early_shadow_page &&
+	    shadow_addr < (void *)(kasan_early_shadow_page + PAGE_SIZE))
+		return (void *)KERN_VIRT_START;
+
+	return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET) <<
+			KASAN_SHADOW_SCALE_SHIFT);
+}
+#define kasan_shadow_to_mem kasan_shadow_to_mem_book3e
+
+static inline bool kasan_addr_has_shadow_book3e(const void *ptr)
+{
+	unsigned long addr = (unsigned long)ptr;
+
+	/*
+	 * We want to specifically assert that the addresses in the 0x8000...
+	 * region have a shadow, otherwise they are considered by the kasan
+	 * core to be wild pointers
+	 */
+	if (addr >= KERN_VIRT_START && addr < (KERN_VIRT_START + KERN_VIRT_SIZE))
+		return true;
+
+	return (ptr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
+}
+#define kasan_addr_has_shadow kasan_addr_has_shadow_book3e
+
+#endif /* CONFIG_PPC_BOOK3E_64 */
+
 #endif /* __ASSEMBLY */
 #endif
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 80382a2d169b..fc49231f807c 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -8,9 +8,11 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
 CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
 
 KASAN_SANITIZE_ppc_mmu_32.o := n
+KASAN_SANITIZE_fsl_booke_mmu.o := n
 
 ifdef CONFIG_KASAN
 CFLAGS_ppc_mmu_32.o  		+= -DDISABLE_BRANCH_PROFILING
+CFLAGS_fsl_booke_mmu.o		+= -DDISABLE_BRANCH_PROFILING
 endif
 
 obj-y				:= fault.o mem.o pgtable.o mmap.o \
diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
index 6577897673dd..f8f164ad8ade 100644
--- a/arch/powerpc/mm/kasan/Makefile
+++ b/arch/powerpc/mm/kasan/Makefile
@@ -3,3 +3,4 @@
 KASAN_SANITIZE := n
 
 obj-$(CONFIG_PPC32)           += kasan_init_32.o
+obj-$(CONFIG_PPC_BOOK3E_64)   += kasan_init_book3e_64.o
diff --git a/arch/powerpc/mm/kasan/kasan_init_book3e_64.c b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
new file mode 100644
index 000000000000..f116c211d83c
--- /dev/null
+++ b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define DISABLE_BRANCH_PROFILING
+
+#include <linux/kasan.h>
+#include <linux/printk.h>
+#include <linux/memblock.h>
+#include <linux/sched/task.h>
+#include <asm/pgalloc.h>
+
+DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
+
+static void __init kasan_init_region(struct memblock_region *reg)
+{
+	void *start = __va(reg->base);
+	void *end = __va(reg->base + reg->size);
+	unsigned long k_start, k_end, k_cur;
+
+	if (start >= end)
+		return;
+
+	k_start = (unsigned long)kasan_mem_to_shadow(start);
+	k_end = (unsigned long)kasan_mem_to_shadow(end);
+
+	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
+		void *va = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
+
+		map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
+	}
+	flush_tlb_kernel_range(k_start, k_end);
+}
+
+void __init kasan_init(void)
+{
+	struct memblock_region *reg;
+
+	for_each_memblock(memory, reg)
+		kasan_init_region(reg);
+
+	/* map the zero page RO */
+	map_kernel_page((unsigned long)kasan_early_shadow_page,
+			__pa(kasan_early_shadow_page), PAGE_KERNEL_RO);
+
+	/* Turn on checking */
+	static_branch_inc(&powerpc_kasan_enabled_key);
+
+	/* Enable error messages */
+	init_task.kasan_depth = 0;
+	pr_info("KASAN init done (64-bit Book3E)\n");
+}
-- 
2.13.3


^ permalink raw reply related

* [PATCH RFC v3 16/18] kasan: allow architectures to manage the memory-to-shadow mapping
From: Christophe Leroy @ 2019-03-12 22:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1552428161.git.christophe.leroy@c-s.fr>

From: Daniel Axtens <dja@axtens.net>

Currently, shadow addresses are always addr >> shift + offset.
However, for powerpc, the virtual address space is fragmented in
ways that make this simple scheme impractical.

Allow architectures to override:
 - kasan_shadow_to_mem
 - kasan_mem_to_shadow
 - addr_has_shadow

Rename addr_has_shadow to kasan_addr_has_shadow as if it is
overridden it will be available in more places, increasing the
risk of collisions.

If architectures do not #define their own versions, the generic
code will continue to run as usual.

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 include/linux/kasan.h     | 2 ++
 mm/kasan/generic.c        | 2 +-
 mm/kasan/generic_report.c | 2 +-
 mm/kasan/kasan.h          | 6 +++++-
 mm/kasan/report.c         | 6 +++---
 mm/kasan/tags.c           | 2 +-
 6 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index b40ea104dd36..f6261840f94c 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -23,11 +23,13 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
 int kasan_populate_early_shadow(const void *shadow_start,
 				const void *shadow_end);
 
+#ifndef kasan_mem_to_shadow
 static inline void *kasan_mem_to_shadow(const void *addr)
 {
 	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
 		+ KASAN_SHADOW_OFFSET;
 }
+#endif
 
 /* Enable reporting bugs after kasan_disable_current() */
 extern void kasan_enable_current(void);
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index 9e5c989dab8c..a5b28e3ceacb 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -173,7 +173,7 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
 	if (unlikely(size == 0))
 		return;
 
-	if (unlikely(!addr_has_shadow((void *)addr))) {
+	if (unlikely(!kasan_addr_has_shadow((void *)addr))) {
 		kasan_report(addr, size, write, ret_ip);
 		return;
 	}
diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c
index 36c645939bc9..6caafd61fc3a 100644
--- a/mm/kasan/generic_report.c
+++ b/mm/kasan/generic_report.c
@@ -107,7 +107,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
 
 const char *get_bug_type(struct kasan_access_info *info)
 {
-	if (addr_has_shadow(info->access_addr))
+	if (kasan_addr_has_shadow(info->access_addr))
 		return get_shadow_bug_type(info);
 	return get_wild_bug_type(info);
 }
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3e0c11f7d7a1..958e984d4544 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -110,16 +110,20 @@ struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
 struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
 					const void *object);
 
+#ifndef kasan_shadow_to_mem
 static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 {
 	return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
+#endif
 
-static inline bool addr_has_shadow(const void *addr)
+#ifndef kasan_addr_has_shadow
+static inline bool kasan_addr_has_shadow(const void *addr)
 {
 	return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
 }
+#endif
 
 void kasan_poison_shadow(const void *address, size_t size, u8 value);
 
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index ca9418fe9232..bc3355ee2dd0 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -298,7 +298,7 @@ void kasan_report(unsigned long addr, size_t size,
 	untagged_addr = reset_tag(tagged_addr);
 
 	info.access_addr = tagged_addr;
-	if (addr_has_shadow(untagged_addr))
+	if (kasan_addr_has_shadow(untagged_addr))
 		info.first_bad_addr = find_first_bad_addr(tagged_addr, size);
 	else
 		info.first_bad_addr = untagged_addr;
@@ -309,11 +309,11 @@ void kasan_report(unsigned long addr, size_t size,
 	start_report(&flags);
 
 	print_error_description(&info);
-	if (addr_has_shadow(untagged_addr))
+	if (kasan_addr_has_shadow(untagged_addr))
 		print_tags(get_tag(tagged_addr), info.first_bad_addr);
 	pr_err("\n");
 
-	if (addr_has_shadow(untagged_addr)) {
+	if (kasan_addr_has_shadow(untagged_addr)) {
 		print_address_description(untagged_addr);
 		pr_err("\n");
 		print_shadow_for_address(info.first_bad_addr);
diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
index 87ebee0a6aea..661c23dd5340 100644
--- a/mm/kasan/tags.c
+++ b/mm/kasan/tags.c
@@ -109,7 +109,7 @@ void check_memory_region(unsigned long addr, size_t size, bool write,
 		return;
 
 	untagged_addr = reset_tag((const void *)addr);
-	if (unlikely(!addr_has_shadow(untagged_addr))) {
+	if (unlikely(!kasan_addr_has_shadow(untagged_addr))) {
 		kasan_report(addr, size, write, ret_ip);
 		return;
 	}
-- 
2.13.3


^ permalink raw reply related

* [PATCH RFC v3 17/18] kasan: allow architectures to provide an outline readiness check
From: Christophe Leroy @ 2019-03-12 22:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1552428161.git.christophe.leroy@c-s.fr>

From: Daniel Axtens <dja@axtens.net>

In powerpc (as I understand it), we spend a lot of time in boot
running in real mode before MMU paging is initialised. During
this time we call a lot of generic code, including printk(). If
we try to access the shadow region during this time, things fail.

My attempts to move early init before the first printk have not
been successful. (Both previous RFCs for ppc64 - by 2 different
people - have needed this trick too!)

So, allow architectures to define a kasan_arch_is_ready()
hook that bails out of check_memory_region_inline() unless the
arch has done all of the init.

Link: https://lore.kernel.org/patchwork/patch/592820/ # ppc64 hash series
Link: https://patchwork.ozlabs.org/patch/795211/      # ppc radix series
Originally-by: Balbir Singh <bsingharora@gmail.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
[check_return_arch_not_ready() ==> static inline kasan_arch_is_ready()]
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 include/linux/kasan.h | 4 ++++
 mm/kasan/generic.c    | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index f6261840f94c..a630d53f1a36 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -14,6 +14,10 @@ struct task_struct;
 #include <asm/kasan.h>
 #include <asm/pgtable.h>
 
+#ifndef kasan_arch_is_ready
+static inline bool kasan_arch_is_ready(void)	{ return true; }
+#endif
+
 extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
 extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE];
 extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index a5b28e3ceacb..0336f31bbae3 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -170,6 +170,9 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
 						size_t size, bool write,
 						unsigned long ret_ip)
 {
+	if (!kasan_arch_is_ready())
+		return;
+
 	if (unlikely(size == 0))
 		return;
 
-- 
2.13.3


^ permalink raw reply related

* [PATCH RFC v3 15/18] kasan: do not open-code addr_has_shadow
From: Christophe Leroy @ 2019-03-12 22:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1552428161.git.christophe.leroy@c-s.fr>

From: Daniel Axtens <dja@axtens.net>

We have a couple of places checking for the existence of a shadow
mapping for an address by open-coding the inverse of the check in
addr_has_shadow.

Replace the open-coded versions with the helper. This will be
needed in future to allow architectures to override the layout
of the shadow mapping.

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 mm/kasan/generic.c | 3 +--
 mm/kasan/tags.c    | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index 504c79363a34..9e5c989dab8c 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -173,8 +173,7 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
 	if (unlikely(size == 0))
 		return;
 
-	if (unlikely((void *)addr <
-		kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) {
+	if (unlikely(!addr_has_shadow((void *)addr))) {
 		kasan_report(addr, size, write, ret_ip);
 		return;
 	}
diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
index 63fca3172659..87ebee0a6aea 100644
--- a/mm/kasan/tags.c
+++ b/mm/kasan/tags.c
@@ -109,8 +109,7 @@ void check_memory_region(unsigned long addr, size_t size, bool write,
 		return;
 
 	untagged_addr = reset_tag((const void *)addr);
-	if (unlikely(untagged_addr <
-			kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) {
+	if (unlikely(!addr_has_shadow(untagged_addr))) {
 		kasan_report(addr, size, write, ret_ip);
 		return;
 	}
-- 
2.13.3


^ 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