linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/14] staging: gasket: assorted cleanups
@ 2018-07-21 12:56 Todd Poynor
  2018-07-21 12:56 ` [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig Todd Poynor
                   ` (34 more replies)
  0 siblings, 35 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Various fixups arising from the chromium review of the gasket and apex
drivers.

Todd Poynor (14):
  staging: gasket: fix check_and_invoke_callback log param
  staging: gasket: remove duplicate call to retrieve device callback
  staging: gasket: gasket_handle_ioctl fix ioctl exit trace param
  staging: gasket: avoid copy to user on error in coherent alloc config
  staging: gasket: print mmap starting address as unsigned long
  staging: gasket: remove unnecessary NULL checks on calls from VFS
  staging: gasket: gasket_get_device drop check for NULL pci_dev
  staging: gasket: apex return error on sysfs show of missing attribute
  staging: gasket: core: convert various logs to debug level
  staging: gasket: interrupts: convert various logs to debug level
  staging: gasket: ioctl common: convert various logs to debug level
  staging: gasket: page table: convert various logs to debug level
  staging: gasket: page table: remove unnecessary logs
  staging: gasket: apex: convert various logs to debug level

 drivers/staging/gasket/apex_driver.c       | 14 ++---
 drivers/staging/gasket/gasket_core.c       | 63 +++++++++-------------
 drivers/staging/gasket/gasket_interrupt.c  | 24 ++++-----
 drivers/staging/gasket/gasket_ioctl.c      | 19 ++++---
 drivers/staging/gasket/gasket_page_table.c | 35 ++++--------
 5 files changed, 65 insertions(+), 90 deletions(-)

-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param Todd Poynor
                   ` (33 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

The gasket and apex drivers are also to be used on ARM64 architectures.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/Kconfig b/drivers/staging/gasket/Kconfig
index c836389c1402d..970e299046c37 100644
--- a/drivers/staging/gasket/Kconfig
+++ b/drivers/staging/gasket/Kconfig
@@ -2,7 +2,7 @@ menu "Gasket devices"
 
 config STAGING_GASKET_FRAMEWORK
 	tristate "Gasket framework"
-	depends on PCI && X86_64
+	depends on PCI && (X86_64 || ARM64)
 	help
 	  This framework supports Gasket-compatible devices, such as Apex.
 	  It is required for any of the following module(s).
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
  2018-07-21 12:56 ` [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 02/20] staging: gasket: gasket_enable_dev remove unnecessary variable Todd Poynor
                   ` (32 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

The message should be passed the callback function pointer, not
the pointer to the gasket device.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 40e46ca5228c8..2cd232230845c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -205,7 +205,7 @@ static inline int check_and_invoke_callback(
 {
 	int ret = 0;
 
-	gasket_nodev_error("check_and_invoke_callback %p", gasket_dev);
+	gasket_nodev_error("check_and_invoke_callback %p", cb_function);
 	if (cb_function) {
 		mutex_lock(&gasket_dev->mutex);
 		ret = cb_function(gasket_dev);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/20] staging: gasket: gasket_enable_dev remove unnecessary variable
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
  2018-07-21 12:56 ` [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig Todd Poynor
  2018-07-21 12:56 ` [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback Todd Poynor
                   ` (31 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove unnecessary variable, pass constant param instead.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 0d5ba7359af73..f327c9d7f90a3 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -898,7 +898,6 @@ static int gasket_enable_dev(
 {
 	int tbl_idx;
 	int ret;
-	bool has_dma_ops;
 	struct device *ddev;
 	const struct gasket_driver_desc *driver_desc =
 		internal_desc->driver_desc;
@@ -917,8 +916,6 @@ static int gasket_enable_dev(
 		return ret;
 	}
 
-	has_dma_ops = true;
-
 	for (tbl_idx = 0; tbl_idx < driver_desc->num_page_tables; tbl_idx++) {
 		gasket_log_debug(
 			gasket_dev, "Initializing page table %d.", tbl_idx);
@@ -936,7 +933,7 @@ static int gasket_enable_dev(
 			&gasket_dev->bar_data[
 				driver_desc->page_table_bar_index],
 			&driver_desc->page_table_configs[tbl_idx],
-			ddev, gasket_dev->pci_dev, has_dma_ops);
+			ddev, gasket_dev->pci_dev, true);
 		if (ret) {
 			gasket_log_error(
 				gasket_dev,
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (2 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 02/20] staging: gasket: gasket_enable_dev remove unnecessary variable Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param Todd Poynor
                   ` (30 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

gasket_handle_ioctl() calls gasket_get_ioctl_permissions_cb() twice;
simplify the code and avoid duplicated work by fetching the callback
pointer only once.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 2e2c9b997093b..dbe9fdef0c268 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -55,14 +55,15 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 {
 	struct gasket_dev *gasket_dev;
 	unsigned long arg = (unsigned long)argp;
+	gasket_ioctl_permissions_cb_t ioctl_permissions_cb;
 	int retval;
 
 	gasket_dev = (struct gasket_dev *)filp->private_data;
 	trace_gasket_ioctl_entry(gasket_dev->dev_info.name, cmd);
 
-	if (gasket_get_ioctl_permissions_cb(gasket_dev)) {
-		retval = gasket_get_ioctl_permissions_cb(gasket_dev)(
-			filp, cmd, argp);
+	ioctl_permissions_cb = gasket_get_ioctl_permissions_cb(gasket_dev);
+	if (ioctl_permissions_cb) {
+		retval = ioctl_permissions_cb(filp, cmd, argp);
 		if (retval < 0) {
 			trace_gasket_ioctl_exit(-EPERM);
 			return retval;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (3 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 03/20] staging: gasket: remove code for no physical device Todd Poynor
                   ` (29 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Pass the return value from the device ioctl permissions callback to the
tracepoint when the callback returns an error.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index dbe9fdef0c268..1b164ac7a0496 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -65,7 +65,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 	if (ioctl_permissions_cb) {
 		retval = ioctl_permissions_cb(filp, cmd, argp);
 		if (retval < 0) {
-			trace_gasket_ioctl_exit(-EPERM);
+			trace_gasket_ioctl_exit(retval);
 			return retval;
 		} else if (retval == 0) {
 			trace_gasket_ioctl_exit(-EPERM);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/20] staging: gasket: remove code for no physical device
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (4 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config Todd Poynor
                   ` (28 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

gasket_enable_dev code for enabling a gasket device with no physical PCI
device registered shouldn't be necessary.

Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index f327c9d7f90a3..18cc8e3283b39 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -898,7 +898,6 @@ static int gasket_enable_dev(
 {
 	int tbl_idx;
 	int ret;
-	struct device *ddev;
 	const struct gasket_driver_desc *driver_desc =
 		internal_desc->driver_desc;
 
@@ -919,21 +918,12 @@ static int gasket_enable_dev(
 	for (tbl_idx = 0; tbl_idx < driver_desc->num_page_tables; tbl_idx++) {
 		gasket_log_debug(
 			gasket_dev, "Initializing page table %d.", tbl_idx);
-		if (gasket_dev->pci_dev) {
-			ddev = &gasket_dev->pci_dev->dev;
-		} else {
-			gasket_log_error(
-				gasket_dev,
-				"%s with no physical device!!", __func__);
-			WARN_ON(1);
-			ddev = NULL;
-		}
 		ret = gasket_page_table_init(
 			&gasket_dev->page_table[tbl_idx],
 			&gasket_dev->bar_data[
 				driver_desc->page_table_bar_index],
 			&driver_desc->page_table_configs[tbl_idx],
-			ddev, gasket_dev->pci_dev, true);
+			&gasket_dev->pci_dev->dev, gasket_dev->pci_dev, true);
 		if (ret) {
 			gasket_log_error(
 				gasket_dev,
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (5 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 03/20] staging: gasket: remove code for no physical device Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 04/20] staging: gasket: fix class create bug handling Todd Poynor
                   ` (27 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

gasket_config_coherent_allocator() on error return the error to caller
without copying a possibly-update DMA address back to userspace.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 1b164ac7a0496..8cf094b90cdb0 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -441,8 +441,10 @@ static int gasket_config_coherent_allocator(
 			gasket_dev, ibuf.size, &ibuf.dma_address,
 			ibuf.page_table_index);
 	}
+	if (ret)
+		return ret;
 	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
-	return ret;
+	return 0;
 }
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/20] staging: gasket: fix class create bug handling
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (6 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long Todd Poynor
                   ` (26 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

class_create() never returns NULL, and this driver should never return
PTR_ERR(NULL) anyway.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
---
 drivers/staging/gasket/gasket_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 18cc8e3283b39..53236e1ba4e48 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -321,7 +321,7 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
 	internal->class =
 		class_create(driver_desc->module, driver_desc->name);
 
-	if (IS_ERR_OR_NULL(internal->class)) {
+	if (IS_ERR(internal->class)) {
 		gasket_nodev_error("Cannot register %s class [ret=%ld]",
 				   driver_desc->name, PTR_ERR(internal->class));
 		ret = PTR_ERR(internal->class);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (7 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 04/20] staging: gasket: fix class create bug handling Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 05/20] staging: gasket: remove unnecessary code in coherent allocator Todd Poynor
                   ` (25 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Page alignment error log should print the offending value as an unsigned
long, not as a kernel pointer.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 2cd232230845c..11ab049854493 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1596,8 +1596,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 
 	if (vma->vm_start & ~PAGE_MASK) {
 		gasket_log_error(
-			gasket_dev, "Base address not page-aligned: 0x%p\n",
-			(void *)vma->vm_start);
+			gasket_dev, "Base address not page-aligned: 0x%lx\n",
+			vma->vm_start);
 		trace_gasket_mmap_exit(-EINVAL);
 		return -EINVAL;
 	}
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/20] staging: gasket: remove unnecessary code in coherent allocator
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (8 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 06/20] staging: gasket: don't treat no device reset callback as an error Todd Poynor
                   ` (24 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove extraneous statement in gasket_config_coherent_allocator()

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 0c2f85cf54480..d0142ed048a65 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -420,10 +420,8 @@ static int gasket_config_coherent_allocator(
 	if (ibuf.page_table_index >= gasket_dev->num_page_tables)
 		return -EFAULT;
 
-	if (ibuf.size > PAGE_SIZE * MAX_NUM_COHERENT_PAGES) {
-		ibuf.size = PAGE_SIZE * MAX_NUM_COHERENT_PAGES;
+	if (ibuf.size > PAGE_SIZE * MAX_NUM_COHERENT_PAGES)
 		return -ENOMEM;
-	}
 
 	if (ibuf.enable == 0) {
 		ret = gasket_free_coherent_memory(
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/20] staging: gasket: don't treat no device reset callback as an error
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (9 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 05/20] staging: gasket: remove unnecessary code in coherent allocator Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS Todd Poynor
                   ` (23 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

It is not an error for a device to not have a reset callback registered.

Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 53236e1ba4e48..eb5ad161ccda2 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1836,11 +1836,8 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
 	const struct gasket_driver_desc *driver_desc;
 
 	driver_desc = gasket_dev->internal_desc->driver_desc;
-	if (!driver_desc->device_reset_cb) {
-		gasket_log_error(
-			gasket_dev, "No device reset callback was registered.");
-		return -EINVAL;
-	}
+	if (!driver_desc->device_reset_cb)
+		return 0;
 
 	/* Perform a device reset of the requested type. */
 	ret = driver_desc->device_reset_cb(gasket_dev, reset_type);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (10 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 06/20] staging: gasket: don't treat no device reset callback as an error Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev Todd Poynor
                   ` (22 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Remove unneeded checks for NULL pointers in struct file pointers passed
from the VFS layer or the private_data that must have been properly set
at file open time.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 11ab049854493..e82f8ce39c9fd 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1587,11 +1587,6 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 	int num_map_regions = 0;
 	enum do_map_region_status map_status;
 
-	if (!gasket_dev) {
-		gasket_nodev_error("Unable to retrieve device data");
-		trace_gasket_mmap_exit(-EINVAL);
-		return -EINVAL;
-	}
 	driver_desc = gasket_dev->internal_desc->driver_desc;
 
 	if (vma->vm_start & ~PAGE_MASK) {
@@ -1785,17 +1780,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 	void __user *argp = (void __user *)arg;
 	char path[256];
 
-	if (!filp)
-		return -ENODEV;
-
 	gasket_dev = (struct gasket_dev *)filp->private_data;
-	if (!gasket_dev) {
-		gasket_nodev_error(
-			"Unable to find Gasket structure for file %s",
-			d_path(&filp->f_path, path, 256));
-		return -ENODEV;
-	}
-
 	driver_desc = gasket_dev->internal_desc->driver_desc;
 	if (!driver_desc) {
 		gasket_log_error(
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (11 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 07/20] staging: gasket: gasket_mmap return error instead of valid BAR index Todd Poynor
                   ` (21 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

The pci_dev field of a struct gasket_dev can never be NULL, there's no
need to check for this in gasket_get_device().

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index e82f8ce39c9fd..8265d543623d7 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -2026,9 +2026,7 @@ const struct gasket_driver_desc *gasket_get_driver_desc(struct gasket_dev *dev)
  */
 struct device *gasket_get_device(struct gasket_dev *dev)
 {
-	if (dev->pci_dev)
-		return &dev->pci_dev->dev;
-	return NULL;
+	return &dev->pci_dev->dev;
 }
 
 /**
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/20] staging: gasket: gasket_mmap return error instead of valid BAR index
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (12 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute Todd Poynor
                   ` (20 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

When offset to be mapped matches both a BAR region and a coherent mapped
region return an error as intended, not the BAR index.

Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index eb5ad161ccda2..3cf918f9d2604 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1627,7 +1627,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 			"0x%lx",
 			raw_offset);
 		trace_gasket_mmap_exit(bar_index);
-		return bar_index;
+		return -EINVAL;
 	}
 
 	vma->vm_private_data = gasket_dev;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (13 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 07/20] staging: gasket: gasket_mmap return error instead of valid BAR index Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 08/20] staging: gasket: apex_clock_gating simplify logic, reduce indentation Todd Poynor
                   ` (19 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Apex sysfs show function return -ENODEV if the attribute is not present,
rather than silently failing from the standpoint of the userspace
accessor.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 3e76c4db5db2e..1c6f73b5a2a9e 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -710,14 +710,14 @@ static ssize_t sysfs_show(
 	gasket_dev = gasket_sysfs_get_device_data(device);
 	if (!gasket_dev) {
 		gasket_nodev_error("No Apex device sysfs mapping found");
-		return 0;
+		return -ENODEV;
 	}
 
 	gasket_attr = gasket_sysfs_get_attr(device, attr);
 	if (!gasket_attr) {
 		gasket_nodev_error("No Apex device sysfs attr data found");
 		gasket_sysfs_put_device_data(device, gasket_dev);
-		return 0;
+		return -ENODEV;
 	}
 
 	type = (enum sysfs_attribute_type)gasket_sysfs_get_attr(device, attr);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/20] staging: gasket: apex_clock_gating simplify logic, reduce indentation
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (14 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 09/14] staging: gasket: core: convert various logs to debug level Todd Poynor
                   ` (18 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Collapse together two checks and return immediately, avoid conditional
indentation for most of function code.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 43 +++++++++++++---------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 3a83c3d4d5561..a01b1f2b827ea 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -666,33 +666,30 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev, ulong arg)
 {
 	struct apex_gate_clock_ioctl ibuf;
 
-	if (bypass_top_level)
+	if (bypass_top_level || !allow_sw_clock_gating)
 		return 0;
 
-	if (allow_sw_clock_gating) {
-		if (copy_from_user(&ibuf, (void __user *)arg, sizeof(ibuf)))
-			return -EFAULT;
+	if (copy_from_user(&ibuf, (void __user *)arg, sizeof(ibuf)))
+		return -EFAULT;
 
-		gasket_log_error(
-			gasket_dev, "%s %llu", __func__, ibuf.enable);
+	gasket_log_error(gasket_dev, "%s %llu", __func__, ibuf.enable);
 
-		if (ibuf.enable) {
-			/* Quiesce AXI, gate GCB clock. */
-			gasket_read_modify_write_32(
-				gasket_dev, APEX_BAR_INDEX,
-				APEX_BAR2_REG_AXI_QUIESCE, 0x1, 1, 16);
-			gasket_read_modify_write_32(
-				gasket_dev, APEX_BAR_INDEX,
-				APEX_BAR2_REG_GCB_CLOCK_GATE, 0x1, 2, 18);
-		} else {
-			/* Un-gate GCB clock, un-quiesce AXI. */
-			gasket_read_modify_write_32(
-				gasket_dev, APEX_BAR_INDEX,
-				APEX_BAR2_REG_GCB_CLOCK_GATE, 0x0, 2, 18);
-			gasket_read_modify_write_32(
-				gasket_dev, APEX_BAR_INDEX,
-				APEX_BAR2_REG_AXI_QUIESCE, 0x0, 1, 16);
-		}
+	if (ibuf.enable) {
+		/* Quiesce AXI, gate GCB clock. */
+		gasket_read_modify_write_32(
+		    gasket_dev, APEX_BAR_INDEX,
+		    APEX_BAR2_REG_AXI_QUIESCE, 0x1, 1, 16);
+		gasket_read_modify_write_32(
+		    gasket_dev, APEX_BAR_INDEX,
+		    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x1, 2, 18);
+	} else {
+		/* Un-gate GCB clock, un-quiesce AXI. */
+		gasket_read_modify_write_32(
+		    gasket_dev, APEX_BAR_INDEX,
+		    APEX_BAR2_REG_GCB_CLOCK_GATE, 0x0, 2, 18);
+		gasket_read_modify_write_32(
+		    gasket_dev, APEX_BAR_INDEX,
+		    APEX_BAR2_REG_AXI_QUIESCE, 0x0, 1, 16);
 	}
 	return 0;
 }
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/14] staging: gasket: core: convert various logs to debug level
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (15 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 08/20] staging: gasket: apex_clock_gating simplify logic, reduce indentation Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 09/20] staging: gasket: gasket page table functions use bool return type Todd Poynor
                   ` (17 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs.  Convert these to debug log level.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 38 +++++++++++++++-------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 8265d543623d7..1d04fd0990e45 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -205,7 +205,8 @@ static inline int check_and_invoke_callback(
 {
 	int ret = 0;
 
-	gasket_nodev_error("check_and_invoke_callback %p", cb_function);
+	gasket_log_debug(gasket_dev, "check_and_invoke_callback %p",
+			 cb_function);
 	if (cb_function) {
 		mutex_lock(&gasket_dev->mutex);
 		ret = cb_function(gasket_dev);
@@ -227,7 +228,7 @@ static inline int gasket_check_and_invoke_callback_nolock(
 	int ret = 0;
 
 	if (cb_function) {
-		gasket_log_info(
+		gasket_log_debug(
 			gasket_dev, "Invoking device-specific callback.");
 		ret = cb_function(gasket_dev);
 	}
@@ -1177,7 +1178,7 @@ static int gasket_release(struct inode *inode, struct file *file)
 	if (file->f_mode & FMODE_WRITE) {
 		ownership->write_open_count--;
 		if (ownership->write_open_count == 0) {
-			gasket_log_info(gasket_dev, "Device is now free");
+			gasket_log_debug(gasket_dev, "Device is now free");
 			ownership->is_owned = 0;
 			ownership->owner = 0;
 
@@ -1198,7 +1199,7 @@ static int gasket_release(struct inode *inode, struct file *file)
 		}
 	}
 
-	gasket_log_info(
+	gasket_log_debug(
 		gasket_dev, "New open count (owning tgid %u): %d",
 		ownership->owner, ownership->write_open_count);
 	mutex_unlock(&gasket_dev->mutex);
@@ -1225,7 +1226,7 @@ static bool gasket_mmap_has_permissions(
 
 	/* Never allow non-sysadmins to access to a dead device. */
 	if (gasket_dev->status != GASKET_STATUS_ALIVE) {
-		gasket_log_info(gasket_dev, "Device is dead.");
+		gasket_log_debug(gasket_dev, "Device is dead.");
 		return false;
 	}
 
@@ -1233,7 +1234,7 @@ static bool gasket_mmap_has_permissions(
 	requested_permissions =
 		(vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
 	if (requested_permissions & ~(bar_permissions)) {
-		gasket_log_info(
+		gasket_log_debug(
 			gasket_dev,
 			"Attempting to map a region with requested permissions "
 			"0x%x, but region has permissions 0x%x.",
@@ -1244,7 +1245,7 @@ static bool gasket_mmap_has_permissions(
 	/* Do not allow a non-owner to write. */
 	if ((vma->vm_flags & VM_WRITE) &&
 	    !gasket_owned_by_current_tgid(&gasket_dev->dev_info)) {
-		gasket_log_info(
+		gasket_log_debug(
 			gasket_dev,
 			"Attempting to mmap a region for write without owning "
 			"device.");
@@ -1736,15 +1737,16 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
 	status = gasket_check_and_invoke_callback_nolock(
 		gasket_dev, driver_desc->device_status_cb);
 	if (status != GASKET_STATUS_ALIVE) {
-		gasket_log_info(gasket_dev, "Hardware reported status %d.",
-				status);
+		gasket_log_debug(gasket_dev, "Hardware reported status %d.",
+				 status);
 		return status;
 	}
 
 	status = gasket_interrupt_system_status(gasket_dev);
 	if (status != GASKET_STATUS_ALIVE) {
-		gasket_log_info(gasket_dev,
-				"Interrupt system reported status %d.", status);
+		gasket_log_debug(gasket_dev,
+				 "Interrupt system reported status %d.",
+				 status);
 		return status;
 	}
 
@@ -1752,7 +1754,7 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
 		status = gasket_page_table_system_status(
 			gasket_dev->page_table[i]);
 		if (status != GASKET_STATUS_ALIVE) {
-			gasket_log_info(
+			gasket_log_debug(
 				gasket_dev, "Page table %d reported status %d.",
 				i, status);
 			return status;
@@ -1783,7 +1785,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 	gasket_dev = (struct gasket_dev *)filp->private_data;
 	driver_desc = gasket_dev->internal_desc->driver_desc;
 	if (!driver_desc) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev,
 			"Unable to find device descriptor for file %s",
 			d_path(&filp->f_path, path, 256));
@@ -1831,7 +1833,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
 	/* Perform a device reset of the requested type. */
 	ret = driver_desc->device_reset_cb(gasket_dev, reset_type);
 	if (ret) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Device reset cb returned %d.", ret);
 		return ret;
 	}
@@ -1842,7 +1844,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
 
 	ret = gasket_interrupt_reinit(gasket_dev);
 	if (ret) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Unable to reinit interrupts: %d.", ret);
 		return ret;
 	}
@@ -1850,7 +1852,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
 	/* Get current device health. */
 	gasket_dev->status = gasket_get_hw_status(gasket_dev);
 	if (gasket_dev->status == GASKET_STATUS_DEAD) {
-		gasket_log_error(gasket_dev, "Device reported as dead.");
+		gasket_log_debug(gasket_dev, "Device reported as dead.");
 		return -EINVAL;
 	}
 
@@ -2002,7 +2004,7 @@ static ssize_t gasket_sysfs_data_show(
 		}
 		break;
 	default:
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Unknown attribute: %s", attr->attr.name);
 		ret = 0;
 		break;
@@ -2056,7 +2058,7 @@ int gasket_wait_with_reschedule(
 		msleep(delay_ms);
 		retries++;
 	}
-	gasket_log_error(gasket_dev, "%s timeout: reg %llx timeout (%llu ms)",
+	gasket_log_debug(gasket_dev, "%s timeout: reg %llx timeout (%llu ms)",
 			 __func__, offset, max_retries * delay_ms);
 	return -ETIMEDOUT;
 }
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/20] staging: gasket: gasket page table functions use bool return type
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (16 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 09/14] staging: gasket: core: convert various logs to debug level Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 10/14] staging: gasket: interrupts: convert various logs to debug level Todd Poynor
                   ` (16 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Convert from int to bool return type for gasket page table functions
that return values used as booleans.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 58 +++++++++++-----------
 drivers/staging/gasket/gasket_page_table.h |  8 +--
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 36a560c87af36..2a27db658a4e4 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -262,16 +262,16 @@ static void gasket_perform_unmapping(
 static void gasket_free_extended_subtable(
 	struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte,
 	u64 __iomem *att_reg);
-static int gasket_release_page(struct page *page);
+static bool gasket_release_page(struct page *page);
 
 /* Other/utility declarations */
-static inline int gasket_addr_is_simple(
+static inline bool gasket_addr_is_simple(
 	struct gasket_page_table *pg_tbl, ulong addr);
-static int gasket_is_simple_dev_addr_bad(
+static bool gasket_is_simple_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages);
-static int gasket_is_extended_dev_addr_bad(
+static bool gasket_is_extended_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages);
-static int gasket_is_pte_range_free(
+static bool gasket_is_pte_range_free(
 	struct gasket_page_table_entry *pte, uint num_entries);
 static void gasket_page_table_garbage_collect_nolock(
 	struct gasket_page_table *pg_tbl);
@@ -558,7 +558,7 @@ int gasket_page_table_lookup_page(
 }
 
 /* See gasket_page_table.h for description. */
-int gasket_page_table_are_addrs_bad(
+bool gasket_page_table_are_addrs_bad(
 	struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr,
 	ulong bytes)
 {
@@ -567,7 +567,7 @@ int gasket_page_table_are_addrs_bad(
 			pg_tbl,
 			"host mapping address 0x%lx must be page aligned",
 			host_addr);
-		return 1;
+		return true;
 	}
 
 	return gasket_page_table_is_dev_addr_bad(pg_tbl, dev_addr, bytes);
@@ -575,7 +575,7 @@ int gasket_page_table_are_addrs_bad(
 EXPORT_SYMBOL(gasket_page_table_are_addrs_bad);
 
 /* See gasket_page_table.h for description. */
-int gasket_page_table_is_dev_addr_bad(
+bool gasket_page_table_is_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, ulong bytes)
 {
 	uint num_pages = bytes / PAGE_SIZE;
@@ -584,7 +584,7 @@ int gasket_page_table_is_dev_addr_bad(
 		gasket_pg_tbl_error(
 			pg_tbl,
 			"mapping size 0x%lX must be page aligned", bytes);
-		return 1;
+		return true;
 	}
 
 	if (num_pages == 0) {
@@ -592,7 +592,7 @@ int gasket_page_table_is_dev_addr_bad(
 			pg_tbl,
 			"requested mapping is less than one page: %lu / %lu",
 			bytes, PAGE_SIZE);
-		return 1;
+		return true;
 	}
 
 	if (gasket_addr_is_simple(pg_tbl, dev_addr))
@@ -1285,23 +1285,23 @@ static void gasket_free_extended_subtable(
 /*
  * Safely return a page to the OS.
  * @page: The page to return to the OS.
- * Returns 1 if the page was released, 0 if it was
+ * Returns true if the page was released, false if it was
  * ignored.
  */
-static int gasket_release_page(struct page *page)
+static bool gasket_release_page(struct page *page)
 {
 	if (!page)
-		return 0;
+		return false;
 
 	if (!PageReserved(page))
 		SetPageDirty(page);
 	put_page(page);
 
-	return 1;
+	return true;
 }
 
 /* Evaluates to nonzero if the specified virtual address is simple. */
-static inline int gasket_addr_is_simple(
+static inline bool gasket_addr_is_simple(
 	struct gasket_page_table *pg_tbl, ulong addr)
 {
 	return !((addr) & (pg_tbl)->extended_flag);
@@ -1317,7 +1317,7 @@ static inline int gasket_addr_is_simple(
  * address to/from page + offset) and that the requested page range starts and
  * ends within the set of currently-partitioned simple pages.
  */
-static int gasket_is_simple_dev_addr_bad(
+static bool gasket_is_simple_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
 {
 	ulong page_offset = dev_addr & (PAGE_SIZE - 1);
@@ -1328,7 +1328,7 @@ static int gasket_is_simple_dev_addr_bad(
 		pg_tbl, 1, page_index, page_offset) != dev_addr) {
 		gasket_pg_tbl_error(
 			pg_tbl, "address is invalid, 0x%lX", dev_addr);
-		return 1;
+		return true;
 	}
 
 	if (page_index >= pg_tbl->num_simple_entries) {
@@ -1336,7 +1336,7 @@ static int gasket_is_simple_dev_addr_bad(
 			pg_tbl,
 			"starting slot at %lu is too large, max is < %u",
 			page_index, pg_tbl->num_simple_entries);
-		return 1;
+		return true;
 	}
 
 	if (page_index + num_pages > pg_tbl->num_simple_entries) {
@@ -1344,10 +1344,10 @@ static int gasket_is_simple_dev_addr_bad(
 			pg_tbl,
 			"ending slot at %lu is too large, max is <= %u",
 			page_index + num_pages, pg_tbl->num_simple_entries);
-		return 1;
+		return true;
 	}
 
-	return 0;
+	return false;
 }
 
 /*
@@ -1359,7 +1359,7 @@ static int gasket_is_simple_dev_addr_bad(
  * @dev_addr: The device address to which the pages will be mapped.
  * @num_pages: The number of second-level/sub pages in the range to consider.
  */
-static int gasket_is_extended_dev_addr_bad(
+static bool gasket_is_extended_dev_addr_bad(
 	struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
 {
 	/* Starting byte index of dev_addr into the first mapped page */
@@ -1373,7 +1373,7 @@ static int gasket_is_extended_dev_addr_bad(
 	if (addr >> (GASKET_EXTENDED_LVL0_WIDTH + GASKET_EXTENDED_LVL0_SHIFT)) {
 		gasket_pg_tbl_error(pg_tbl, "device address out of bound, 0x%p",
 				    (void *)dev_addr);
-		return 1;
+		return true;
 	}
 
 	/* Find the starting sub-page index in the space of all sub-pages. */
@@ -1391,7 +1391,7 @@ static int gasket_is_extended_dev_addr_bad(
 		pg_tbl, 0, page_global_idx, page_offset) != dev_addr) {
 		gasket_pg_tbl_error(
 			pg_tbl, "address is invalid, 0x%p", (void *)dev_addr);
-		return 1;
+		return true;
 	}
 
 	if (page_lvl0_idx >= pg_tbl->num_extended_entries) {
@@ -1399,7 +1399,7 @@ static int gasket_is_extended_dev_addr_bad(
 			pg_tbl,
 			"starting level 0 slot at %lu is too large, max is < "
 			"%u", page_lvl0_idx, pg_tbl->num_extended_entries);
-		return 1;
+		return true;
 	}
 
 	if (page_lvl0_idx + num_lvl0_pages > pg_tbl->num_extended_entries) {
@@ -1408,10 +1408,10 @@ static int gasket_is_extended_dev_addr_bad(
 			"ending level 0 slot at %lu is too large, max is <= %u",
 			page_lvl0_idx + num_lvl0_pages,
 			pg_tbl->num_extended_entries);
-		return 1;
+		return true;
 	}
 
-	return 0;
+	return false;
 }
 
 /*
@@ -1425,17 +1425,17 @@ static int gasket_is_extended_dev_addr_bad(
  *
  * The page table mutex must be held before this call.
  */
-static int gasket_is_pte_range_free(
+static bool gasket_is_pte_range_free(
 	struct gasket_page_table_entry *ptes, uint num_entries)
 {
 	int i;
 
 	for (i = 0; i < num_entries; i++) {
 		if (ptes[i].status != PTE_FREE)
-			return 0;
+			return false;
 	}
 
-	return 1;
+	return true;
 }
 
 /*
diff --git a/drivers/staging/gasket/gasket_page_table.h b/drivers/staging/gasket/gasket_page_table.h
index 720ce2bc2c012..0e8afdb8c1139 100644
--- a/drivers/staging/gasket/gasket_page_table.h
+++ b/drivers/staging/gasket/gasket_page_table.h
@@ -162,9 +162,9 @@ int gasket_page_table_lookup_page(
  * specified by both addresses and the size are valid for mapping pages into
  * device memory.
  *
- * Returns 1 if true - if the mapping is bad, 0 otherwise.
+ * Returns true if the mapping is bad, false otherwise.
  */
-int gasket_page_table_are_addrs_bad(
+bool gasket_page_table_are_addrs_bad(
 	struct gasket_page_table *page_table, ulong host_addr, ulong dev_addr,
 	ulong bytes);
 
@@ -178,9 +178,9 @@ int gasket_page_table_are_addrs_bad(
  * specified by the device address and the size is valid for mapping pages into
  * device memory.
  *
- * Returns 1 if true - if the address is bad, 0 otherwise.
+ * Returns true if the address is bad, false otherwise.
  */
-int gasket_page_table_is_dev_addr_bad(
+bool gasket_page_table_is_dev_addr_bad(
 	struct gasket_page_table *page_table, ulong dev_addr, ulong bytes);
 
 /*
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/14] staging: gasket: interrupts: convert various logs to debug level
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (17 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 09/20] staging: gasket: gasket page table functions use bool return type Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:56 ` [PATCH 10/20] staging: gasket: remove else clause after return in if clause Todd Poynor
                   ` (15 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs.  Convert these to debug log level.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_interrupt.c | 24 +++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index d096ce73cc8b3..2b8c26d065336 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -287,7 +287,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
 	int ret;
 
 	if (!gasket_dev->interrupt_data) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev,
 			"Attempted to reinit uninitialized interrupt data.");
 		return -EINVAL;
@@ -305,7 +305,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
 	case PCI_MSI:
 	case PLATFORM_WIRE:
 	default:
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"Cannot handle unsupported interrupt type %d.",
 			gasket_dev->interrupt_data->type);
 		ret = -EINVAL;
@@ -351,7 +351,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
 		gasket_dev->interrupt_data;
 
 	if (!interrupt_data) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Interrupt data is not initialized.");
 		return;
 	}
@@ -365,7 +365,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
 	}
 
 	if (interrupt_data->type != PCI_MSIX) {
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"Cannot handle unsupported interrupt type %d.",
 			interrupt_data->type);
 		return;
@@ -403,7 +403,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
 				pack_shift = 3 * interrupt_data->pack_width;
 				break;
 			default:
-				gasket_nodev_error(
+				gasket_nodev_debug(
 					"Found interrupt description with "
 					"unknown enum %d.",
 					interrupt_data->interrupts[i].packing);
@@ -445,7 +445,7 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
 	case PCI_MSI:
 	case PLATFORM_WIRE:
 	default:
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"Cannot handle unsupported interrupt type %d.",
 			interrupt_data->type);
 	};
@@ -460,18 +460,18 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
 int gasket_interrupt_system_status(struct gasket_dev *gasket_dev)
 {
 	if (!gasket_dev->interrupt_data) {
-		gasket_nodev_info("Interrupt data is null.");
+		gasket_nodev_debug("Interrupt data is null.");
 		return GASKET_STATUS_DEAD;
 	}
 
 	if (!gasket_dev->interrupt_data->msix_configured) {
-		gasket_nodev_info("Interrupt not initialized.");
+		gasket_nodev_debug("Interrupt not initialized.");
 		return GASKET_STATUS_LAMED;
 	}
 
 	if (gasket_dev->interrupt_data->num_configured !=
 		gasket_dev->interrupt_data->num_interrupts) {
-		gasket_nodev_info("Not all interrupts were configured.");
+		gasket_nodev_debug("Not all interrupts were configured.");
 		return GASKET_STATUS_LAMED;
 	}
 
@@ -516,14 +516,14 @@ static ssize_t interrupt_sysfs_show(
 
 	gasket_dev = gasket_sysfs_get_device_data(device);
 	if (!gasket_dev) {
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"No sysfs mapping found for device 0x%p", device);
 		return 0;
 	}
 
 	gasket_attr = gasket_sysfs_get_attr(device, attr);
 	if (!gasket_attr) {
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"No sysfs attr data found for device 0x%p", device);
 		gasket_sysfs_put_device_data(device, gasket_dev);
 		return 0;
@@ -545,7 +545,7 @@ static ssize_t interrupt_sysfs_show(
 		ret = total_written;
 		break;
 	default:
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Unknown attribute: %s", attr->attr.name);
 		ret = 0;
 		break;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/20] staging: gasket: remove else clause after return in if clause
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (18 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 10/14] staging: gasket: interrupts: convert various logs to debug level Todd Poynor
@ 2018-07-21 12:56 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 11/20] staging: gasket: fix comment syntax in apex.h Todd Poynor
                   ` (14 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:56 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Else after return is unnecessary and may cause static code checkers to
complain.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 2a27db658a4e4..617d602b8b447 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -598,9 +598,7 @@ bool gasket_page_table_is_dev_addr_bad(
 	if (gasket_addr_is_simple(pg_tbl, dev_addr))
 		return gasket_is_simple_dev_addr_bad(
 			pg_tbl, dev_addr, num_pages);
-	else
-		return gasket_is_extended_dev_addr_bad(
-			pg_tbl, dev_addr, num_pages);
+	return gasket_is_extended_dev_addr_bad(pg_tbl, dev_addr, num_pages);
 }
 EXPORT_SYMBOL(gasket_page_table_is_dev_addr_bad);
 
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/20] staging: gasket: fix comment syntax in apex.h
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (19 preceding siblings ...)
  2018-07-21 12:56 ` [PATCH 10/20] staging: gasket: remove else clause after return in if clause Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 11/14] staging: gasket: ioctl common: convert various logs to debug level Todd Poynor
                   ` (13 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Use kernel-style multi-line comment syntax.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/apex.h b/drivers/staging/gasket/apex.h
index 4ef264106f503..d89cc2387b7d4 100644
--- a/drivers/staging/gasket/apex.h
+++ b/drivers/staging/gasket/apex.h
@@ -22,9 +22,10 @@
 
 #define APEX_EXTENDED_SHIFT 63 /* Extended address bit position. */
 
-/* Addresses are 2^3=8 bytes each. */
-/* page in second level page table */
-/* holds APEX_PAGE_SIZE/8 addresses  */
+/*
+ * Addresses are 2^3=8 bytes each. Page in second level page table holds
+ * APEX_PAGE_SIZE/8 addresses.
+ */
 #define APEX_ADDR_SHIFT 3
 #define APEX_LEVEL_SHIFT (APEX_PAGE_SHIFT - APEX_ADDR_SHIFT)
 #define APEX_LEVEL_SIZE BIT(APEX_LEVEL_SHIFT)
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/14] staging: gasket: ioctl common: convert various logs to debug level
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (20 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 11/20] staging: gasket: fix comment syntax in apex.h Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 12/14] staging: gasket: page table: " Todd Poynor
                   ` (12 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs.  Convert these to debug log level.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c  | 2 +-
 drivers/staging/gasket/gasket_ioctl.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 1d04fd0990e45..732218773c3c6 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1801,7 +1801,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 		if (driver_desc->ioctl_handler_cb)
 			return driver_desc->ioctl_handler_cb(filp, cmd, argp);
 
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Received unknown ioctl 0x%x", cmd);
 		return -EINVAL;
 	}
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 8cf094b90cdb0..63e139ab7ff89 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -73,7 +73,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		}
 	} else if (!gasket_ioctl_check_permissions(filp, cmd)) {
 		trace_gasket_ioctl_exit(-EPERM);
-		gasket_log_error(gasket_dev, "ioctl cmd=%x noperm.", cmd);
+		gasket_log_debug(gasket_dev, "ioctl cmd=%x noperm.", cmd);
 		return -EPERM;
 	}
 
@@ -132,7 +132,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		 * the arg.
 		 */
 		trace_gasket_ioctl_integer_data(arg);
-		gasket_log_warn(
+		gasket_log_debug(
 			gasket_dev,
 			"Unknown ioctl cmd=0x%x not caught by "
 			"gasket_is_supported_ioctl",
@@ -329,7 +329,7 @@ static int gasket_partition_page_table(
 		gasket_dev->page_table[ibuf.page_table_index]);
 
 	if (ibuf.size > max_page_table_size) {
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev,
 			"Partition request 0x%llx too large, max is 0x%x.",
 			ibuf.size, max_page_table_size);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/14] staging: gasket: page table: convert various logs to debug level
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (21 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 11/14] staging: gasket: ioctl common: convert various logs to debug level Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 12/20] staging: gasket: remove unnecessary parens in page table code Todd Poynor
                   ` (11 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs.  Convert these to debug log level.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 9f8116112e0ac..f0c4884cb4bc6 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -306,7 +306,7 @@ int gasket_page_table_init(
 	 * hardware register that contains the page table size.
 	 */
 	if (total_entries == ULONG_MAX) {
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"Error reading page table size. "
 			"Initializing page table with size 0.");
 		total_entries = 0;
@@ -323,7 +323,7 @@ int gasket_page_table_init(
 
 	*ppg_tbl = kzalloc(sizeof(**ppg_tbl), GFP_KERNEL);
 	if (!*ppg_tbl) {
-		gasket_nodev_error("No memory for page table.");
+		gasket_nodev_debug("No memory for page table.");
 		return -ENOMEM;
 	}
 
@@ -332,7 +332,7 @@ int gasket_page_table_init(
 	if (bytes != 0) {
 		pg_tbl->entries = vmalloc(bytes);
 		if (!pg_tbl->entries) {
-			gasket_nodev_error(
+			gasket_nodev_debug(
 				"No memory for address translation metadata.");
 			kfree(pg_tbl);
 			*ppg_tbl = NULL;
@@ -658,7 +658,7 @@ int gasket_page_table_system_status(struct gasket_page_table *page_table)
 	}
 
 	if (gasket_page_table_num_entries(page_table) == 0) {
-		gasket_nodev_error("Page table size is 0.");
+		gasket_nodev_debug("Page table size is 0.");
 		return GASKET_STATUS_LAMED;
 	}
 
@@ -903,7 +903,7 @@ static int gasket_perform_mapping(
 				(unsigned long long)ptes[i].dma_addr);
 
 			if (ptes[i].dma_addr == -1) {
-				gasket_nodev_error(
+				gasket_nodev_debug(
 					"%s i %d"
 					" -> fail to map page %llx "
 					"[pfn %p ohys %p]\n",
@@ -1612,7 +1612,7 @@ int gasket_set_user_virt(
 	 */
 	pg_tbl = gasket_dev->page_table[0];
 	if (!pg_tbl) {
-		gasket_nodev_error(
+		gasket_nodev_debug(
 			"%s: invalid page table index", __func__);
 		return 0;
 	}
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/20] staging: gasket: remove unnecessary parens in page table code
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (22 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 12/14] staging: gasket: page table: " Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 13/20] staging: gasket: gasket_mmap use PAGE_MASK Todd Poynor
                   ` (10 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

gasket_alloc_coherent_memory() extra parentheses in statement.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 617d602b8b447..9f8116112e0ac 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1639,7 +1639,7 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 	dma_addr_t handle;
 	void *mem;
 	int j;
-	unsigned int num_pages = (size + PAGE_SIZE - 1) / (PAGE_SIZE);
+	unsigned int num_pages = (size + PAGE_SIZE - 1) / PAGE_SIZE;
 	const struct gasket_driver_desc *driver_desc =
 		gasket_get_driver_desc(gasket_dev);
 
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/20] staging: gasket: gasket_mmap use PAGE_MASK
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (23 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 12/20] staging: gasket: remove unnecessary parens in page table code Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 13/14] staging: gasket: page table: remove unnecessary logs Todd Poynor
                   ` (9 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

gasket_mmap use PAGE_MASK, instead of performing math on PAGE_SIZE, for
simplicity and clarity.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 3cf918f9d2604..ae5febec8844c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1591,7 +1591,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
 	}
 	driver_desc = gasket_dev->internal_desc->driver_desc;
 
-	if (vma->vm_start & (PAGE_SIZE - 1)) {
+	if (vma->vm_start & ~PAGE_MASK) {
 		gasket_log_error(
 			gasket_dev, "Base address not page-aligned: 0x%p\n",
 			(void *)vma->vm_start);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/14] staging: gasket: page table: remove unnecessary logs
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (24 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 13/20] staging: gasket: gasket_mmap use PAGE_MASK Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 14/14] staging: gasket: apex: convert various logs to debug level Todd Poynor
                   ` (8 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Some error logs in page table handling code could only be hit in
cases of programming errors not expected in the current code base, and
aren't likely to be useful on their own.  Remove these.

Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_page_table.c | 23 +++++-----------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index f0c4884cb4bc6..4f2ff77006586 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -605,10 +605,8 @@ EXPORT_SYMBOL(gasket_page_table_is_dev_addr_bad);
 /* See gasket_page_table.h for description. */
 uint gasket_page_table_max_size(struct gasket_page_table *page_table)
 {
-	if (!page_table) {
-		gasket_nodev_error("Passed a null page table.");
+	if (!page_table)
 		return 0;
-	}
 	return page_table->config.total_entries;
 }
 EXPORT_SYMBOL(gasket_page_table_max_size);
@@ -616,11 +614,8 @@ EXPORT_SYMBOL(gasket_page_table_max_size);
 /* See gasket_page_table.h for description. */
 uint gasket_page_table_num_entries(struct gasket_page_table *pg_tbl)
 {
-	if (!pg_tbl) {
-		gasket_nodev_error("Passed a null page table.");
+	if (!pg_tbl)
 		return 0;
-	}
-
 	return pg_tbl->num_simple_entries + pg_tbl->num_extended_entries;
 }
 EXPORT_SYMBOL(gasket_page_table_num_entries);
@@ -628,11 +623,8 @@ EXPORT_SYMBOL(gasket_page_table_num_entries);
 /* See gasket_page_table.h for description. */
 uint gasket_page_table_num_simple_entries(struct gasket_page_table *pg_tbl)
 {
-	if (!pg_tbl) {
-		gasket_nodev_error("Passed a null page table.");
+	if (!pg_tbl)
 		return 0;
-	}
-
 	return pg_tbl->num_simple_entries;
 }
 EXPORT_SYMBOL(gasket_page_table_num_simple_entries);
@@ -640,11 +632,8 @@ EXPORT_SYMBOL(gasket_page_table_num_simple_entries);
 /* See gasket_page_table.h for description. */
 uint gasket_page_table_num_active_pages(struct gasket_page_table *pg_tbl)
 {
-	if (!pg_tbl) {
-		gasket_nodev_error("Passed a null page table.");
+	if (!pg_tbl)
 		return 0;
-	}
-
 	return pg_tbl->num_active_pages;
 }
 EXPORT_SYMBOL(gasket_page_table_num_active_pages);
@@ -652,10 +641,8 @@ EXPORT_SYMBOL(gasket_page_table_num_active_pages);
 /* See gasket_page_table.h */
 int gasket_page_table_system_status(struct gasket_page_table *page_table)
 {
-	if (!page_table) {
-		gasket_nodev_error("Passed a null page table.");
+	if (!page_table)
 		return GASKET_STATUS_LAMED;
-	}
 
 	if (gasket_page_table_num_entries(page_table) == 0) {
 		gasket_nodev_debug("Page table size is 0.");
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/14] staging: gasket: apex: convert various logs to debug level
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (25 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 13/14] staging: gasket: page table: remove unnecessary logs Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 14/20] staging: gasket: remove extra parens in gasket_write_mappable_regions Todd Poynor
                   ` (7 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs.  Convert these to debug log level.

Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 1c6f73b5a2a9e..6396b18b246a5 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -420,7 +420,7 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
 		gasket_dev, APEX_BAR_INDEX,
 		APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
 
-	gasket_log_info(
+	gasket_log_debug(
 		gasket_dev,
 		"%s 0x%p hib_error 0x%llx scalar_error "
 		"0x%llx.",
@@ -589,7 +589,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 			SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
 		val1 = gasket_dev_read_32(
 			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Disallow HW clock gating 0x%x -> 0x%x",
 			val0, val1);
 	} else {
@@ -602,7 +602,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
 			SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
 		val1 = gasket_dev_read_32(
 			gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Allow HW clock gating 0x%x -> 0x%x", val0,
 			val1);
 	}
@@ -668,7 +668,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
 	if (copy_from_user(&ibuf, argp, sizeof(ibuf)))
 		return -EFAULT;
 
-	gasket_log_error(gasket_dev, "%s %llu", __func__, ibuf.enable);
+	gasket_log_debug(gasket_dev, "%s %llu", __func__, ibuf.enable);
 
 	if (ibuf.enable) {
 		/* Quiesce AXI, gate GCB clock. */
@@ -738,7 +738,7 @@ static ssize_t sysfs_show(
 					gasket_dev->page_table[0]));
 		break;
 	default:
-		gasket_log_error(
+		gasket_log_debug(
 			gasket_dev, "Unknown attribute: %s", attr->attr.name);
 		ret = 0;
 		break;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/20] staging: gasket: remove extra parens in gasket_write_mappable_regions
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (26 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 14/14] staging: gasket: apex: convert various logs to debug level Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 15/20] staging: gasket: fix multi-line comment syntax in gasket_core.h Todd Poynor
                   ` (6 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Remove unneeded parentheses around subexpressions.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index ae5febec8844c..ba48a379b0ada 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1889,7 +1889,7 @@ static ssize_t gasket_write_mappable_regions(
 	if (bar_desc.permissions == GASKET_NOMAP)
 		return 0;
 	for (i = 0;
-	     (i < bar_desc.num_mappable_regions) && (total_written < PAGE_SIZE);
+	     i < bar_desc.num_mappable_regions && total_written < PAGE_SIZE;
 	     i++) {
 		min_addr = bar_desc.mappable_regions[i].start -
 			   driver_desc->legacy_mmap_address_offset;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/20] staging: gasket: fix multi-line comment syntax in gasket_core.h
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (27 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 14/20] staging: gasket: remove extra parens in gasket_write_mappable_regions Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 16/20] staging: gasket: always allow root open for write Todd Poynor
                   ` (5 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor, Simon Que

From: Todd Poynor <toddpoynor@google.com>

Use consistent kernel-style multi-line comment syntax.

Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Simon Que <sque@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 50ad0c8853183..7ea1df123ba5d 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -54,7 +54,8 @@ enum gasket_interrupt_type {
 	PLATFORM_WIRE = 2,
 };
 
-/* Used to describe a Gasket interrupt. Contains an interrupt index, a register,
+/*
+ * Used to describe a Gasket interrupt. Contains an interrupt index, a register,
  * and packing data for that interrupt. The register and packing data
  * fields are relevant only for PCI_MSIX interrupt type and can be
  * set to 0 for everything else.
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/20] staging: gasket: always allow root open for write
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (28 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 15/20] staging: gasket: fix multi-line comment syntax in gasket_core.h Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 17/20] staging: gasket: top ioctl handler add __user annotations Todd Poynor
                   ` (4 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Always allow root to open device for writing.

Drop special-casing of ioctl permissions for root vs. owner.

Convert to bool types as appropriate.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c  | 15 ++++----------
 drivers/staging/gasket/gasket_core.c  |  8 ++++---
 drivers/staging/gasket/gasket_ioctl.c | 30 +++++++++++++--------------
 3 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index a01b1f2b827ea..4c00f3609f081 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -140,7 +140,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type);
 
 static int apex_get_status(struct gasket_dev *gasket_dev);
 
-static uint apex_ioctl_check_permissions(struct file *file, uint cmd);
+static bool apex_ioctl_check_permissions(struct file *file, uint cmd);
 
 static long apex_ioctl(struct file *file, uint cmd, ulong arg);
 
@@ -625,18 +625,11 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
  * @file: File pointer from ioctl.
  * @cmd: ioctl command.
  *
- * Returns 1 if the current user may execute this ioctl, and 0 otherwise.
+ * Returns true if the current user may execute this ioctl, and false otherwise.
  */
-static uint apex_ioctl_check_permissions(struct file *filp, uint cmd)
+static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
 {
-	struct gasket_dev *gasket_dev = filp->private_data;
-	int root = capable(CAP_SYS_ADMIN);
-	int is_owner = gasket_dev->dev_info.ownership.is_owned &&
-		       current->tgid == gasket_dev->dev_info.ownership.owner;
-
-	if (root || is_owner)
-		return 1;
-	return 0;
+	return !!(filp->f_mode & FMODE_WRITE);
 }
 
 /*
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index ba48a379b0ada..254fb392c05c1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1072,6 +1072,7 @@ static int gasket_open(struct inode *inode, struct file *filp)
 	char task_name[TASK_COMM_LEN];
 	struct gasket_cdev_info *dev_info =
 	    container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
+	int is_root = capable(CAP_SYS_ADMIN);
 
 	gasket_dev = dev_info->gasket_dev_ptr;
 	driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1085,7 +1086,7 @@ static int gasket_open(struct inode *inode, struct file *filp)
 		"Attempting to open with tgid %u (%s) (f_mode: 0%03o, "
 		"fmode_write: %d is_root: %u)",
 		current->tgid, task_name, filp->f_mode,
-		(filp->f_mode & FMODE_WRITE), capable(CAP_SYS_ADMIN));
+		(filp->f_mode & FMODE_WRITE), is_root);
 
 	/* Always allow non-writing accesses. */
 	if (!(filp->f_mode & FMODE_WRITE)) {
@@ -1099,8 +1100,9 @@ static int gasket_open(struct inode *inode, struct file *filp)
 		gasket_dev, "Current owner open count (owning tgid %u): %d.",
 		ownership->owner, ownership->write_open_count);
 
-	/* Opening a node owned by another TGID is an error (even root.) */
-	if (ownership->is_owned && ownership->owner != current->tgid) {
+	/* Opening a node owned by another TGID is an error (unless root) */
+	if (ownership->is_owned && ownership->owner != current->tgid &&
+	    !is_root) {
 		gasket_log_error(
 			gasket_dev,
 			"Process %u is opening a node held by %u.",
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index d0142ed048a65..8fd44979fe713 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -22,7 +22,7 @@
 #define trace_gasket_ioctl_config_coherent_allocator(x, ...)
 #endif
 
-static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd);
+static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd);
 static int gasket_set_event_fd(struct gasket_dev *dev, ulong arg);
 static int gasket_read_page_table_size(
 	struct gasket_dev *gasket_dev, ulong arg);
@@ -167,12 +167,13 @@ long gasket_is_supported_ioctl(uint cmd)
  * @filp: File structure pointer describing this node usage session.
  * @cmd: ioctl number to handle.
  *
- * Standard permissions checker.
+ * Check permissions for Gasket ioctls.
+ * Returns true if the file opener may execute this ioctl, or false otherwise.
  */
-static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd)
+static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 {
-	uint alive, root, device_owner;
-	fmode_t read, write;
+	bool alive;
+	bool read, write;
 	struct gasket_dev *gasket_dev = (struct gasket_dev *)filp->private_data;
 
 	alive = (gasket_dev->status == GASKET_STATUS_ALIVE);
@@ -183,36 +184,33 @@ static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 			alive, gasket_dev->status);
 	}
 
-	root = capable(CAP_SYS_ADMIN);
-	read = filp->f_mode & FMODE_READ;
-	write = filp->f_mode & FMODE_WRITE;
-	device_owner = (gasket_dev->dev_info.ownership.is_owned &&
-			current->tgid == gasket_dev->dev_info.ownership.owner);
+	read = !!(filp->f_mode & FMODE_READ);
+	write = !!(filp->f_mode & FMODE_WRITE);
 
 	switch (cmd) {
 	case GASKET_IOCTL_RESET:
 	case GASKET_IOCTL_CLEAR_INTERRUPT_COUNTS:
-		return root || (write && device_owner);
+		return write;
 
 	case GASKET_IOCTL_PAGE_TABLE_SIZE:
 	case GASKET_IOCTL_SIMPLE_PAGE_TABLE_SIZE:
 	case GASKET_IOCTL_NUMBER_PAGE_TABLES:
-		return root || read;
+		return read;
 
 	case GASKET_IOCTL_PARTITION_PAGE_TABLE:
 	case GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 
 	case GASKET_IOCTL_MAP_BUFFER:
 	case GASKET_IOCTL_UNMAP_BUFFER:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 
 	case GASKET_IOCTL_CLEAR_EVENTFD:
 	case GASKET_IOCTL_SET_EVENTFD:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 	}
 
-	return 0; /* unknown permissions */
+	return false; /* unknown permissions */
 }
 
 /*
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/20] staging: gasket: top ioctl handler add __user annotations
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (29 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 16/20] staging: gasket: always allow root open for write Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 18/20] staging: gasket: apex ioctl " Todd Poynor
                   ` (3 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add __user annotation to gasket_core top-level ioctl handling pointer
arguments, for sparse checking.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_core.c | 6 ++++--
 drivers/staging/gasket/gasket_core.h | 7 +++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 254fb392c05c1..40e46ca5228c8 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -14,6 +14,7 @@
 #include "gasket_page_table.h"
 #include "gasket_sysfs.h"
 
+#include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/fs.h>
 #include <linux/init.h>
@@ -1781,6 +1782,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 {
 	struct gasket_dev *gasket_dev;
 	const struct gasket_driver_desc *driver_desc;
+	void __user *argp = (void __user *)arg;
 	char path[256];
 
 	if (!filp)
@@ -1810,14 +1812,14 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
 		 * check_and_invoke_callback.
 		 */
 		if (driver_desc->ioctl_handler_cb)
-			return driver_desc->ioctl_handler_cb(filp, cmd, arg);
+			return driver_desc->ioctl_handler_cb(filp, cmd, argp);
 
 		gasket_log_error(
 			gasket_dev, "Received unknown ioctl 0x%x", cmd);
 		return -EINVAL;
 	}
 
-	return gasket_handle_ioctl(filp, cmd, arg);
+	return gasket_handle_ioctl(filp, cmd, argp);
 }
 
 int gasket_reset(struct gasket_dev *gasket_dev, uint reset_type)
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index 7ea1df123ba5d..bf4ed3769efb2 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -314,9 +314,12 @@ struct gasket_dev {
 	struct hlist_node legacy_hlist_node;
 };
 
+/* Type of the ioctl handler callback. */
+typedef long (*gasket_ioctl_handler_cb_t)
+		(struct file *file, uint cmd, void __user *argp);
 /* Type of the ioctl permissions check callback. See below. */
 typedef int (*gasket_ioctl_permissions_cb_t)(
-	struct file *filp, uint cmd, ulong arg);
+	struct file *filp, uint cmd, void __user *argp);
 
 /*
  * Device type descriptor.
@@ -550,7 +553,7 @@ struct gasket_driver_desc {
 	 * return -EINVAL. Should return an error status (either -EINVAL or
 	 * the error result of the ioctl being handled).
 	 */
-	long (*ioctl_handler_cb)(struct file *filp, uint cmd, ulong arg);
+	gasket_ioctl_handler_cb_t ioctl_handler_cb;
 
 	/*
 	 * device_status_cb: Callback to determine device health.
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 18/20] staging: gasket: apex ioctl add __user annotations
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (30 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 17/20] staging: gasket: top ioctl handler add __user annotations Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 19/20] staging: gasket: common ioctl dispatcher " Todd Poynor
                   ` (2 subsequent siblings)
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add __user annotation to ioctl pointer argument, for sparse checking.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 4c00f3609f081..3e76c4db5db2e 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2018 Google, Inc.
  */
 
+#include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/fs.h>
 #include <linux/init.h>
@@ -142,9 +143,10 @@ static int apex_get_status(struct gasket_dev *gasket_dev);
 
 static bool apex_ioctl_check_permissions(struct file *file, uint cmd);
 
-static long apex_ioctl(struct file *file, uint cmd, ulong arg);
+static long apex_ioctl(struct file *file, uint cmd, void __user *argp);
 
-static long apex_clock_gating(struct gasket_dev *gasket_dev, ulong arg);
+static long apex_clock_gating(struct gasket_dev *gasket_dev,
+			      struct apex_gate_clock_ioctl __user *argp);
 
 static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type);
 
@@ -635,7 +637,7 @@ static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
 /*
  * Apex-specific ioctl handler.
  */
-static long apex_ioctl(struct file *filp, uint cmd, ulong arg)
+static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
 {
 	struct gasket_dev *gasket_dev = filp->private_data;
 
@@ -644,7 +646,7 @@ static long apex_ioctl(struct file *filp, uint cmd, ulong arg)
 
 	switch (cmd) {
 	case APEX_IOCTL_GATE_CLOCK:
-		return apex_clock_gating(gasket_dev, arg);
+		return apex_clock_gating(gasket_dev, argp);
 	default:
 		return -ENOTTY; /* unknown command */
 	}
@@ -653,16 +655,17 @@ static long apex_ioctl(struct file *filp, uint cmd, ulong arg)
 /*
  * Gates or un-gates Apex clock.
  * @gasket_dev: Gasket device pointer.
- * @arg: User ioctl arg, in this case to a apex_gate_clock_ioctl struct.
+ * @argp: User ioctl arg, pointer to a apex_gate_clock_ioctl struct.
  */
-static long apex_clock_gating(struct gasket_dev *gasket_dev, ulong arg)
+static long apex_clock_gating(struct gasket_dev *gasket_dev,
+			      struct apex_gate_clock_ioctl __user *argp)
 {
 	struct apex_gate_clock_ioctl ibuf;
 
 	if (bypass_top_level || !allow_sw_clock_gating)
 		return 0;
 
-	if (copy_from_user(&ibuf, (void __user *)arg, sizeof(ibuf)))
+	if (copy_from_user(&ibuf, argp, sizeof(ibuf)))
 		return -EFAULT;
 
 	gasket_log_error(gasket_dev, "%s %llu", __func__, ibuf.enable);
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 19/20] staging: gasket: common ioctl dispatcher add __user annotations
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (31 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 18/20] staging: gasket: apex ioctl " Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 12:57 ` [PATCH 20/20] staging: gasket: common ioctls " Todd Poynor
  2018-07-21 13:17 ` [PATCH 00/14] staging: gasket: assorted cleanups Greg Kroah-Hartman
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add __user annotation to gasket core common ioctl pointer arguments for
sparse checking.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 8 +++++---
 drivers/staging/gasket/gasket_ioctl.h | 4 +++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 8fd44979fe713..998d0e215523c 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -7,6 +7,7 @@
 #include "gasket_interrupt.h"
 #include "gasket_logging.h"
 #include "gasket_page_table.h"
+#include <linux/compiler.h>
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 
@@ -39,13 +40,14 @@ static int gasket_config_coherent_allocator(
  * standard ioctl dispatch function.
  * @filp: File structure pointer describing this node usage session.
  * @cmd: ioctl number to handle.
- * @arg: ioctl-specific data pointer.
+ * @argp: ioctl-specific data pointer.
  *
  * Standard ioctl dispatcher; forwards operations to individual handlers.
  */
-long gasket_handle_ioctl(struct file *filp, uint cmd, ulong arg)
+long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 {
 	struct gasket_dev *gasket_dev;
+	unsigned long arg = (unsigned long)argp;
 	int retval;
 
 	gasket_dev = (struct gasket_dev *)filp->private_data;
@@ -53,7 +55,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, ulong arg)
 
 	if (gasket_get_ioctl_permissions_cb(gasket_dev)) {
 		retval = gasket_get_ioctl_permissions_cb(gasket_dev)(
-			filp, cmd, arg);
+			filp, cmd, argp);
 		if (retval < 0) {
 			trace_gasket_ioctl_exit(-EPERM);
 			return retval;
diff --git a/drivers/staging/gasket/gasket_ioctl.h b/drivers/staging/gasket/gasket_ioctl.h
index 461fab27a3e52..51f468c77f041 100644
--- a/drivers/staging/gasket/gasket_ioctl.h
+++ b/drivers/staging/gasket/gasket_ioctl.h
@@ -5,6 +5,8 @@
 
 #include "gasket_core.h"
 
+#include <linux/compiler.h>
+
 /*
  * Handle Gasket common ioctls.
  * @filp: Pointer to the ioctl's file.
@@ -13,7 +15,7 @@
  *
  * Returns 0 on success and nonzero on failure.
  */
-long gasket_handle_ioctl(struct file *filp, uint cmd, ulong arg);
+long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp);
 
 /*
  * Determines if an ioctl is part of the standard Gasket framework.
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 20/20] staging: gasket: common ioctls add __user annotations
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (32 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 19/20] staging: gasket: common ioctl dispatcher " Todd Poynor
@ 2018-07-21 12:57 ` Todd Poynor
  2018-07-21 13:17 ` [PATCH 00/14] staging: gasket: assorted cleanups Greg Kroah-Hartman
  34 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 12:57 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
	Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add __user annotation to gasket common ioctl pointer arguments for
sparse checking.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 102 ++++++++++++++------------
 1 file changed, 55 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 998d0e215523c..2e2c9b997093b 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -24,17 +24,24 @@
 #endif
 
 static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd);
-static int gasket_set_event_fd(struct gasket_dev *dev, ulong arg);
+static int gasket_set_event_fd(struct gasket_dev *dev,
+			       struct gasket_interrupt_eventfd __user *argp);
 static int gasket_read_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
 static int gasket_read_simple_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
 static int gasket_partition_page_table(
-	struct gasket_dev *gasket_dev, ulong arg);
-static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg);
-static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
+static int gasket_map_buffers(struct gasket_dev *gasket_dev,
+			      struct gasket_page_table_ioctl __user *argp);
+static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
+				struct gasket_page_table_ioctl __user *argp);
 static int gasket_config_coherent_allocator(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_coherent_alloc_config_ioctl __user *argp);
 
 /*
  * standard ioctl dispatch function.
@@ -80,7 +87,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		retval = gasket_reset(gasket_dev, arg);
 		break;
 	case GASKET_IOCTL_SET_EVENTFD:
-		retval = gasket_set_event_fd(gasket_dev, arg);
+		retval = gasket_set_event_fd(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CLEAR_EVENTFD:
 		trace_gasket_ioctl_integer_data(arg);
@@ -89,31 +96,30 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		break;
 	case GASKET_IOCTL_PARTITION_PAGE_TABLE:
 		trace_gasket_ioctl_integer_data(arg);
-		retval = gasket_partition_page_table(gasket_dev, arg);
+		retval = gasket_partition_page_table(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_NUMBER_PAGE_TABLES:
 		trace_gasket_ioctl_integer_data(gasket_dev->num_page_tables);
-		if (copy_to_user((void __user *)arg,
-				 &gasket_dev->num_page_tables,
+		if (copy_to_user(argp, &gasket_dev->num_page_tables,
 				 sizeof(uint64_t)))
 			retval = -EFAULT;
 		else
 			retval = 0;
 		break;
 	case GASKET_IOCTL_PAGE_TABLE_SIZE:
-		retval = gasket_read_page_table_size(gasket_dev, arg);
+		retval = gasket_read_page_table_size(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_SIMPLE_PAGE_TABLE_SIZE:
-		retval = gasket_read_simple_page_table_size(gasket_dev, arg);
+		retval = gasket_read_simple_page_table_size(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_MAP_BUFFER:
-		retval = gasket_map_buffers(gasket_dev, arg);
+		retval = gasket_map_buffers(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR:
-		retval = gasket_config_coherent_allocator(gasket_dev, arg);
+		retval = gasket_config_coherent_allocator(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_UNMAP_BUFFER:
-		retval = gasket_unmap_buffers(gasket_dev, arg);
+		retval = gasket_unmap_buffers(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CLEAR_INTERRUPT_COUNTS:
 		/* Clear interrupt counts doesn't take an arg, so use 0. */
@@ -218,16 +224,15 @@ static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 /*
  * Associate an eventfd with an interrupt.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_interrupt_eventfd struct in userspace.
+ * @argp: Pointer to gasket_interrupt_eventfd struct in userspace.
  */
-static int gasket_set_event_fd(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
+			       struct gasket_interrupt_eventfd __user *argp)
 {
 	struct gasket_interrupt_eventfd die;
 
-	if (copy_from_user(&die, (void __user *)arg,
-			   sizeof(struct gasket_interrupt_eventfd))) {
+	if (copy_from_user(&die, argp, sizeof(struct gasket_interrupt_eventfd)))
 		return -EFAULT;
-	}
 
 	trace_gasket_ioctl_eventfd_data(die.interrupt, die.event_fd);
 
@@ -238,15 +243,16 @@ static int gasket_set_event_fd(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Reads the size of the page table.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_read_page_table_size(
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret = 0;
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	if (ibuf.page_table_index >= gasket_dev->num_page_tables)
@@ -259,7 +265,7 @@ static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
 		ibuf.page_table_index, ibuf.size, ibuf.host_address,
 		ibuf.device_address);
 
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
@@ -268,16 +274,16 @@ static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Reads the size of the simple page table.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
 static int gasket_read_simple_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg)
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret = 0;
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	if (ibuf.page_table_index >= gasket_dev->num_page_tables)
@@ -290,7 +296,7 @@ static int gasket_read_simple_page_table_size(
 		ibuf.page_table_index, ibuf.size, ibuf.host_address,
 		ibuf.device_address);
 
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
@@ -299,16 +305,17 @@ static int gasket_read_simple_page_table_size(
 /*
  * Sets the boundary between the simple and extended page tables.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_partition_page_table(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_partition_page_table(
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret;
 	struct gasket_page_table_ioctl ibuf;
 	uint max_page_table_size;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -340,14 +347,14 @@ static int gasket_partition_page_table(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Maps a userspace buffer to a device virtual address.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_map_buffers(struct gasket_dev *gasket_dev,
+			      struct gasket_page_table_ioctl __user *argp)
 {
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -370,14 +377,14 @@ static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Unmaps a userspace buffer from a device virtual address.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
+				struct gasket_page_table_ioctl __user *argp)
 {
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -402,15 +409,16 @@ static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg)
  * Tell the driver to reserve structures for coherent allocation, and allocate
  * or free the corresponding memory.
  * @dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
  */
 static int gasket_config_coherent_allocator(
-	struct gasket_dev *gasket_dev, ulong arg)
+	struct gasket_dev *gasket_dev,
+	struct gasket_coherent_alloc_config_ioctl __user *argp)
 {
 	int ret;
 	struct gasket_coherent_alloc_config_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
+	if (copy_from_user(&ibuf, argp,
 			   sizeof(struct gasket_coherent_alloc_config_ioctl)))
 		return -EFAULT;
 
@@ -432,7 +440,7 @@ static int gasket_config_coherent_allocator(
 			gasket_dev, ibuf.size, &ibuf.dma_address,
 			ibuf.page_table_index);
 	}
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/14] staging: gasket: assorted cleanups
  2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
                   ` (33 preceding siblings ...)
  2018-07-21 12:57 ` [PATCH 20/20] staging: gasket: common ioctls " Todd Poynor
@ 2018-07-21 13:17 ` Greg Kroah-Hartman
  2018-07-21 13:30   ` Todd Poynor
  34 siblings, 1 reply; 40+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-21 13:17 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Rob Springer, John Joseph, Ben Chan, devel, Zhongze Hu,
	linux-kernel, Guenter Roeck, Todd Poynor, Dmitry Torokhov

On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
> 
> Various fixups arising from the chromium review of the gasket and apex
> drivers.

Didn't I just apply this whole series?  Why resend it?

confused,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/14] staging: gasket: assorted cleanups
  2018-07-21 13:17 ` [PATCH 00/14] staging: gasket: assorted cleanups Greg Kroah-Hartman
@ 2018-07-21 13:30   ` Todd Poynor
  2018-07-21 13:31     ` Todd Poynor
  2018-07-21 13:40     ` Greg KH
  0 siblings, 2 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 13:30 UTC (permalink / raw)
  To: Greg KH
  Cc: toddpoynor, Rob Springer, John Joseph, benchan, devel, frankhu,
	LKML, groeck, Dmitry Torokhov

On Sat, Jul 21, 2018 at 6:17 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > From: Todd Poynor <toddpoynor@google.com>
> >
> > Various fixups arising from the chromium review of the gasket and apex
> > drivers.
>
> Didn't I just apply this whole series?  Why resend it?

This is another round of cleanups from the chromium review, I can
update the subject to make that more clear.

>
> confused,
>
> greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/14] staging: gasket: assorted cleanups
  2018-07-21 13:30   ` Todd Poynor
@ 2018-07-21 13:31     ` Todd Poynor
  2018-07-21 13:40     ` Greg KH
  1 sibling, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-21 13:31 UTC (permalink / raw)
  To: Greg KH
  Cc: toddpoynor, Rob Springer, John Joseph, benchan, devel, frankhu,
	LKML, groeck, Dmitry Torokhov

On Sat, Jul 21, 2018 at 6:30 AM Todd Poynor <toddpoynor@google.com> wrote:
>
> On Sat, Jul 21, 2018 at 6:17 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > > From: Todd Poynor <toddpoynor@google.com>
> > >
> > > Various fixups arising from the chromium review of the gasket and apex
> > > drivers.
> >
> > Didn't I just apply this whole series?  Why resend it?
>
> This is another round of cleanups from the chromium review, I can
> update the subject to make that more clear.

Oh, something went wrong, will fix up my end.

>
> >
> > confused,
> >
> > greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/14] staging: gasket: assorted cleanups
  2018-07-21 13:30   ` Todd Poynor
  2018-07-21 13:31     ` Todd Poynor
@ 2018-07-21 13:40     ` Greg KH
  1 sibling, 0 replies; 40+ messages in thread
From: Greg KH @ 2018-07-21 13:40 UTC (permalink / raw)
  To: Todd Poynor
  Cc: devel, toddpoynor, frankhu, John Joseph, LKML, Rob Springer,
	groeck, Dmitry Torokhov

On Sat, Jul 21, 2018 at 06:30:33AM -0700, Todd Poynor wrote:
> On Sat, Jul 21, 2018 at 6:17 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > > From: Todd Poynor <toddpoynor@google.com>
> > >
> > > Various fixups arising from the chromium review of the gasket and apex
> > > drivers.
> >
> > Didn't I just apply this whole series?  Why resend it?
> 
> This is another round of cleanups from the chromium review, I can
> update the subject to make that more clear.

Given that the first patch just didn't apply to my tree at all, you need
to just start over with a new series as I think you are not in sync.
Please rebase and resend.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 20/20] staging: gasket: common ioctls add __user annotations
  2018-07-20  3:49 [PATCH 00/20 v4] staging: gasket: sundry fixes and fixups Todd Poynor
@ 2018-07-20  3:49 ` Todd Poynor
  0 siblings, 0 replies; 40+ messages in thread
From: Todd Poynor @ 2018-07-20  3:49 UTC (permalink / raw)
  To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
  Cc: Zhongze Hu, Simon Que, Dmitry Torokhov, Guenter Roeck, devel,
	linux-kernel, Todd Poynor

From: Todd Poynor <toddpoynor@google.com>

Add __user annotation to gasket common ioctl pointer arguments for
sparse checking.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/gasket_ioctl.c | 102 ++++++++++++++------------
 1 file changed, 55 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 998d0e215523c..2e2c9b997093b 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -24,17 +24,24 @@
 #endif
 
 static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd);
-static int gasket_set_event_fd(struct gasket_dev *dev, ulong arg);
+static int gasket_set_event_fd(struct gasket_dev *dev,
+			       struct gasket_interrupt_eventfd __user *argp);
 static int gasket_read_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
 static int gasket_read_simple_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
 static int gasket_partition_page_table(
-	struct gasket_dev *gasket_dev, ulong arg);
-static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg);
-static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp);
+static int gasket_map_buffers(struct gasket_dev *gasket_dev,
+			      struct gasket_page_table_ioctl __user *argp);
+static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
+				struct gasket_page_table_ioctl __user *argp);
 static int gasket_config_coherent_allocator(
-	struct gasket_dev *gasket_dev, ulong arg);
+	struct gasket_dev *gasket_dev,
+	struct gasket_coherent_alloc_config_ioctl __user *argp);
 
 /*
  * standard ioctl dispatch function.
@@ -80,7 +87,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		retval = gasket_reset(gasket_dev, arg);
 		break;
 	case GASKET_IOCTL_SET_EVENTFD:
-		retval = gasket_set_event_fd(gasket_dev, arg);
+		retval = gasket_set_event_fd(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CLEAR_EVENTFD:
 		trace_gasket_ioctl_integer_data(arg);
@@ -89,31 +96,30 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
 		break;
 	case GASKET_IOCTL_PARTITION_PAGE_TABLE:
 		trace_gasket_ioctl_integer_data(arg);
-		retval = gasket_partition_page_table(gasket_dev, arg);
+		retval = gasket_partition_page_table(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_NUMBER_PAGE_TABLES:
 		trace_gasket_ioctl_integer_data(gasket_dev->num_page_tables);
-		if (copy_to_user((void __user *)arg,
-				 &gasket_dev->num_page_tables,
+		if (copy_to_user(argp, &gasket_dev->num_page_tables,
 				 sizeof(uint64_t)))
 			retval = -EFAULT;
 		else
 			retval = 0;
 		break;
 	case GASKET_IOCTL_PAGE_TABLE_SIZE:
-		retval = gasket_read_page_table_size(gasket_dev, arg);
+		retval = gasket_read_page_table_size(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_SIMPLE_PAGE_TABLE_SIZE:
-		retval = gasket_read_simple_page_table_size(gasket_dev, arg);
+		retval = gasket_read_simple_page_table_size(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_MAP_BUFFER:
-		retval = gasket_map_buffers(gasket_dev, arg);
+		retval = gasket_map_buffers(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR:
-		retval = gasket_config_coherent_allocator(gasket_dev, arg);
+		retval = gasket_config_coherent_allocator(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_UNMAP_BUFFER:
-		retval = gasket_unmap_buffers(gasket_dev, arg);
+		retval = gasket_unmap_buffers(gasket_dev, argp);
 		break;
 	case GASKET_IOCTL_CLEAR_INTERRUPT_COUNTS:
 		/* Clear interrupt counts doesn't take an arg, so use 0. */
@@ -218,16 +224,15 @@ static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 /*
  * Associate an eventfd with an interrupt.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_interrupt_eventfd struct in userspace.
+ * @argp: Pointer to gasket_interrupt_eventfd struct in userspace.
  */
-static int gasket_set_event_fd(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
+			       struct gasket_interrupt_eventfd __user *argp)
 {
 	struct gasket_interrupt_eventfd die;
 
-	if (copy_from_user(&die, (void __user *)arg,
-			   sizeof(struct gasket_interrupt_eventfd))) {
+	if (copy_from_user(&die, argp, sizeof(struct gasket_interrupt_eventfd)))
 		return -EFAULT;
-	}
 
 	trace_gasket_ioctl_eventfd_data(die.interrupt, die.event_fd);
 
@@ -238,15 +243,16 @@ static int gasket_set_event_fd(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Reads the size of the page table.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_read_page_table_size(
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret = 0;
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	if (ibuf.page_table_index >= gasket_dev->num_page_tables)
@@ -259,7 +265,7 @@ static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
 		ibuf.page_table_index, ibuf.size, ibuf.host_address,
 		ibuf.device_address);
 
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
@@ -268,16 +274,16 @@ static int gasket_read_page_table_size(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Reads the size of the simple page table.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
 static int gasket_read_simple_page_table_size(
-	struct gasket_dev *gasket_dev, ulong arg)
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret = 0;
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	if (ibuf.page_table_index >= gasket_dev->num_page_tables)
@@ -290,7 +296,7 @@ static int gasket_read_simple_page_table_size(
 		ibuf.page_table_index, ibuf.size, ibuf.host_address,
 		ibuf.device_address);
 
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
@@ -299,16 +305,17 @@ static int gasket_read_simple_page_table_size(
 /*
  * Sets the boundary between the simple and extended page tables.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_partition_page_table(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_partition_page_table(
+	struct gasket_dev *gasket_dev,
+	struct gasket_page_table_ioctl __user *argp)
 {
 	int ret;
 	struct gasket_page_table_ioctl ibuf;
 	uint max_page_table_size;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -340,14 +347,14 @@ static int gasket_partition_page_table(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Maps a userspace buffer to a device virtual address.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_map_buffers(struct gasket_dev *gasket_dev,
+			      struct gasket_page_table_ioctl __user *argp)
 {
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -370,14 +377,14 @@ static int gasket_map_buffers(struct gasket_dev *gasket_dev, ulong arg)
 /*
  * Unmaps a userspace buffer from a device virtual address.
  * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_page_table_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
  */
-static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg)
+static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
+				struct gasket_page_table_ioctl __user *argp)
 {
 	struct gasket_page_table_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
-			   sizeof(struct gasket_page_table_ioctl)))
+	if (copy_from_user(&ibuf, argp, sizeof(struct gasket_page_table_ioctl)))
 		return -EFAULT;
 
 	trace_gasket_ioctl_page_table_data(
@@ -402,15 +409,16 @@ static int gasket_unmap_buffers(struct gasket_dev *gasket_dev, ulong arg)
  * Tell the driver to reserve structures for coherent allocation, and allocate
  * or free the corresponding memory.
  * @dev: Pointer to the current gasket_dev we're using.
- * @arg: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
+ * @argp: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
  */
 static int gasket_config_coherent_allocator(
-	struct gasket_dev *gasket_dev, ulong arg)
+	struct gasket_dev *gasket_dev,
+	struct gasket_coherent_alloc_config_ioctl __user *argp)
 {
 	int ret;
 	struct gasket_coherent_alloc_config_ioctl ibuf;
 
-	if (copy_from_user(&ibuf, (void __user *)arg,
+	if (copy_from_user(&ibuf, argp,
 			   sizeof(struct gasket_coherent_alloc_config_ioctl)))
 		return -EFAULT;
 
@@ -432,7 +440,7 @@ static int gasket_config_coherent_allocator(
 			gasket_dev, ibuf.size, &ibuf.dma_address,
 			ibuf.page_table_index);
 	}
-	if (copy_to_user((void __user *)arg, &ibuf, sizeof(ibuf)))
+	if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
 		return -EFAULT;
 
 	return ret;
-- 
2.18.0.233.g985f88cf7e-goog


^ permalink raw reply related	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2018-07-21 13:40 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
2018-07-21 12:56 ` [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig Todd Poynor
2018-07-21 12:56 ` [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param Todd Poynor
2018-07-21 12:56 ` [PATCH 02/20] staging: gasket: gasket_enable_dev remove unnecessary variable Todd Poynor
2018-07-21 12:56 ` [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback Todd Poynor
2018-07-21 12:56 ` [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param Todd Poynor
2018-07-21 12:56 ` [PATCH 03/20] staging: gasket: remove code for no physical device Todd Poynor
2018-07-21 12:56 ` [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config Todd Poynor
2018-07-21 12:56 ` [PATCH 04/20] staging: gasket: fix class create bug handling Todd Poynor
2018-07-21 12:56 ` [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long Todd Poynor
2018-07-21 12:56 ` [PATCH 05/20] staging: gasket: remove unnecessary code in coherent allocator Todd Poynor
2018-07-21 12:56 ` [PATCH 06/20] staging: gasket: don't treat no device reset callback as an error Todd Poynor
2018-07-21 12:56 ` [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS Todd Poynor
2018-07-21 12:56 ` [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev Todd Poynor
2018-07-21 12:56 ` [PATCH 07/20] staging: gasket: gasket_mmap return error instead of valid BAR index Todd Poynor
2018-07-21 12:56 ` [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute Todd Poynor
2018-07-21 12:56 ` [PATCH 08/20] staging: gasket: apex_clock_gating simplify logic, reduce indentation Todd Poynor
2018-07-21 12:56 ` [PATCH 09/14] staging: gasket: core: convert various logs to debug level Todd Poynor
2018-07-21 12:56 ` [PATCH 09/20] staging: gasket: gasket page table functions use bool return type Todd Poynor
2018-07-21 12:56 ` [PATCH 10/14] staging: gasket: interrupts: convert various logs to debug level Todd Poynor
2018-07-21 12:56 ` [PATCH 10/20] staging: gasket: remove else clause after return in if clause Todd Poynor
2018-07-21 12:57 ` [PATCH 11/20] staging: gasket: fix comment syntax in apex.h Todd Poynor
2018-07-21 12:57 ` [PATCH 11/14] staging: gasket: ioctl common: convert various logs to debug level Todd Poynor
2018-07-21 12:57 ` [PATCH 12/14] staging: gasket: page table: " Todd Poynor
2018-07-21 12:57 ` [PATCH 12/20] staging: gasket: remove unnecessary parens in page table code Todd Poynor
2018-07-21 12:57 ` [PATCH 13/20] staging: gasket: gasket_mmap use PAGE_MASK Todd Poynor
2018-07-21 12:57 ` [PATCH 13/14] staging: gasket: page table: remove unnecessary logs Todd Poynor
2018-07-21 12:57 ` [PATCH 14/14] staging: gasket: apex: convert various logs to debug level Todd Poynor
2018-07-21 12:57 ` [PATCH 14/20] staging: gasket: remove extra parens in gasket_write_mappable_regions Todd Poynor
2018-07-21 12:57 ` [PATCH 15/20] staging: gasket: fix multi-line comment syntax in gasket_core.h Todd Poynor
2018-07-21 12:57 ` [PATCH 16/20] staging: gasket: always allow root open for write Todd Poynor
2018-07-21 12:57 ` [PATCH 17/20] staging: gasket: top ioctl handler add __user annotations Todd Poynor
2018-07-21 12:57 ` [PATCH 18/20] staging: gasket: apex ioctl " Todd Poynor
2018-07-21 12:57 ` [PATCH 19/20] staging: gasket: common ioctl dispatcher " Todd Poynor
2018-07-21 12:57 ` [PATCH 20/20] staging: gasket: common ioctls " Todd Poynor
2018-07-21 13:17 ` [PATCH 00/14] staging: gasket: assorted cleanups Greg Kroah-Hartman
2018-07-21 13:30   ` Todd Poynor
2018-07-21 13:31     ` Todd Poynor
2018-07-21 13:40     ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2018-07-20  3:49 [PATCH 00/20 v4] staging: gasket: sundry fixes and fixups Todd Poynor
2018-07-20  3:49 ` [PATCH 20/20] staging: gasket: common ioctls add __user annotations Todd Poynor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).