From: Todd Poynor <[email protected]>
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
From: Todd Poynor <[email protected]>
It is not an error for a device to not have a reset callback registered.
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Remove unneeded parentheses around subexpressions.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Add __user annotation to gasket common ioctl pointer arguments for
sparse checking.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Add __user annotation to gasket core common ioctl pointer arguments for
sparse checking.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Add __user annotation to gasket_core top-level ioctl handling pointer
arguments, for sparse checking.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Use consistent kernel-style multi-line comment syntax.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
gasket_mmap use PAGE_MASK, instead of performing math on PAGE_SIZE, for
simplicity and clarity.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Add __user annotation to ioctl pointer argument, for sparse checking.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Convert from int to bool return type for gasket page table functions
that return values used as booleans.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
gasket_alloc_coherent_memory() extra parentheses in statement.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Use kernel-style multi-line comment syntax.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Else after return is unnecessary and may cause static code checkers to
complain.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
class_create() never returns NULL, and this driver should never return
PTR_ERR(NULL) anyway.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
Reviewed-by: Dmitry Torokhov <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Collapse together two checks and return immediately, avoid conditional
indentation for most of function code.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Simon Que <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Remove extraneous statement in gasket_config_coherent_allocator()
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
gasket_enable_dev code for enabling a gasket device with no physical PCI
device registered shouldn't be necessary.
Reported-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Page alignment error log should print the offending value as an unsigned
long, not as a kernel pointer.
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
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 <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Pass the return value from the device ioctl permissions callback to the
tracepoint when the callback returns an error.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
The message should be passed the callback function pointer, not
the pointer to the gasket device.
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
The gasket and apex drivers are also to be used on ARM64 architectures.
Signed-off-by: Todd Poynor <[email protected]>
---
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
From: Todd Poynor <[email protected]>
Remove unnecessary variable, pass constant param instead.
Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Zhongze Hu <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
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
On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> From: Todd Poynor <[email protected]>
>
> 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
On Sat, Jul 21, 2018 at 6:17 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > From: Todd Poynor <[email protected]>
> >
> > 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
On Sat, Jul 21, 2018 at 6:30 AM Todd Poynor <[email protected]> wrote:
>
> On Sat, Jul 21, 2018 at 6:17 AM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > > From: Todd Poynor <[email protected]>
> > >
> > > 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
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
> <[email protected]> wrote:
> >
> > On Sat, Jul 21, 2018 at 05:56:39AM -0700, Todd Poynor wrote:
> > > From: Todd Poynor <[email protected]>
> > >
> > > 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