Re: [PATCH v6 16/17] powerpc/vas: Implement a simple FTW driver

[Michael Ellerman]

Hi Suka,

Some comments inline ...


Sukadev Bhattiprolu  writes:

> The Fast Thread Wake-up (FTW) driver provides user space applications an
> interface to the Core-to-Core functionality in POWER9. The driver provides
> the device node/ioctl API to applications and uses the external interfaces
> to the VAS driver to interact with the VAS hardware.
>
> A follow-on patch provides detailed description of the API for the driver.
>
> Signed-off-by: Sukadev Bhattiprolu 
> ---
>  MAINTAINERS |   1 +
>  arch/powerpc/platforms/powernv/Kconfig  |  16 ++
>  arch/powerpc/platforms/powernv/Makefile |   1 +
>  arch/powerpc/platforms/powernv/nx-ftw.c | 486 
> 

AFAICS this has nothing to do with NX, so why is it called nx-ftw ?

Also aren't we going to want to use this on pseries eventually? If so
should it go in arch/powerpc/sysdev ?

> diff --git a/arch/powerpc/platforms/powernv/Makefile 
> b/arch/powerpc/platforms/powernv/Makefile
> index e4db292..dc60046 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_MEMORY_FAILURE)+= opal-memory-errors.o
>  obj-$(CONFIG_TRACEPOINTS)+= opal-tracepoints.o
>  obj-$(CONFIG_OPAL_PRD)   += opal-prd.o
>  obj-$(CONFIG_PPC_VAS)+= vas.o vas-window.o
> +obj-$(CONFIG_PPC_FTW)+= nx-ftw.o
> diff --git a/arch/powerpc/platforms/powernv/nx-ftw.c 
> b/arch/powerpc/platforms/powernv/nx-ftw.c
> new file mode 100644
> index 000..a0b6388
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/nx-ftw.c
> @@ -0,0 +1,486 @@

Missing license header.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please try and trim the list to what you need.

> +
> +/*
> + * NX-FTW is a device driver used to provide user space access to the
> + * Core-to-Core aka Fast Thread Wakeup (FTW) functionality provided by
> + * the Virtual Accelerator Subsystem (VAS) in POWER9 systems. See also
> + * arch/powerpc/platforms/powernv/vas*.
> + *
> + * The driver creates the device node /dev/crypto/nx-ftw that can be
> + * used as follows:
> + *
> + *   fd = open("/dev/crypto/nx-ftw", O_RDWR);
> + *   rc = ioctl(fd, VAS_RX_WIN_OPEN, );
> + *   rc = ioctl(fd, VAS_TX_WIN_OPEN, );
> + *   paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
> + *   vas_copy(, 0, 1);
> + *   vas_paste(paste_addr, 0, 1);
> + *
> + * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
> + */
> +
> +static char  *nxftw_dev_name = "nx-ftw";
> +static atomic_t  nxftw_instid = ATOMIC_INIT(0);
> +static dev_t nxftw_devt;
> +static struct dentry *nxftw_debugfs;
> +static struct class  *nxftw_dbgfs_class;

The class doesn't go in debugfs, which is what "dbgfs" says to me.

> +/*
> + * Wrapper object for the nx-ftw device node - there is just one

Just "device".

"device node" is ambiguous vs device tree.

> + * instance of this node for the whole system.

So why not put the globals above in here also?

> + */
> +struct nxftw_dev {
> + struct cdev cdev;
> + struct device *device;
> + char *name;
> + atomic_t refcount;
> +} nxftw_device;
> +
> +/*
> + * One instance per open of a nx-ftw device. Each nxftw_instance is
> + * associated with a VAS window, after the caller issues VAS_RX_WIN_OPEN
> + * or VAS_TX_WIN_OPEN ioctl.
> + */
> +struct nxftw_instance {
> + int instance;
> + bool tx_win;
> + struct vas_window *window;
> +};
> +
> +#define VAS_DEFAULT_VAS_ID   0
> +#define POWERNV_LPID 0   /* TODO: For VM/KVM guests? */

mfspr(SPRN_LPID)

would seem to do the trick?

> +static char *nxftw_devnode(struct device *dev, umode_t *mode)
> +{
> + return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));

This isn't a crypto device?

> +}
> +
> +static int nxftw_open(struct inode *inode, struct file *fp)
> +{
> + int minor;
> + struct nxftw_instance *nxti;

instance would be a better name.

> + minor = MINOR(inode->i_rdev);

Not used?

> + nxti = kzalloc(sizeof(*nxti), GFP_KERNEL);
> + if (!nxti)
> + return -ENOMEM;
> +
> + nxti->instance = atomic_inc_return(_instid);

And this would read better if the variable was "id". eg.

instance->id = atomic_inc_return(_instance_id);

> + nxti->window = NULL;
> +
> + fp->private_data = nxti;
> + return 0;
> +}
> +
> +static int validate_txwin_user_attr(struct vas_tx_win_open_attr *uattr)
> +{
> + int i;
> +
> + if (uattr->version != 1)
> + return -EINVAL;
> +
> + if (uattr->flags & ~VAS_FLAGS_HIGH_PRI)
> + return -EINVAL;
> +
> + if (uattr->reserved1 || uattr->reserved2)
> + return -EINVAL;
> +
> + 

Re: [PATCH v6 16/17] powerpc/vas: Implement a simple FTW driver

[Michael Ellerman]

Hi Suka,

Some comments inline ...


Sukadev Bhattiprolu  writes:

> The Fast Thread Wake-up (FTW) driver provides user space applications an
> interface to the Core-to-Core functionality in POWER9. The driver provides
> the device node/ioctl API to applications and uses the external interfaces
> to the VAS driver to interact with the VAS hardware.
>
> A follow-on patch provides detailed description of the API for the driver.
>
> Signed-off-by: Sukadev Bhattiprolu 
> ---
>  MAINTAINERS |   1 +
>  arch/powerpc/platforms/powernv/Kconfig  |  16 ++
>  arch/powerpc/platforms/powernv/Makefile |   1 +
>  arch/powerpc/platforms/powernv/nx-ftw.c | 486 
> 

AFAICS this has nothing to do with NX, so why is it called nx-ftw ?

Also aren't we going to want to use this on pseries eventually? If so
should it go in arch/powerpc/sysdev ?

> diff --git a/arch/powerpc/platforms/powernv/Makefile 
> b/arch/powerpc/platforms/powernv/Makefile
> index e4db292..dc60046 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_MEMORY_FAILURE)+= opal-memory-errors.o
>  obj-$(CONFIG_TRACEPOINTS)+= opal-tracepoints.o
>  obj-$(CONFIG_OPAL_PRD)   += opal-prd.o
>  obj-$(CONFIG_PPC_VAS)+= vas.o vas-window.o
> +obj-$(CONFIG_PPC_FTW)+= nx-ftw.o
> diff --git a/arch/powerpc/platforms/powernv/nx-ftw.c 
> b/arch/powerpc/platforms/powernv/nx-ftw.c
> new file mode 100644
> index 000..a0b6388
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/nx-ftw.c
> @@ -0,0 +1,486 @@

Missing license header.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please try and trim the list to what you need.

> +
> +/*
> + * NX-FTW is a device driver used to provide user space access to the
> + * Core-to-Core aka Fast Thread Wakeup (FTW) functionality provided by
> + * the Virtual Accelerator Subsystem (VAS) in POWER9 systems. See also
> + * arch/powerpc/platforms/powernv/vas*.
> + *
> + * The driver creates the device node /dev/crypto/nx-ftw that can be
> + * used as follows:
> + *
> + *   fd = open("/dev/crypto/nx-ftw", O_RDWR);
> + *   rc = ioctl(fd, VAS_RX_WIN_OPEN, );
> + *   rc = ioctl(fd, VAS_TX_WIN_OPEN, );
> + *   paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
> + *   vas_copy(, 0, 1);
> + *   vas_paste(paste_addr, 0, 1);
> + *
> + * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
> + */
> +
> +static char  *nxftw_dev_name = "nx-ftw";
> +static atomic_t  nxftw_instid = ATOMIC_INIT(0);
> +static dev_t nxftw_devt;
> +static struct dentry *nxftw_debugfs;
> +static struct class  *nxftw_dbgfs_class;

The class doesn't go in debugfs, which is what "dbgfs" says to me.

> +/*
> + * Wrapper object for the nx-ftw device node - there is just one

Just "device".

"device node" is ambiguous vs device tree.

> + * instance of this node for the whole system.

So why not put the globals above in here also?

> + */
> +struct nxftw_dev {
> + struct cdev cdev;
> + struct device *device;
> + char *name;
> + atomic_t refcount;
> +} nxftw_device;
> +
> +/*
> + * One instance per open of a nx-ftw device. Each nxftw_instance is
> + * associated with a VAS window, after the caller issues VAS_RX_WIN_OPEN
> + * or VAS_TX_WIN_OPEN ioctl.
> + */
> +struct nxftw_instance {
> + int instance;
> + bool tx_win;
> + struct vas_window *window;
> +};
> +
> +#define VAS_DEFAULT_VAS_ID   0
> +#define POWERNV_LPID 0   /* TODO: For VM/KVM guests? */

mfspr(SPRN_LPID)

would seem to do the trick?

> +static char *nxftw_devnode(struct device *dev, umode_t *mode)
> +{
> + return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));

This isn't a crypto device?

> +}
> +
> +static int nxftw_open(struct inode *inode, struct file *fp)
> +{
> + int minor;
> + struct nxftw_instance *nxti;

instance would be a better name.

> + minor = MINOR(inode->i_rdev);

Not used?

> + nxti = kzalloc(sizeof(*nxti), GFP_KERNEL);
> + if (!nxti)
> + return -ENOMEM;
> +
> + nxti->instance = atomic_inc_return(_instid);

And this would read better if the variable was "id". eg.

instance->id = atomic_inc_return(_instance_id);

> + nxti->window = NULL;
> +
> + fp->private_data = nxti;
> + return 0;
> +}
> +
> +static int validate_txwin_user_attr(struct vas_tx_win_open_attr *uattr)
> +{
> + int i;
> +
> + if (uattr->version != 1)
> + return -EINVAL;
> +
> + if (uattr->flags & ~VAS_FLAGS_HIGH_PRI)
> + return -EINVAL;
> +
> + if (uattr->reserved1 || uattr->reserved2)
> + return -EINVAL;
> +
> + for (i = 0; i < sizeof(uattr->reserved3) / 

[PATCH v6 16/17] powerpc/vas: Implement a simple FTW driver

[Sukadev Bhattiprolu]

The Fast Thread Wake-up (FTW) driver provides user space applications an
interface to the Core-to-Core functionality in POWER9. The driver provides
the device node/ioctl API to applications and uses the external interfaces
to the VAS driver to interact with the VAS hardware.

A follow-on patch provides detailed description of the API for the driver.

Signed-off-by: Sukadev Bhattiprolu 
---
 MAINTAINERS |   1 +
 arch/powerpc/platforms/powernv/Kconfig  |  16 ++
 arch/powerpc/platforms/powernv/Makefile |   1 +
 arch/powerpc/platforms/powernv/nx-ftw.c | 486 
 4 files changed, 504 insertions(+)
 create mode 100644 arch/powerpc/platforms/powernv/nx-ftw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c3f156c..a45c0c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6431,6 +6431,7 @@ L:linuxppc-...@lists.ozlabs.org
 S: Supported
 F: arch/powerpc/platforms/powernv/vas*
 F: arch/powerpc/platforms/powernv/copy-paste.h
+F: arch/powerpc/platforms/powernv/nx-ftw*
 F: arch/powerpc/include/asm/vas.h
 F: arch/powerpc/include/uapi/asm/vas.h
 
diff --git a/arch/powerpc/platforms/powernv/Kconfig 
b/arch/powerpc/platforms/powernv/Kconfig
index f565454..67ea0ff 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -44,3 +44,19 @@ config PPC_VAS
  VAS adapters are found in POWER9 based systems.
 
  If unsure, say N.
+
+config PPC_FTW
+   bool "IBM Fast Thread-Wakeup (FTW)"
+   depends on PPC_VAS
+   default n
+   help
+ This enables support for IBM Fast Thread-Wakeup driver.
+
+ The FTW driver allows applications to utilize a low overhead
+ core-to-core wake up mechansim in the IBM Virtual Accelerator
+ Switchboard (VAS) to improve performance.
+
+ VAS adapters are found in POWER9 based systems and are required
+ for the FTW driver to be operational.
+
+ If unsure, say N.
diff --git a/arch/powerpc/platforms/powernv/Makefile 
b/arch/powerpc/platforms/powernv/Makefile
index e4db292..dc60046 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_MEMORY_FAILURE)  += opal-memory-errors.o
 obj-$(CONFIG_TRACEPOINTS)  += opal-tracepoints.o
 obj-$(CONFIG_OPAL_PRD) += opal-prd.o
 obj-$(CONFIG_PPC_VAS)  += vas.o vas-window.o
+obj-$(CONFIG_PPC_FTW)  += nx-ftw.o
diff --git a/arch/powerpc/platforms/powernv/nx-ftw.c 
b/arch/powerpc/platforms/powernv/nx-ftw.c
new file mode 100644
index 000..a0b6388
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/nx-ftw.c
@@ -0,0 +1,486 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * NX-FTW is a device driver used to provide user space access to the
+ * Core-to-Core aka Fast Thread Wakeup (FTW) functionality provided by
+ * the Virtual Accelerator Subsystem (VAS) in POWER9 systems. See also
+ * arch/powerpc/platforms/powernv/vas*.
+ *
+ * The driver creates the device node /dev/crypto/nx-ftw that can be
+ * used as follows:
+ *
+ * fd = open("/dev/crypto/nx-ftw", O_RDWR);
+ * rc = ioctl(fd, VAS_RX_WIN_OPEN, );
+ * rc = ioctl(fd, VAS_TX_WIN_OPEN, );
+ * paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
+ * vas_copy(, 0, 1);
+ * vas_paste(paste_addr, 0, 1);
+ *
+ * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
+ */
+
+static char*nxftw_dev_name = "nx-ftw";
+static atomic_tnxftw_instid = ATOMIC_INIT(0);
+static dev_t   nxftw_devt;
+static struct dentry   *nxftw_debugfs;
+static struct class*nxftw_dbgfs_class;
+
+/*
+ * Wrapper object for the nx-ftw device node - there is just one
+ * instance of this node for the whole system.
+ */
+struct nxftw_dev {
+   struct cdev cdev;
+   struct device *device;
+   char *name;
+   atomic_t refcount;
+} nxftw_device;
+
+/*
+ * One instance per open of a nx-ftw device. Each nxftw_instance is
+ * associated with a VAS window, after the caller issues VAS_RX_WIN_OPEN
+ * or VAS_TX_WIN_OPEN ioctl.
+ */
+struct nxftw_instance {
+   int instance;
+   bool tx_win;
+   struct vas_window *window;
+};
+
+#define VAS_DEFAULT_VAS_ID 0
+#define POWERNV_LPID   0   /* TODO: For VM/KVM guests? */
+
+static char *nxftw_devnode(struct device *dev, umode_t *mode)
+{
+   return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));
+}
+
+static int nxftw_open(struct inode *inode, struct file *fp)
+{
+   int minor;
+   struct nxftw_instance *nxti;
+
+   minor = MINOR(inode->i_rdev);
+
+   nxti = kzalloc(sizeof(*nxti), GFP_KERNEL);
+   if (!nxti)
+   return -ENOMEM;
+
+   nxti->instance = atomic_inc_return(_instid);
+   

[PATCH v6 16/17] powerpc/vas: Implement a simple FTW driver

[Sukadev Bhattiprolu]

The Fast Thread Wake-up (FTW) driver provides user space applications an
interface to the Core-to-Core functionality in POWER9. The driver provides
the device node/ioctl API to applications and uses the external interfaces
to the VAS driver to interact with the VAS hardware.

A follow-on patch provides detailed description of the API for the driver.

Signed-off-by: Sukadev Bhattiprolu 
---
 MAINTAINERS |   1 +
 arch/powerpc/platforms/powernv/Kconfig  |  16 ++
 arch/powerpc/platforms/powernv/Makefile |   1 +
 arch/powerpc/platforms/powernv/nx-ftw.c | 486 
 4 files changed, 504 insertions(+)
 create mode 100644 arch/powerpc/platforms/powernv/nx-ftw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c3f156c..a45c0c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6431,6 +6431,7 @@ L:linuxppc-...@lists.ozlabs.org
 S: Supported
 F: arch/powerpc/platforms/powernv/vas*
 F: arch/powerpc/platforms/powernv/copy-paste.h
+F: arch/powerpc/platforms/powernv/nx-ftw*
 F: arch/powerpc/include/asm/vas.h
 F: arch/powerpc/include/uapi/asm/vas.h
 
diff --git a/arch/powerpc/platforms/powernv/Kconfig 
b/arch/powerpc/platforms/powernv/Kconfig
index f565454..67ea0ff 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -44,3 +44,19 @@ config PPC_VAS
  VAS adapters are found in POWER9 based systems.
 
  If unsure, say N.
+
+config PPC_FTW
+   bool "IBM Fast Thread-Wakeup (FTW)"
+   depends on PPC_VAS
+   default n
+   help
+ This enables support for IBM Fast Thread-Wakeup driver.
+
+ The FTW driver allows applications to utilize a low overhead
+ core-to-core wake up mechansim in the IBM Virtual Accelerator
+ Switchboard (VAS) to improve performance.
+
+ VAS adapters are found in POWER9 based systems and are required
+ for the FTW driver to be operational.
+
+ If unsure, say N.
diff --git a/arch/powerpc/platforms/powernv/Makefile 
b/arch/powerpc/platforms/powernv/Makefile
index e4db292..dc60046 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_MEMORY_FAILURE)  += opal-memory-errors.o
 obj-$(CONFIG_TRACEPOINTS)  += opal-tracepoints.o
 obj-$(CONFIG_OPAL_PRD) += opal-prd.o
 obj-$(CONFIG_PPC_VAS)  += vas.o vas-window.o
+obj-$(CONFIG_PPC_FTW)  += nx-ftw.o
diff --git a/arch/powerpc/platforms/powernv/nx-ftw.c 
b/arch/powerpc/platforms/powernv/nx-ftw.c
new file mode 100644
index 000..a0b6388
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/nx-ftw.c
@@ -0,0 +1,486 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * NX-FTW is a device driver used to provide user space access to the
+ * Core-to-Core aka Fast Thread Wakeup (FTW) functionality provided by
+ * the Virtual Accelerator Subsystem (VAS) in POWER9 systems. See also
+ * arch/powerpc/platforms/powernv/vas*.
+ *
+ * The driver creates the device node /dev/crypto/nx-ftw that can be
+ * used as follows:
+ *
+ * fd = open("/dev/crypto/nx-ftw", O_RDWR);
+ * rc = ioctl(fd, VAS_RX_WIN_OPEN, );
+ * rc = ioctl(fd, VAS_TX_WIN_OPEN, );
+ * paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
+ * vas_copy(, 0, 1);
+ * vas_paste(paste_addr, 0, 1);
+ *
+ * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
+ */
+
+static char*nxftw_dev_name = "nx-ftw";
+static atomic_tnxftw_instid = ATOMIC_INIT(0);
+static dev_t   nxftw_devt;
+static struct dentry   *nxftw_debugfs;
+static struct class*nxftw_dbgfs_class;
+
+/*
+ * Wrapper object for the nx-ftw device node - there is just one
+ * instance of this node for the whole system.
+ */
+struct nxftw_dev {
+   struct cdev cdev;
+   struct device *device;
+   char *name;
+   atomic_t refcount;
+} nxftw_device;
+
+/*
+ * One instance per open of a nx-ftw device. Each nxftw_instance is
+ * associated with a VAS window, after the caller issues VAS_RX_WIN_OPEN
+ * or VAS_TX_WIN_OPEN ioctl.
+ */
+struct nxftw_instance {
+   int instance;
+   bool tx_win;
+   struct vas_window *window;
+};
+
+#define VAS_DEFAULT_VAS_ID 0
+#define POWERNV_LPID   0   /* TODO: For VM/KVM guests? */
+
+static char *nxftw_devnode(struct device *dev, umode_t *mode)
+{
+   return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));
+}
+
+static int nxftw_open(struct inode *inode, struct file *fp)
+{
+   int minor;
+   struct nxftw_instance *nxti;
+
+   minor = MINOR(inode->i_rdev);
+
+   nxti = kzalloc(sizeof(*nxti), GFP_KERNEL);
+   if (!nxti)
+   return -ENOMEM;
+
+   nxti->instance = atomic_inc_return(_instid);
+   nxti->window = NULL;
+
+