2018-07-29 19:38:03

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 00/13] staging: gasket: fixes and cleanups

From: Todd Poynor <[email protected]>

Fixes for device reference counting and root access based on user
namespace for containers, plus cleanups of comments, forward
declarations for static functions (more forthcoming) and multi-line
continuation style (more to come).

Todd Poynor (13):
staging: gasket: core: hold reference to pci_dev while used
staging: gasket: sysfs: hold reference to device while in use
staging: gasket: page table: hold references to device and pci_dev
staging: gasket: core: allow root access based on user namespace
staging: gasket: apex: simplify comments for static functions
staging: gasket: core: simplify comments for static functions
staging: gasket: ioctl: simplify comments for static functions
staging: gasket: page table: simplify comments for static functions
staging: gasket: interrupt: simplify comments for static functions
staging: gasket: sysfs: simplify comments for static functions
staging: gasket: TODO: remove entry for static function kernel docs
staging: gasket: apex: remove static function forward declarations
staging: gasket: apex: fix function param line continuation style

drivers/staging/gasket/TODO | 1 -
drivers/staging/gasket/apex_driver.c | 559 +++++++++------------
drivers/staging/gasket/gasket_core.c | 165 ++----
drivers/staging/gasket/gasket_interrupt.c | 18 +-
drivers/staging/gasket/gasket_ioctl.c | 51 +-
drivers/staging/gasket/gasket_page_table.c | 329 ++----------
drivers/staging/gasket/gasket_sysfs.c | 39 +-
7 files changed, 350 insertions(+), 812 deletions(-)

--
2.18.0.345.g5c9ce644c3-goog



2018-07-29 19:38:21

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 09/13] staging: gasket: interrupt: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_interrupt.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 3be8e24c126d0..27fde991edc6b 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -87,13 +87,6 @@ static struct gasket_sysfs_attribute interrupt_sysfs_attrs[] = {
GASKET_END_OF_ATTR_ARRAY,
};

-/*
- * Set up device registers for interrupt handling.
- * @gasket_dev: The Gasket information structure for this device.
- *
- * Sets up the device registers with the correct indices for the relevant
- * interrupts.
- */
static void gasket_interrupt_setup(struct gasket_dev *gasket_dev);

/* MSIX init and cleanup. */
@@ -334,13 +327,7 @@ int gasket_interrupt_reset_counts(struct gasket_dev *gasket_dev)
return 0;
}

-/*
- * Set up device registers for interrupt handling.
- * @gasket_dev: The Gasket information structure for this device.
- *
- * Sets up the device registers with the correct indices for the relevant
- * interrupts.
- */
+/* Set up device registers for interrupt handling. */
static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
{
int i;
@@ -553,9 +540,6 @@ static ssize_t interrupt_sysfs_show(
return ret;
}

-/*
- * MSIX interrupt handler, used with PCI driver.
- */
static irqreturn_t gasket_msix_interrupt_handler(int irq, void *dev_id)
{
struct eventfd_ctx *ctx;
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:38:22

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 08/13] staging: gasket: page table: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_page_table.c | 323 +++------------------
1 file changed, 48 insertions(+), 275 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 6b946a155ee3a..b42f6637b909b 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -635,27 +635,9 @@ int gasket_page_table_system_status(struct gasket_page_table *page_table)
return GASKET_STATUS_ALIVE;
}

-/* Internal functions */
-
-/* Mapping functions */
/*
* Allocate and map pages to simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @host_addr: Starting host virtual memory address of the pages.
- * @dev_addr: Starting device address of the pages.
- * @cnt: Count of the number of device pages to map.
- *
- * Description: gasket_map_simple_pages calls gasket_simple_alloc_pages() to
- * allocate the page table slots, then calls
- * gasket_perform_mapping() to actually do the work of mapping the
- * pages into the the simple page table (device translation table
- * registers).
- *
- * The sd_mutex must be held when gasket_map_simple_pages() is
- * called.
- *
- * Returns 0 if successful or a non-zero error number otherwise.
- * If there is an error, no pages are mapped.
+ * If there is an error, no pages are mapped.
*/
static int gasket_map_simple_pages(
struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr,
@@ -685,22 +667,7 @@ static int gasket_map_simple_pages(

/*
* gasket_map_extended_pages - Get and map buffers to extended addresses.
- * @pg_tbl: Gasket page table pointer.
- * @host_addr: Starting host virtual memory address of the pages.
- * @dev_addr: Starting device address of the pages.
- * @num_pages: The number of device pages to map.
- *
- * Description: gasket_map_extended_buffers calls
- * gasket_alloc_extended_entries() to allocate the page table
- * slots, then loops over the level 0 page table entries, and for
- * each calls gasket_perform_mapping() to map the buffers into the
- * level 1 page table for that level 0 entry.
- *
- * The page table mutex must be held when
- * gasket_map_extended_pages() is called.
- *
- * Returns 0 if successful or a non-zero error number otherwise.
- * If there is an error, no pages are mapped.
+ * If there is an error, no pages are mapped.
*/
static int gasket_map_extended_pages(
struct gasket_page_table *pg_tbl, ulong host_addr, ulong dev_addr,
@@ -756,32 +723,11 @@ static int gasket_map_extended_pages(

/*
* Get and map last level page table buffers.
- * @pg_tbl: Gasket page table pointer.
- * @ptes: Array of page table entries to describe this mapping, one per
- * page to map.
- * @slots: Location(s) to write device-mapped page address. If this is a simple
- * mapping, these will be address translation registers. If this is
- * an extended mapping, these will be within a second-level page table
- * allocated by the host and so must have their __iomem attribute
- * casted away.
- * @host_addr: Starting [host] virtual memory address of the buffers.
- * @num_pages: The number of device pages to map.
- * @is_simple_mapping: 1 if this is a simple mapping, 0 otherwise.
- *
- * Description: gasket_perform_mapping calls get_user_pages() to get pages
- * of user memory and pin them. It then calls dma_map_page() to
- * map them for DMA. Finally, the mapped DMA addresses are written
- * into the page table.
*
- * This function expects that the page table entries are
- * already allocated. The level argument determines how the
- * final page table entries are written: either into PCIe memory
- * mapped space for a level 0 page table or into kernel memory
- * for a level 1 page table.
- *
- * The page pointers are saved for later releasing the pages.
- *
- * Returns 0 if successful or a non-zero error number otherwise.
+ * slots is the location(s) to write device-mapped page address. If this is a
+ * simple mapping, these will be address translation registers. If this is
+ * an extended mapping, these will be within a second-level page table
+ * allocated by the host and so must have their __iomem attribute casted away.
*/
static int gasket_perform_mapping(
struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *ptes,
@@ -866,21 +812,9 @@ static int gasket_perform_mapping(
return 0;
}

-/**
+/*
* Allocate page table entries in a simple table.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address for the (eventual) mappings.
- * @num_pages: Count of pages to be mapped.
- *
- * Description: gasket_alloc_simple_entries checks to see if a range of page
- * table slots are available. As long as the sd_mutex is
- * held, the slots will be available.
- *
- * The page table mutex must be held when
- * gasket_alloc_simple entries() is called.
- *
- * Returns 0 if successful, or non-zero if the requested device
- * addresses are not available.
+ * The page table mutex must be held by the caller.
*/
static int gasket_alloc_simple_entries(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -893,29 +827,19 @@ static int gasket_alloc_simple_entries(
return 0;
}

-/**
- * Allocate slots in an extended page table.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address for the (eventual) mappings.
- * @num_pages: Count of pages to be mapped.
- *
- * Description: gasket_alloc_extended_entries checks to see if a range of page
- * table slots are available. If necessary, memory is allocated for
- * second level page tables.
- *
- * Note that memory for second level page tables is allocated
- * as needed, but that memory is only freed on the final close
- * of the device file, when the page tables are repartitioned,
- * or the the device is removed. If there is an error or if
- * the full range of slots is not available, any memory
- * allocated for second level page tables remains allocated
- * until final close, repartition, or device removal.
+/*
+ * Allocate slots in an extended page table. Check to see if a range of page
+ * table slots are available. If necessary, memory is allocated for second level
+ * page tables.
*
- * The page table mutex must be held when
- * gasket_alloc_extended_entries() is called.
+ * Note that memory for second level page tables is allocated as needed, but
+ * that memory is only freed on the final close of the device file, when the
+ * page tables are repartitioned, or the the device is removed. If there is an
+ * error or if the full range of slots is not available, any memory
+ * allocated for second level page tables remains allocated until final close,
+ * repartition, or device removal.
*
- * Returns 0 if successful, or non-zero if the slots are
- * not available.
+ * The page table mutex must be held by the caller.
*/
static int gasket_alloc_extended_entries(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_entries)
@@ -958,21 +882,9 @@ static int gasket_alloc_extended_entries(
return 0;
}

-/**
+/*
* Allocate a second level page table.
- * @pg_tbl: Gasket page table pointer.
- * @pte: Extended page table entry under/for which to allocate a second level.
- * @slot: [Device] slot corresponding to pte.
- *
- * Description: Allocate the memory for a second level page table (subtable) at
- * the given level 0 entry. Then call dma_map_page() to map the
- * second level page table for DMA. Finally, write the
- * mapped DMA address into the device page table.
- *
- * The page table mutex must be held when
- * gasket_alloc_extended_subtable() is called.
- *
- * Returns 0 if successful, or a non-zero error otherwise.
+ * The page table mutex must be held by the caller.
*/
static int gasket_alloc_extended_subtable(
struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *pte,
@@ -1017,15 +929,9 @@ static int gasket_alloc_extended_subtable(
return 0;
}

-/* Unmapping functions */
/*
* Non-locking entry to unmapping routines.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: Starting device address of the pages to unmap.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: Version of gasket_unmap_pages that assumes the page table lock
- * is held.
+ * The page table mutex must be held by the caller.
*/
static void gasket_page_table_unmap_nolock(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1041,14 +947,7 @@ static void gasket_page_table_unmap_nolock(

/*
* Unmap and release pages mapped to simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address of the buffers.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: gasket_simple_unmap_pages calls gasket_perform_unmapping() to
- * unmap and release the buffers in the level 0 page table.
- *
- * The sd_mutex must be held when gasket_unmap_simple_pages() is called.
+ * The page table mutex must be held by the caller.
*/
static void gasket_unmap_simple_pages(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1059,20 +958,9 @@ static void gasket_unmap_simple_pages(
pg_tbl->base_slot + slot, num_pages, 1);
}

-/**
+/*
* Unmap and release buffers to extended addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: Starting device address of the pages to unmap.
- * @addr: Starting device address of the buffers.
- * @num_pages: The number of device pages to unmap.
- *
- * Description: gasket_extended_unmap_pages loops over the level 0 page table
- * entries, and for each calls gasket_perform_unmapping() to unmap
- * the buffers from the level 1 page [sub]table for that level 0
- * entry.
- *
- * The page table mutex must be held when
- * gasket_unmap_extended_pages() is called.
+ * The page table mutex must be held by the caller.
*/
static void gasket_unmap_extended_pages(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1106,28 +994,7 @@ static void gasket_unmap_extended_pages(

/*
* Unmap and release mapped pages.
- * @pg_tbl: Gasket page table pointer.
- * @ptes: Array of page table entries to describe the mapped range, one per
- * page to unmap.
- * @slots: Device slots corresponding to the mappings described by "ptes".
- * As with ptes, one element per page to unmap.
- * If these are simple mappings, these will be address translation
- * registers. If these are extended mappings, these will be witin a
- * second-level page table allocated on the host, and so must have
- * their __iomem attribute casted away.
- * @num_pages: Number of pages to unmap.
- * @is_simple_mapping: 1 if this is a simple mapping, 0 otherwise.
- *
- * Description: gasket_perform_unmapping() loops through the metadata entries
- * in a last level page table (simple table or extended subtable),
- * and for each page:
- * - Unmaps the page from DMA space (dma_unmap_page),
- * - Returns the page to the OS (gasket_release_page),
- * The entry in the page table is written to 0. The metadata
- * type is set to PTE_FREE and the metadata is all reset
- * to 0.
- *
- * The page table mutex must be held when this function is called.
+ * The page table mutex must be held by the caller.
*/
static void gasket_perform_unmapping(
struct gasket_page_table *pg_tbl, struct gasket_page_table_entry *ptes,
@@ -1165,17 +1032,6 @@ static void gasket_perform_unmapping(

/*
* Free a second level page [sub]table.
- * @pg_tbl: Gasket page table pointer.
- * @pte: Page table entry _pointing_to_ the subtable to free.
- * @slot: Device slot holding a pointer to the sublevel's contents.
- *
- * Description: Safely deallocates a second-level [sub]table by:
- * - Marking the containing first-level PTE as free
- * - Setting the corresponding [extended] device slot as NULL
- * - Unmapping the PTE from DMA space.
- * - Freeing the subtable's memory.
- * - Deallocating the page and clearing out the PTE.
- *
* The page table mutex must be held before this call.
*/
static void gasket_free_extended_subtable(
@@ -1202,12 +1058,7 @@ static void gasket_free_extended_subtable(
memset(pte, 0, sizeof(struct gasket_page_table_entry));
}

-/*
- * Safely return a page to the OS.
- * @page: The page to return to the OS.
- * Returns true if the page was released, false if it was
- * ignored.
- */
+/* Safely return a page to the OS. */
static bool gasket_release_page(struct page *page)
{
if (!page)
@@ -1229,13 +1080,10 @@ static inline bool gasket_addr_is_simple(

/*
* Validity checking for simple addresses.
- * @pg_tbl: Gasket page table pointer.
- * @dev_addr: The device address to which the pages will be mapped.
- * @num_pages: The number of pages in the range to consider.
*
- * Description: This call verifies that address translation commutes (from
- * address to/from page + offset) and that the requested page range starts and
- * ends within the set of currently-partitioned simple pages.
+ * Verify that address translation commutes (from address to/from page + offset)
+ * and that the requested page range starts and ends within the set of
+ * currently-partitioned simple pages.
*/
static bool gasket_is_simple_dev_addr_bad(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1269,13 +1117,11 @@ static bool gasket_is_simple_dev_addr_bad(
}

/*
- * Verifies that address translation commutes (from address to/from page +
- * offset) and that the requested page range starts and ends within the set of
- * currently-partitioned simple pages.
+ * Validity checking for extended addresses.
*
- * @pg_tbl: Gasket page table pointer.
- * @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.
+ * Verify that address translation commutes (from address to/from page +
+ * offset) and that the requested page range starts and ends within the set of
+ * currently-partitioned extended pages.
*/
static bool gasket_is_extended_dev_addr_bad(
struct gasket_page_table *pg_tbl, ulong dev_addr, uint num_pages)
@@ -1331,15 +1177,8 @@ static bool gasket_is_extended_dev_addr_bad(
}

/*
- * Checks if a range of PTEs is free.
- * @ptes: The set of PTEs to check.
- * @num_entries: The number of PTEs to check.
- *
- * Description: Iterates over the input PTEs to determine if all have been
- * marked as FREE or if any are INUSE. In the former case, 1/true is returned.
- * Otherwise, 0/false is returned.
- *
- * The page table mutex must be held before this call.
+ * Check if a range of PTEs is free.
+ * The page table mutex must be held by the caller.
*/
static bool gasket_is_pte_range_free(
struct gasket_page_table_entry *ptes, uint num_entries)
@@ -1356,10 +1195,7 @@ static bool gasket_is_pte_range_free(

/*
* Actually perform collection.
- * @pg_tbl: Gasket page table structure.
- *
- * Description: Version of gasket_page_table_garbage_collect that assumes the
- * page table lock is held.
+ * The page table mutex must be held by the caller.
*/
static void gasket_page_table_garbage_collect_nolock(
struct gasket_page_table *pg_tbl)
@@ -1384,14 +1220,7 @@ static void gasket_page_table_garbage_collect_nolock(
}

/*
- * Converts components to a device address.
- * @pg_tbl: Gasket page table structure.
- * @is_simple: nonzero if this should be a simple entry, zero otherwise.
- * @page_index: The page index into the respective table.
- * @offset: The offset within the requested page.
- *
- * Simple utility function to convert (simple, page, offset) into a device
- * address.
+ * Convert (simple, page, offset) into a device address.
* Examples:
* Simple page 0, offset 32:
* Input (0, 0, 32), Output 0x20
@@ -1429,14 +1258,7 @@ static ulong gasket_components_to_dev_address(
}

/*
- * Gets the index of the address' page in the simple table.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as a simple address and determines the
- * index of its underlying page in the simple page table (i.e., device address
- * translation registers.
- *
+ * Return the index of the page for the address in the simple table.
* Does not perform validity checking.
*/
static int gasket_simple_page_idx(
@@ -1447,14 +1269,7 @@ static int gasket_simple_page_idx(
}

/*
- * Gets the level 0 page index for the given address.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as an extended address and determines
- * the index of its underlying page in the first-level extended page table
- * (i.e., device extended address translation registers).
- *
+ * Return the level 0 page index for the given address.
* Does not perform validity checking.
*/
static ulong gasket_extended_lvl0_page_idx(
@@ -1465,14 +1280,7 @@ static ulong gasket_extended_lvl0_page_idx(
}

/*
- * Gets the level 1 page index for the given address.
- * @pg_tbl: Gasket page table structure.
- * @dev_addr: The address whose page index to retrieve.
- *
- * Description: Treats the input address as an extended address and determines
- * the index of its underlying page in the second-level extended page table
- * (i.e., host memory pointed to by a first-level page table entry).
- *
+ * Return the level 1 page index for the given address.
* Does not perform validity checking.
*/
static ulong gasket_extended_lvl1_page_idx(
@@ -1483,13 +1291,10 @@ static ulong gasket_extended_lvl1_page_idx(
}

/*
- * Determines whether a host buffer was mapped as coherent memory.
- * @pg_tbl: gasket_page_table structure tracking the host buffer mapping
- * @host_addr: user virtual address within a host buffer
+ * Return whether a host buffer was mapped as coherent memory.
*
- * Description: A Gasket page_table currently support one contiguous
- * dma range, mapped to one contiguous virtual memory range. Check if the
- * host_addr is within start of page 0, and end of last page, for that range.
+ * A Gasket page_table currently support one contiguous dma range, mapped to one
+ * contiguous virtual memory range. Check if the host_addr is within that range.
*/
static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr)
{
@@ -1505,16 +1310,7 @@ static int is_coherent(struct gasket_page_table *pg_tbl, ulong host_addr)
return min <= host_addr && host_addr < max;
}

-/*
- * Records the host_addr to coherent dma memory mapping.
- * @gasket_dev: Gasket Device.
- * @size: Size of the virtual address range to map.
- * @dma_address: Dma address within the coherent memory range.
- * @vma: Virtual address we wish to map to coherent memory.
- *
- * Description: For each page in the virtual address range, record the
- * coherent page mgasket_pretapping.
- */
+/* Record the host_addr to coherent dma memory mapping. */
int gasket_set_user_virt(
struct gasket_dev *gasket_dev, u64 size, dma_addr_t dma_address,
ulong vma)
@@ -1541,16 +1337,7 @@ int gasket_set_user_virt(
return 0;
}

-/*
- * Allocate a block of coherent memory.
- * @gasket_dev: Gasket Device.
- * @size: Size of the memory block.
- * @dma_address: Dma address allocated by the kernel.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Allocate a contiguous coherent memory block, DMA'ble
- * by this device.
- */
+/* Allocate a block of coherent memory. */
int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
dma_addr_t *dma_address, u64 index)
{
@@ -1613,15 +1400,7 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
return -ENOMEM;
}

-/*
- * Free a block of coherent memory.
- * @gasket_dev: Gasket Device.
- * @size: Size of the memory block.
- * @dma_address: Dma address allocated by the kernel.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Release memory allocated thru gasket_alloc_coherent_memory.
- */
+/* Free a block of coherent memory. */
int gasket_free_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
dma_addr_t dma_address, u64 index)
{
@@ -1647,13 +1426,7 @@ int gasket_free_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
return 0;
}

-/*
- * Release all coherent memory.
- * @gasket_dev: Gasket Device.
- * @index: Index of the gasket_page_table within this Gasket device
- *
- * Description: Release all memory allocated thru gasket_alloc_coherent_memory.
- */
+/* Release all coherent memory. */
void gasket_free_coherent_memory_all(
struct gasket_dev *gasket_dev, u64 index)
{
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:38:27

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 10/13] staging: gasket: sysfs: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_sysfs.c | 28 ++++-----------------------
1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index fde04658419bc..ef4eca02afa63 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -47,22 +47,13 @@ struct gasket_sysfs_mapping {
*/
static struct gasket_sysfs_mapping dev_mappings[GASKET_SYSFS_NUM_MAPPINGS];

-/*
- * Callback when a mapping's refcount goes to zero.
- * @ref: The reference count of the containing sysfs mapping.
- */
+/* Callback when a mapping's refcount goes to zero. */
static void release_entry(struct kref *ref)
{
/* All work is done after the return from kref_put. */
}

-/*
- * Looks up mapping information for the given device.
- * @device: The device whose mapping to look for.
- *
- * Looks up the requested device and takes a reference and returns it if found,
- * and returns NULL otherwise.
- */
+/* Look up mapping information for the given device. */
static struct gasket_sysfs_mapping *get_mapping(struct device *device)
{
int i;
@@ -82,17 +73,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device *device)
return NULL;
}

-/*
- * Returns a reference to a mapping.
- * @mapping: The mapping we're returning.
- *
- * Decrements the refcount for the given mapping (if valid). If the refcount is
- * zero, then it cleans up the mapping - in this function as opposed to the
- * kref_put callback, due to a potential deadlock.
- *
- * Although put_mapping_n exists, this function is left here (as an implicit
- * put_mapping_n(..., 1) for convenience.
- */
+/* Put a reference to a mapping. */
static void put_mapping(struct gasket_sysfs_mapping *mapping)
{
int i;
@@ -140,8 +121,7 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
}

/*
- * Returns a reference N times.
- * @mapping: The mapping to return.
+ * Put a reference to a mapping N times.
*
* In higher-level resource acquire/release function pairs, the release function
* will need to release a mapping 2x - once for the refcount taken in the
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:38:35

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 12/13] staging: gasket: apex: remove static function forward declarations

From: Todd Poynor <[email protected]>

Remove forward declarations of static functions, move code to avoid
forward references, for kernel style.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/apex_driver.c | 416 ++++++++++++---------------
1 file changed, 190 insertions(+), 226 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index a756764751777..f70fea0d80ecf 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -123,55 +123,6 @@ static struct gasket_page_table_config apex_page_table_configs[NUM_NODES] = {
},
};

-/* Function declarations */
-static int __init apex_init(void);
-static void apex_exit(void);
-
-static int apex_add_dev_cb(struct gasket_dev *gasket_dev);
-
-static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev);
-
-static int apex_device_cleanup(struct gasket_dev *gasket_dev);
-
-static int apex_device_open_cb(struct gasket_dev *gasket_dev);
-
-static ssize_t sysfs_show(
- struct device *device, struct device_attribute *attr, char *buf);
-
-static int apex_reset(struct gasket_dev *gasket_dev, uint type);
-
-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, void __user *argp);
-
-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);
-
-static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type);
-
-static bool is_gcb_in_reset(struct gasket_dev *gasket_dev);
-
-/* Data definitions */
-
-/* The data necessary to display this file's sysfs entries. */
-static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
- GASKET_SYSFS_RO(node_0_page_table_entries, sysfs_show,
- ATTR_KERNEL_HIB_PAGE_TABLE_SIZE),
- GASKET_SYSFS_RO(node_0_simple_page_table_entries, sysfs_show,
- ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE),
- GASKET_SYSFS_RO(node_0_num_mapped_pages, sysfs_show,
- ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES),
- GASKET_END_OF_ATTR_ARRAY
-};
-
-static const struct pci_device_id apex_pci_ids[] = {
- { PCI_DEVICE(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID) }, { 0 }
-};
-
/* The regions in the BAR2 space that can be mapped into user space. */
static const struct gasket_mappable_region mappable_regions[NUM_REGIONS] = {
{ 0x40000, 0x1000 },
@@ -251,65 +202,6 @@ static struct gasket_interrupt_desc apex_interrupts[] = {
},
};

-static struct gasket_driver_desc apex_desc = {
- .name = "apex",
- .driver_version = APEX_DRIVER_VERSION,
- .major = 120,
- .minor = 0,
- .module = THIS_MODULE,
- .pci_id_table = apex_pci_ids,
-
- .num_page_tables = NUM_NODES,
- .page_table_bar_index = APEX_BAR_INDEX,
- .page_table_configs = apex_page_table_configs,
- .page_table_extended_bit = APEX_EXTENDED_SHIFT,
-
- .bar_descriptions = {
- GASKET_UNUSED_BAR,
- GASKET_UNUSED_BAR,
- { APEX_BAR_BYTES, (VM_WRITE | VM_READ), APEX_BAR_OFFSET,
- NUM_REGIONS, mappable_regions, PCI_BAR },
- GASKET_UNUSED_BAR,
- GASKET_UNUSED_BAR,
- GASKET_UNUSED_BAR,
- },
- .coherent_buffer_description = {
- APEX_CH_MEM_BYTES,
- (VM_WRITE | VM_READ),
- APEX_CM_OFFSET,
- },
- .interrupt_type = PCI_MSIX,
- .interrupt_bar_index = APEX_BAR_INDEX,
- .num_interrupts = APEX_INTERRUPT_COUNT,
- .interrupts = apex_interrupts,
- .interrupt_pack_width = 7,
-
- .add_dev_cb = apex_add_dev_cb,
- .remove_dev_cb = NULL,
-
- .enable_dev_cb = NULL,
- .disable_dev_cb = NULL,
-
- .sysfs_setup_cb = apex_sysfs_setup_cb,
- .sysfs_cleanup_cb = NULL,
-
- .device_open_cb = apex_device_open_cb,
- .device_close_cb = apex_device_cleanup,
-
- .ioctl_handler_cb = apex_ioctl,
- .device_status_cb = apex_get_status,
- .hardware_revision_cb = NULL,
- .device_reset_cb = apex_reset,
-};
-
-/* Module registration boilerplate */
-MODULE_DESCRIPTION("Google Apex driver");
-MODULE_VERSION(APEX_DRIVER_VERSION);
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("John Joseph <[email protected]>");
-MODULE_DEVICE_TABLE(pci, apex_pci_ids);
-module_init(apex_init);
-module_exit(apex_exit);

/* Allows device to enter power save upon driver close(). */
static int allow_power_save;
@@ -329,61 +221,6 @@ module_param(allow_sw_clock_gating, int, 0644);
module_param(allow_hw_clock_gating, int, 0644);
module_param(bypass_top_level, int, 0644);

-static int __init apex_init(void)
-{
- return gasket_register_device(&apex_desc);
-}
-
-static void apex_exit(void)
-{
- gasket_unregister_device(&apex_desc);
-}
-
-static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
-{
- ulong page_table_ready, msix_table_ready;
- int retries = 0;
-
- apex_reset(gasket_dev, 0);
-
- while (retries < APEX_RESET_RETRY) {
- page_table_ready =
- gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
- msix_table_ready =
- gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
- if (page_table_ready && msix_table_ready)
- break;
- schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
- retries++;
- }
-
- if (retries == APEX_RESET_RETRY) {
- if (!page_table_ready)
- dev_err(gasket_dev->dev, "Page table init timed out\n");
- if (!msix_table_ready)
- dev_err(gasket_dev->dev, "MSI-X table init timed out\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
-{
- return gasket_sysfs_create_entries(
- gasket_dev->dev_info.device, apex_sysfs_attrs);
-}
-
-/* On device open, perform a core reinit reset. */
-static int apex_device_open_cb(struct gasket_dev *gasket_dev)
-{
- return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
-}
-
/* Check the device status registers and return device status ALIVE or DEAD. */
static int apex_get_status(struct gasket_dev *gasket_dev)
{
@@ -391,53 +228,6 @@ static int apex_get_status(struct gasket_dev *gasket_dev)
return GASKET_STATUS_ALIVE;
}

-/* Reset the Apex hardware. Called on final close via device_close_cb. */
-static int apex_device_cleanup(struct gasket_dev *gasket_dev)
-{
- u64 scalar_error;
- u64 hib_error;
- int ret = 0;
-
- hib_error = gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
- scalar_error = gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
-
- dev_dbg(gasket_dev->dev,
- "%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
- __func__, gasket_dev, hib_error, scalar_error);
-
- if (allow_power_save)
- ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
-
- return ret;
-}
-
-/* Reset the hardware, then quit reset. Called on device open. */
-static int apex_reset(struct gasket_dev *gasket_dev, uint type)
-{
- int ret;
-
- if (bypass_top_level)
- return 0;
-
- if (!is_gcb_in_reset(gasket_dev)) {
- /* We are not in reset - toggle the reset bit so as to force
- * re-init of custom block
- */
- dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
-
- ret = apex_enter_reset(gasket_dev, type);
- if (ret)
- return ret;
- }
- ret = apex_quit_reset(gasket_dev, type);
-
- return ret;
-}
-
/* Enter GCB reset state. */
static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
{
@@ -576,6 +366,30 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
return 0;
}

+/* Reset the Apex hardware. Called on final close via device_close_cb. */
+static int apex_device_cleanup(struct gasket_dev *gasket_dev)
+{
+ u64 scalar_error;
+ u64 hib_error;
+ int ret = 0;
+
+ hib_error = gasket_dev_read_64(
+ gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
+ scalar_error = gasket_dev_read_64(
+ gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
+
+ dev_dbg(gasket_dev->dev,
+ "%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
+ __func__, gasket_dev, hib_error, scalar_error);
+
+ if (allow_power_save)
+ ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
+
+ return ret;
+}
+
/* Determine if GCB is in reset state. */
static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
{
@@ -586,29 +400,69 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
return (val & SCU3_CUR_RST_GCB_BIT_MASK);
}

-/*
- * Check permissions for Apex ioctls.
- * Returns true if the current user may execute this ioctl, and false otherwise.
- */
-static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
+/* Reset the hardware, then quit reset. Called on device open. */
+static int apex_reset(struct gasket_dev *gasket_dev, uint type)
{
- return !!(filp->f_mode & FMODE_WRITE);
+ int ret;
+
+ if (bypass_top_level)
+ return 0;
+
+ if (!is_gcb_in_reset(gasket_dev)) {
+ /* We are not in reset - toggle the reset bit so as to force
+ * re-init of custom block
+ */
+ dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
+
+ ret = apex_enter_reset(gasket_dev, type);
+ if (ret)
+ return ret;
+ }
+ ret = apex_quit_reset(gasket_dev, type);
+
+ return ret;
}

-/* Apex-specific ioctl handler. */
-static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
+static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
{
- struct gasket_dev *gasket_dev = filp->private_data;
+ ulong page_table_ready, msix_table_ready;
+ int retries = 0;

- if (!apex_ioctl_check_permissions(filp, cmd))
- return -EPERM;
+ apex_reset(gasket_dev, 0);

- switch (cmd) {
- case APEX_IOCTL_GATE_CLOCK:
- return apex_clock_gating(gasket_dev, argp);
- default:
- return -ENOTTY; /* unknown command */
+ while (retries < APEX_RESET_RETRY) {
+ page_table_ready =
+ gasket_dev_read_64(
+ gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
+ msix_table_ready =
+ gasket_dev_read_64(
+ gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
+ if (page_table_ready && msix_table_ready)
+ break;
+ schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
+ retries++;
}
+
+ if (retries == APEX_RESET_RETRY) {
+ if (!page_table_ready)
+ dev_err(gasket_dev->dev, "Page table init timed out\n");
+ if (!msix_table_ready)
+ dev_err(gasket_dev->dev, "MSI-X table init timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/*
+ * Check permissions for Apex ioctls.
+ * Returns true if the current user may execute this ioctl, and false otherwise.
+ */
+static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
+{
+ return !!(filp->f_mode & FMODE_WRITE);
}

/* Gates or un-gates Apex clock. */
@@ -645,6 +499,22 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
return 0;
}

+/* Apex-specific ioctl handler. */
+static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
+{
+ struct gasket_dev *gasket_dev = filp->private_data;
+
+ if (!apex_ioctl_check_permissions(filp, cmd))
+ return -EPERM;
+
+ switch (cmd) {
+ case APEX_IOCTL_GATE_CLOCK:
+ return apex_clock_gating(gasket_dev, argp);
+ default:
+ return -ENOTTY; /* unknown command */
+ }
+}
+
/* Display driver sysfs entries. */
static ssize_t sysfs_show(
struct device *device, struct device_attribute *attr, char *buf)
@@ -696,9 +566,103 @@ static ssize_t sysfs_show(
return ret;
}

+static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {
+ GASKET_SYSFS_RO(node_0_page_table_entries, sysfs_show,
+ ATTR_KERNEL_HIB_PAGE_TABLE_SIZE),
+ GASKET_SYSFS_RO(node_0_simple_page_table_entries, sysfs_show,
+ ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE),
+ GASKET_SYSFS_RO(node_0_num_mapped_pages, sysfs_show,
+ ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES),
+ GASKET_END_OF_ATTR_ARRAY
+};
+
+static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
+{
+ return gasket_sysfs_create_entries(
+ gasket_dev->dev_info.device, apex_sysfs_attrs);
+}
+
+/* On device open, perform a core reinit reset. */
+static int apex_device_open_cb(struct gasket_dev *gasket_dev)
+{
+ return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
+}
+
+static const struct pci_device_id apex_pci_ids[] = {
+ { PCI_DEVICE(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID) }, { 0 }
+};
+
static void apex_pci_fixup_class(struct pci_dev *pdev)
{
pdev->class = (PCI_CLASS_SYSTEM_OTHER << 8) | pdev->class;
}
DECLARE_PCI_FIXUP_CLASS_HEADER(APEX_PCI_VENDOR_ID, APEX_PCI_DEVICE_ID,
PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class);
+
+static struct gasket_driver_desc apex_desc = {
+ .name = "apex",
+ .driver_version = APEX_DRIVER_VERSION,
+ .major = 120,
+ .minor = 0,
+ .module = THIS_MODULE,
+ .pci_id_table = apex_pci_ids,
+
+ .num_page_tables = NUM_NODES,
+ .page_table_bar_index = APEX_BAR_INDEX,
+ .page_table_configs = apex_page_table_configs,
+ .page_table_extended_bit = APEX_EXTENDED_SHIFT,
+
+ .bar_descriptions = {
+ GASKET_UNUSED_BAR,
+ GASKET_UNUSED_BAR,
+ { APEX_BAR_BYTES, (VM_WRITE | VM_READ), APEX_BAR_OFFSET,
+ NUM_REGIONS, mappable_regions, PCI_BAR },
+ GASKET_UNUSED_BAR,
+ GASKET_UNUSED_BAR,
+ GASKET_UNUSED_BAR,
+ },
+ .coherent_buffer_description = {
+ APEX_CH_MEM_BYTES,
+ (VM_WRITE | VM_READ),
+ APEX_CM_OFFSET,
+ },
+ .interrupt_type = PCI_MSIX,
+ .interrupt_bar_index = APEX_BAR_INDEX,
+ .num_interrupts = APEX_INTERRUPT_COUNT,
+ .interrupts = apex_interrupts,
+ .interrupt_pack_width = 7,
+
+ .add_dev_cb = apex_add_dev_cb,
+ .remove_dev_cb = NULL,
+
+ .enable_dev_cb = NULL,
+ .disable_dev_cb = NULL,
+
+ .sysfs_setup_cb = apex_sysfs_setup_cb,
+ .sysfs_cleanup_cb = NULL,
+
+ .device_open_cb = apex_device_open_cb,
+ .device_close_cb = apex_device_cleanup,
+
+ .ioctl_handler_cb = apex_ioctl,
+ .device_status_cb = apex_get_status,
+ .hardware_revision_cb = NULL,
+ .device_reset_cb = apex_reset,
+};
+
+static int __init apex_init(void)
+{
+ return gasket_register_device(&apex_desc);
+}
+
+static void apex_exit(void)
+{
+ gasket_unregister_device(&apex_desc);
+}
+MODULE_DESCRIPTION("Google Apex driver");
+MODULE_VERSION(APEX_DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("John Joseph <[email protected]>");
+MODULE_DEVICE_TABLE(pci, apex_pci_ids);
+module_init(apex_init);
+module_exit(apex_exit);
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:38:48

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 13/13] staging: gasket: apex: fix function param line continuation style

From: Todd Poynor <[email protected]>

Fix multi-line alignment formatting to look like:
int ret = long_function_name(device, VARIABLE1, VARIABLE2,
VARIABLE3, VARIABLE4);

Many of these TODO items were previously cleaned up during the conversion
to standard logging functions.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/apex_driver.c | 119 +++++++++++++--------------
1 file changed, 58 insertions(+), 61 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index f70fea0d80ecf..c0d3922e1d7c3 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -240,9 +240,9 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
* - Software force GCB idle
* - Enable GCB idle
*/
- gasket_read_modify_write_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER, 0x0, 1, 32);
+ gasket_read_modify_write_64(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER,
+ 0x0, 1, 32);

/* - Initiate DMA pause */
gasket_dev_write_64(gasket_dev, 1, APEX_BAR_INDEX,
@@ -259,16 +259,16 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
}

/* - Enable GCB reset (0x1 to rg_rst_gcb) */
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x1, 2, 2);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_2, 0x1, 2, 2);

/* - Enable GCB clock Gate (0x1 to rg_gated_gcb) */
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x1, 2, 18);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_2, 0x1, 2, 18);

/* - Enable GCB memory shut down (0x3 to rg_force_ram_sd) */
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x3, 2, 14);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3, 0x3, 2, 14);

/* - Wait for RAM shutdown. */
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
@@ -297,24 +297,24 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
* - b00: Not forced (HW controlled)
* - b1x: Force disable
*/
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x0, 2, 14);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3, 0x0, 2, 14);

/*
* - Disable software clock gate:
* - b00: Not forced (HW controlled)
* - b1x: Force disable
*/
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x0, 2, 18);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_2, 0x0, 2, 18);

/*
* - Disable GCB reset (rg_rst_gcb):
* - b00: Not forced (HW controlled)
* - b1x: Force disable = Force not Reset
*/
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_2, 0x2, 2, 2);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_2, 0x2, 2, 2);

/* - Wait for RAM enable. */
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
@@ -338,27 +338,28 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
}

if (!allow_hw_clock_gating) {
- val0 = gasket_dev_read_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+ val0 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3);
/* Inactive and Sleep mode are disabled. */
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 0x3,
- SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
- SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
- val1 = gasket_dev_read_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+ gasket_read_modify_write_32(gasket_dev,
+ APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3, 0x3,
+ SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
+ SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
+ val1 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3);
dev_dbg(gasket_dev->dev,
"Disallow HW clock gating 0x%x -> 0x%x\n", val0, val1);
} else {
- val0 = gasket_dev_read_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+ val0 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3);
/* Inactive mode enabled - Sleep mode disabled. */
- gasket_read_modify_write_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3, 2,
- SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
- SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
- val1 = gasket_dev_read_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+ gasket_read_modify_write_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3, 2,
+ SCU3_RG_PWR_STATE_OVR_MASK_WIDTH,
+ SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
+ val1 = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3);
dev_dbg(gasket_dev->dev, "Allow HW clock gating 0x%x -> 0x%x\n",
val0, val1);
}
@@ -373,12 +374,10 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
u64 hib_error;
int ret = 0;

- hib_error = gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
- scalar_error = gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
+ hib_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
+ scalar_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);

dev_dbg(gasket_dev->dev,
"%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
@@ -393,8 +392,8 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
/* Determine if GCB is in reset state. */
static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
{
- u32 val = gasket_dev_read_32(
- gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
+ u32 val = gasket_dev_read_32(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_SCU_3);

/* Masks rg_rst_gcb bit of SCU_CTRL_2 */
return (val & SCU3_CUR_RST_GCB_BIT_MASK);
@@ -432,13 +431,11 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev)

while (retries < APEX_RESET_RETRY) {
page_table_ready =
- gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
+ gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
msix_table_ready =
- gasket_dev_read_64(
- gasket_dev, APEX_BAR_INDEX,
- APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
+ gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
+ APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
if (page_table_ready && msix_table_ready)
break;
schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
@@ -481,20 +478,20 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,

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);
+ 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);
+ 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;
}
@@ -516,8 +513,8 @@ static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
}

/* Display driver sysfs entries. */
-static ssize_t sysfs_show(
- struct device *device, struct device_attribute *attr, char *buf)
+static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
+ char *buf)
{
int ret;
struct gasket_dev *gasket_dev;
@@ -578,8 +575,8 @@ static struct gasket_sysfs_attribute apex_sysfs_attrs[] = {

static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
{
- return gasket_sysfs_create_entries(
- gasket_dev->dev_info.device, apex_sysfs_attrs);
+ return gasket_sysfs_create_entries(gasket_dev->dev_info.device,
+ apex_sysfs_attrs);
}

/* On device open, perform a core reinit reset. */
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:38:51

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 11/13] staging: gasket: TODO: remove entry for static function kernel docs

From: Todd Poynor <[email protected]>

Remove the TODO entry for simplifying kernel doc style comments for
static functions, now that this has been addressed.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/TODO | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO
index fb71997fb5612..7f4c13ce021ba 100644
--- a/drivers/staging/gasket/TODO
+++ b/drivers/staging/gasket/TODO
@@ -5,7 +5,6 @@ staging directory.
- Use misc interface instead of major number for driver version description.
- Add descriptions of module_param's
- apex_get_status() should actually check status.
-- Static functions don't need kernel doc formatting, can be simplified.
- Fix multi-line alignment formatting to look like:
int ret = long_function_name(device, VARIABLE1, VARIABLE2,
VARIABLE3, VARIABLE4);
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:07

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 06/13] staging: gasket: core: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 151 ++++++---------------------
1 file changed, 31 insertions(+), 120 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 291cd6d074a2e..c00774059f9eb 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -199,11 +199,7 @@ MODULE_AUTHOR("Rob Springer <[email protected]>");
module_init(gasket_init);
module_exit(gasket_exit);

-/*
- * Perform a standard Gasket callback.
- * @gasket_dev: Device specific pointer to forward.
- * @cb_function: Standard callback to perform.
- */
+/* Perform a standard Gasket callback. */
static inline int check_and_invoke_callback(
struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *))
{
@@ -219,13 +215,7 @@ static inline int check_and_invoke_callback(
return ret;
}

-/*
- * Perform a standard Gasket callback
- * without grabbing gasket_dev->mutex.
- * @gasket_dev: Device specific pointer to forward.
- * @cb_function: Standard callback to perform.
- *
- */
+/* Perform a standard Gasket callback without grabbing gasket_dev->mutex. */
static inline int gasket_check_and_invoke_callback_nolock(
struct gasket_dev *gasket_dev, int (*cb_function)(struct gasket_dev *))
{
@@ -240,9 +230,8 @@ static inline int gasket_check_and_invoke_callback_nolock(
}

/*
- * Returns nonzero if the gasket_cdev_info is owned by the current thread group
+ * Return nonzero if the gasket_cdev_info is owned by the current thread group
* ID.
- * @info: Device node info.
*/
static int gasket_owned_by_current_tgid(struct gasket_cdev_info *info)
{
@@ -410,14 +399,9 @@ void gasket_unregister_device(const struct gasket_driver_desc *driver_desc)
}
EXPORT_SYMBOL(gasket_unregister_device);

-/**
- * Allocate a Gasket device.
- * @internal_desc: Pointer to the internal data for the device driver.
- * @pdev: Pointer to the Gasket device pointer, the allocated device.
- * @kobj_name: PCIe name for the device
- *
- * Description: Allocates and initializes a Gasket device structure.
- * Adds the device to the device list.
+/*
+ * Allocate and initialize a Gasket device structure, add the device to the
+ * device list.
*
* Returns 0 if successful, a negative error code otherwise.
*/
@@ -476,13 +460,7 @@ static int gasket_alloc_dev(
return 0;
}

-/*
- * Free a Gasket device.
- * @internal_dev: Gasket device pointer; the device to unregister and free.
- *
- * Description: Removes the device from the device list and frees
- * the Gasket device structure.
- */
+/* Free a Gasket device. */
static void gasket_free_dev(struct gasket_dev *gasket_dev)
{
struct gasket_internal_desc *internal_desc = gasket_dev->internal_desc;
@@ -496,7 +474,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
}

/*
- * Finds the next free gasket_internal_dev slot.
+ * Find the next free gasket_internal_dev slot.
*
* Returns the located slot number on success or a negative number on failure.
*/
@@ -533,10 +511,8 @@ static int gasket_find_dev_slot(
return i;
}

-/**
+/*
* PCI subsystem probe function.
- * @pci_dev: PCI device pointer to the new device.
- * @id: PCI device id structure pointer, the vendor and device ids.
*
* Called when a Gasket device is found. Allocates device metadata, maps device
* memory, and calls gasket_enable_dev to prepare the device for active use.
@@ -641,7 +617,6 @@ static int gasket_pci_probe(

/*
* PCI subsystem remove function.
- * @pci_dev: PCI device pointer; the device to remove.
*
* Called to remove a Gasket device. Finds the device in the device list and
* cleans up metadata.
@@ -694,8 +669,6 @@ static void gasket_pci_remove(struct pci_dev *pci_dev)

/*
* Setup PCI & set up memory mapping for the specified device.
- * @pci_dev: pointer to the particular PCI device.
- * @internal_dev: Corresponding Gasket device pointer.
*
* Enables the PCI device, reads the BAR registers and sets up pointers to the
* device's memory mapped IO space.
@@ -746,8 +719,6 @@ static void gasket_cleanup_pci(struct gasket_dev *gasket_dev)

/*
* Maps the specified bar into kernel space.
- * @internal_dev: Device possessing the BAR to map.
- * @bar_num: The BAR to map.
*
* Returns 0 on success, a negative error code otherwise.
* A zero-sized BAR will not be mapped, but is not an error.
@@ -824,7 +795,6 @@ static int gasket_map_pci_bar(struct gasket_dev *gasket_dev, int bar_num)

/*
* Releases PCI BAR mapping.
- * @internal_dev: Device possessing the BAR to unmap.
*
* A zero-sized or not-mapped BAR will not be unmapped, but is not an error.
*/
@@ -856,12 +826,7 @@ static void gasket_unmap_pci_bar(struct gasket_dev *dev, int bar_num)
release_mem_region(base, bytes);
}

-/*
- * Handle adding a char device and related info.
- * @dev_info: Pointer to the dev_info struct for this device.
- * @file_ops: The file operations for this device.
- * @owner: The owning module for this device.
- */
+/* Add a char device and related info. */
static int gasket_add_cdev(
struct gasket_cdev_info *dev_info,
const struct file_operations *file_ops, struct module *owner)
@@ -881,14 +846,7 @@ static int gasket_add_cdev(
return 0;
}

-/*
- * Performs final init and marks the device as active.
- * @internal_desc: Pointer to Gasket [internal] driver descriptor structure.
- * @internal_dev: Pointer to Gasket [internal] device structure.
- *
- * Currently forwards all work to device-specific callback; a future phase will
- * extract elements of character device registration here.
- */
+/* Perform final init and marks the device as active. */
static int gasket_enable_dev(
struct gasket_internal_desc *internal_desc,
struct gasket_dev *gasket_dev)
@@ -966,13 +924,7 @@ static int gasket_enable_dev(
return 0;
}

-/*
- * Disable device operations.
- * @gasket_dev: Pointer to Gasket device structure.
- *
- * Currently forwards all work to device-specific callback; a future phase will
- * extract elements of character device unregistration here.
- */
+/* Disable device operations. */
static void gasket_disable_dev(struct gasket_dev *gasket_dev)
{
const struct gasket_driver_desc *driver_desc =
@@ -997,7 +949,7 @@ static void gasket_disable_dev(struct gasket_dev *gasket_dev)
check_and_invoke_callback(gasket_dev, driver_desc->disable_dev_cb);
}

-/**
+/*
* Registered descriptor lookup.
*
* Precondition: Called with g_mutex held (to avoid a race on return).
@@ -1045,18 +997,15 @@ const char *gasket_num_name_lookup(
}
EXPORT_SYMBOL(gasket_num_name_lookup);

-/**
- * Opens the char device file.
- * @inode: Inode structure pointer of the device file.
- * @file: File structure pointer.
+/*
+ * Open the char device file.
*
- * Description: Called on an open of the device file. If the open is for
- * writing, and the device is not owned, this process becomes
- * the owner. If the open is for writing and the device is
- * already owned by some other process, it is an error. If
- * this process is the owner, increment the open count.
+ * If the open is for writing, and the device is not owned, this process becomes
+ * the owner. If the open is for writing and the device is already owned by
+ * some other process, it is an error. If this process is the owner, increment
+ * the open count.
*
- * Returns 0 if successful, a negative error number otherwise.
+ * Returns 0 if successful, a negative error number otherwise.
*/
static int gasket_open(struct inode *inode, struct file *filp)
{
@@ -1130,17 +1079,12 @@ static int gasket_open(struct inode *inode, struct file *filp)
return 0;
}

-/**
- * gasket_release - Close of the char device file.
- * @inode: Inode structure pointer of the device file.
- * @file: File structure pointer.
- *
- * Description: Called on a close of the device file. If this process
- * is the owner, decrement the open count. On last close
- * by the owner, free up buffers and eventfd contexts, and
- * release ownership.
+/*
+ * Called on a close of the device file. If this process is the owner,
+ * decrement the open count. On last close by the owner, free up buffers and
+ * eventfd contexts, and release ownership.
*
- * Returns 0 if successful, a negative error number otherwise.
+ * Returns 0 if successful, a negative error number otherwise.
*/
static int gasket_release(struct inode *inode, struct file *file)
{
@@ -1199,10 +1143,6 @@ static int gasket_release(struct inode *inode, struct file *file)
}

/*
- * Permission and validity checking for mmap ops.
- * @gasket_dev: Gasket device information structure.
- * @vma: Standard virtual memory area descriptor.
- *
* Verifies that the user has permissions to perform the requested mapping and
* that the provided descriptor/range is of adequate size to hold the range to
* be mapped.
@@ -1246,11 +1186,6 @@ static bool gasket_mmap_has_permissions(
}

/*
- * Checks if an address is within the region
- * allocated for coherent buffer.
- * @driver_desc: driver description.
- * @address: offset of address to check.
- *
* Verifies that the input address is within the region allocated to coherent
* buffer.
*/
@@ -1502,11 +1437,7 @@ static int gasket_mm_vma_bar_offset(
return 0;
}

-/*
- * Map a region of coherent memory.
- * @gasket_dev: Gasket device handle.
- * @vma: Virtual memory area descriptor with region to map.
- */
+/* Map a region of coherent memory. */
static int gasket_mmap_coherent(
struct gasket_dev *gasket_dev, struct vm_area_struct *vma)
{
@@ -1551,16 +1482,7 @@ static int gasket_mmap_coherent(
return 0;
}

-/*
- * Maps a device's BARs into user space.
- * @filp: File structure pointer describing this node usage session.
- * @vma: Standard virtual memory area descriptor.
- *
- * Maps the entirety of each of the device's BAR ranges into the user memory
- * range specified by vma.
- *
- * Returns 0 on success, a negative errno on error.
- */
+/* Map a device's BARs into user space. */
static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
{
int i, ret;
@@ -1704,14 +1626,7 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
return ret;
}

-/*
- * Determine the health of the Gasket device.
- * @gasket_dev: Gasket device structure.
- *
- * Checks the underlying device health (via the device_status_cb)
- * and the status of initialized Gasket code systems (currently
- * only interrupts), then returns a gasket_status appropriately.
- */
+/* Determine the health of the Gasket device. */
static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
{
int status;
@@ -1750,14 +1665,10 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)

/*
* Gasket ioctl dispatch function.
- * @filp: File structure pointer describing this node usage session.
- * @cmd: ioctl number to handle.
- * @arg: ioctl-specific data pointer.
*
- * First, checks if the ioctl is a generic ioctl. If not, it passes
- * the ioctl to the ioctl_handler_cb registered in the driver description.
- * If the ioctl is a generic ioctl, the function passes it to the
- * gasket_ioctl_handler in gasket_ioctl.c.
+ * Check if the ioctl is a generic ioctl. If not, pass the ioctl to the
+ * ioctl_handler_cb registered in the driver description.
+ * If the ioctl is a generic ioctl, pass it to gasket_ioctl_handler.
*/
static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
{
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:12

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 07/13] staging: gasket: ioctl: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_ioctl.c | 51 +++++----------------------
1 file changed, 9 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 78a132a60cc4f..55bdd7bfac866 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -170,14 +170,7 @@ long gasket_is_supported_ioctl(uint cmd)
}
}

-/*
- * Permission checker for Gasket ioctls.
- * @filp: File structure pointer describing this node usage session.
- * @cmd: ioctl number to handle.
- *
- * Check permissions for Gasket ioctls.
- * Returns true if the file opener may execute this ioctl, or false otherwise.
- */
+/* Check permissions for Gasket ioctls. */
static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
{
bool alive;
@@ -218,11 +211,7 @@ static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
return false; /* unknown permissions */
}

-/*
- * Associate an eventfd with an interrupt.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_interrupt_eventfd struct in userspace.
- */
+/* Associate an eventfd with an interrupt. */
static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
struct gasket_interrupt_eventfd __user *argp)
{
@@ -237,11 +226,7 @@ static int gasket_set_event_fd(struct gasket_dev *gasket_dev,
gasket_dev->interrupt_data, die.interrupt, die.event_fd);
}

-/*
- * Reads the size of the page table.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Read the size of the page table. */
static int gasket_read_page_table_size(
struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
@@ -268,11 +253,7 @@ static int gasket_read_page_table_size(
return ret;
}

-/*
- * Reads the size of the simple page table.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Read the size of the simple page table. */
static int gasket_read_simple_page_table_size(
struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
@@ -299,11 +280,7 @@ static int gasket_read_simple_page_table_size(
return ret;
}

-/*
- * Sets the boundary between the simple and extended page tables.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to gasket_page_table_ioctl struct in userspace.
- */
+/* Set the boundary between the simple and extended page tables. */
static int gasket_partition_page_table(
struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
@@ -340,11 +317,7 @@ static int gasket_partition_page_table(
return ret;
}

-/*
- * Maps a userspace buffer to a device virtual address.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
- */
+/* Map a userspace buffer to a device virtual address. */
static int gasket_map_buffers(struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
{
@@ -370,11 +343,7 @@ static int gasket_map_buffers(struct gasket_dev *gasket_dev,
ibuf.host_address, ibuf.device_address, ibuf.size / PAGE_SIZE);
}

-/*
- * Unmaps a userspace buffer from a device virtual address.
- * @gasket_dev: Pointer to the current gasket_dev we're using.
- * @argp: Pointer to a gasket_page_table_ioctl struct in userspace.
- */
+/* Unmap a userspace buffer from a device virtual address. */
static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
{
@@ -402,10 +371,8 @@ static int gasket_unmap_buffers(struct gasket_dev *gasket_dev,
}

/*
- * 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.
- * @argp: Pointer to a gasket_coherent_alloc_config_ioctl struct in userspace.
+ * Reserve structures for coherent allocation, and allocate or free the
+ * corresponding memory.
*/
static int gasket_config_coherent_allocator(
struct gasket_dev *gasket_dev,
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:34

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace

From: Todd Poynor <[email protected]>

Use user namespace to determine whether gasket device file opener is
root, allowing root access to containers, if necessary.

Reported-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index b832a4f529f27..291cd6d074a2e 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -13,13 +13,16 @@
#include "gasket_page_table.h"
#include "gasket_sysfs.h"

+#include <linux/capability.h>
#include <linux/compiler.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/of.h>
+#include <linux/pid_namespace.h>
#include <linux/printk.h>
+#include <linux/sched.h>

#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -1064,7 +1067,8 @@ 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);
+ struct pid_namespace *pid_ns = task_active_pid_ns(current);
+ int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);

gasket_dev = dev_info->gasket_dev_ptr;
driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1147,6 +1151,8 @@ static int gasket_release(struct inode *inode, struct file *file)
char task_name[TASK_COMM_LEN];
struct gasket_cdev_info *dev_info =
container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
+ struct pid_namespace *pid_ns = task_active_pid_ns(current);
+ int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);

gasket_dev = dev_info->gasket_dev_ptr;
driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1158,7 +1164,7 @@ static int gasket_release(struct inode *inode, struct file *file)
"Releasing device node. Call origin: tgid %u (%s) "
"(f_mode: 0%03o, fmode_write: %d, is_root: %u)\n",
current->tgid, task_name, file->f_mode,
- (file->f_mode & FMODE_WRITE), capable(CAP_SYS_ADMIN));
+ (file->f_mode & FMODE_WRITE), is_root);
dev_dbg(gasket_dev->dev, "Current open count (owning tgid %u): %d\n",
ownership->owner, ownership->write_open_count);

--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:37

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 03/13] staging: gasket: page table: hold references to device and pci_dev

From: Todd Poynor <[email protected]>

Hold references to the struct device and the pci_dev for the page table
while the data structures contian pointers to these.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_page_table.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index b9304d221722b..6b946a155ee3a 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -345,8 +345,8 @@ int gasket_page_table_init(
bar_data->virt_base[page_table_config->base_reg]);
pg_tbl->extended_offset_reg = (u64 __iomem *)&(
bar_data->virt_base[page_table_config->extended_reg]);
- pg_tbl->device = device;
- pg_tbl->pci_dev = pci_dev;
+ pg_tbl->device = get_device(device);
+ pg_tbl->pci_dev = pci_dev_get(pci_dev);

dev_dbg(device, "Page table initialized successfully\n");

@@ -364,6 +364,8 @@ void gasket_page_table_cleanup(struct gasket_page_table *pg_tbl)
vfree(pg_tbl->entries);
pg_tbl->entries = NULL;

+ put_device(pg_tbl->device);
+ pci_dev_put(pg_tbl->pci_dev);
kfree(pg_tbl);
}

--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:44

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 02/13] staging: gasket: sysfs: hold reference to device while in use

From: Todd Poynor <[email protected]>

Hold a reference to the struct device while a gasket sysfs mapping
exists for the device and a pointer to the struct is kept in the mapping
data structures.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/gasket_sysfs.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gasket/gasket_sysfs.c b/drivers/staging/gasket/gasket_sysfs.c
index da972ce0e0db0..fde04658419bc 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -126,6 +126,7 @@ static void put_mapping(struct gasket_sysfs_mapping *mapping)
kfree(mapping->attributes);
mapping->attributes = NULL;
mapping->attribute_count = 0;
+ put_device(mapping->device);
mapping->device = NULL;
mapping->gasket_dev = NULL;
}
@@ -208,22 +209,20 @@ int gasket_sysfs_create_mapping(
device->kobj.name);

mapping = &dev_mappings[map_idx];
- kref_init(&mapping->refcount);
- mapping->device = device;
- mapping->gasket_dev = gasket_dev;
mapping->attributes = kcalloc(GASKET_SYSFS_MAX_NODES,
sizeof(*mapping->attributes),
GFP_KERNEL);
- mapping->attribute_count = 0;
if (!mapping->attributes) {
dev_dbg(device, "Unable to allocate sysfs attribute array\n");
- mapping->device = NULL;
- mapping->gasket_dev = NULL;
mutex_unlock(&mapping->mutex);
mutex_unlock(&function_mutex);
return -ENOMEM;
}

+ kref_init(&mapping->refcount);
+ mapping->device = get_device(device);
+ mapping->gasket_dev = gasket_dev;
+ mapping->attribute_count = 0;
mutex_unlock(&mapping->mutex);
mutex_unlock(&function_mutex);

--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:39:56

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 05/13] staging: gasket: apex: simplify comments for static functions

From: Todd Poynor <[email protected]>

Static functions don't need kernel doc formatting, can be simplified.
Reformat comments that can be single-line. Remove extraneous text.

Signed-off-by: Todd Poynor <[email protected]>
---
drivers/staging/gasket/apex_driver.c | 64 +++++-----------------------
1 file changed, 10 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index ab466d49608a7..a756764751777 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -378,34 +378,20 @@ static int apex_sysfs_setup_cb(struct gasket_dev *gasket_dev)
gasket_dev->dev_info.device, apex_sysfs_attrs);
}

-/* On device open, we want to perform a core reinit reset. */
+/* On device open, perform a core reinit reset. */
static int apex_device_open_cb(struct gasket_dev *gasket_dev)
{
return gasket_reset_nolock(gasket_dev, APEX_CHIP_REINIT_RESET);
}

-/**
- * apex_get_status - Set device status.
- * @dev: Apex device struct.
- *
- * Description: Check the device status registers and set the driver status
- * to ALIVE or DEAD.
- *
- * Returns 0 if status is ALIVE, a negative error number otherwise.
- */
+/* Check the device status registers and return device status ALIVE or DEAD. */
static int apex_get_status(struct gasket_dev *gasket_dev)
{
/* TODO: Check device status. */
return GASKET_STATUS_ALIVE;
}

-/**
- * apex_device_cleanup - Clean up Apex HW after close.
- * @gasket_dev: Gasket device pointer.
- *
- * Description: Resets the Apex hardware. Called on final close via
- * device_close_cb.
- */
+/* Reset the Apex hardware. Called on final close via device_close_cb. */
static int apex_device_cleanup(struct gasket_dev *gasket_dev)
{
u64 scalar_error;
@@ -429,14 +415,7 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
return ret;
}

-/**
- * apex_reset - Quits reset.
- * @gasket_dev: Gasket device pointer.
- *
- * Description: Resets the hardware, then quits reset.
- * Called on device open.
- *
- */
+/* Reset the hardware, then quit reset. Called on device open. */
static int apex_reset(struct gasket_dev *gasket_dev, uint type)
{
int ret;
@@ -459,9 +438,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type)
return ret;
}

-/*
- * Enters GCB reset state.
- */
+/* Enter GCB reset state. */
static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
{
if (bypass_top_level)
@@ -516,9 +493,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type)
return 0;
}

-/*
- * Quits GCB reset state.
- */
+/* Quit GCB reset state. */
static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
{
u32 val0, val1;
@@ -601,9 +576,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
return 0;
}

-/*
- * Determines if GCB is in reset state.
- */
+/* Determine if GCB is in reset state. */
static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
{
u32 val = gasket_dev_read_32(
@@ -615,9 +588,6 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)

/*
* Check permissions for Apex ioctls.
- * @file: File pointer from ioctl.
- * @cmd: ioctl command.
- *
* Returns true if the current user may execute this ioctl, and false otherwise.
*/
static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
@@ -625,9 +595,7 @@ static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
return !!(filp->f_mode & FMODE_WRITE);
}

-/*
- * Apex-specific ioctl handler.
- */
+/* Apex-specific ioctl handler. */
static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
{
struct gasket_dev *gasket_dev = filp->private_data;
@@ -643,11 +611,7 @@ static long apex_ioctl(struct file *filp, uint cmd, void __user *argp)
}
}

-/*
- * Gates or un-gates Apex clock.
- * @gasket_dev: Gasket device pointer.
- * @argp: User ioctl arg, pointer to a apex_gate_clock_ioctl struct.
- */
+/* Gates or un-gates Apex clock. */
static long apex_clock_gating(struct gasket_dev *gasket_dev,
struct apex_gate_clock_ioctl __user *argp)
{
@@ -681,15 +645,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
return 0;
}

-/*
- * Display driver sysfs entries.
- * @device: Kernel device structure.
- * @attr: Attribute to display.
- * @buf: Buffer to which to write output.
- *
- * Description: Looks up the driver data and file-specific attribute data (the
- * type of the attribute), then fills "buf" accordingly.
- */
+/* Display driver sysfs entries. */
static ssize_t sysfs_show(
struct device *device, struct device_attribute *attr, char *buf)
{
--
2.18.0.345.g5c9ce644c3-goog


2018-07-29 19:40:12

by Todd Poynor

[permalink] [raw]
Subject: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used

From: Todd Poynor <[email protected]>

Hold a reference on the struct pci_dev while a pointer to it is held in
the gasket data structures.

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 2b484d067c38a..b832a4f529f27 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
internal_desc->devs[gasket_dev->dev_idx] = NULL;
mutex_unlock(&internal_desc->mutex);
put_device(gasket_dev->dev);
+ pci_dev_put(gasket_dev->pci_dev);
kfree(gasket_dev);
}

@@ -565,6 +566,7 @@ static int gasket_pci_probe(
ret = gasket_alloc_dev(internal_desc, parent, &gasket_dev, kobj_name);
if (ret)
return ret;
+ gasket_dev->pci_dev = pci_dev_get(pci_dev);
if (IS_ERR_OR_NULL(gasket_dev->dev_info.device)) {
pr_err("Cannot create %s device %s [ret = %ld]\n",
driver_desc->name, gasket_dev->dev_info.name,
@@ -572,7 +574,6 @@ static int gasket_pci_probe(
ret = -ENODEV;
goto fail1;
}
- gasket_dev->pci_dev = pci_dev;

ret = gasket_setup_pci(pci_dev, gasket_dev);
if (ret)
@@ -703,7 +704,6 @@ static int gasket_setup_pci(
{
int i, mapped_bars, ret;

- gasket_dev->pci_dev = pci_dev;
ret = pci_enable_device(pci_dev);
if (ret) {
dev_err(gasket_dev->dev, "cannot enable PCI device\n");
--
2.18.0.345.g5c9ce644c3-goog


2018-07-30 17:59:02

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace

Hi Todd,

On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <[email protected]> wrote:
> @@ -1064,7 +1067,8 @@ 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);
> + struct pid_namespace *pid_ns = task_active_pid_ns(current);
> + int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);

ns_capable() returns bool, why did you make is_root an integer?

Thanks,
Dmitry

2018-07-30 18:06:58

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH 04/13] staging: gasket: core: allow root access based on user namespace

Hi Dmitry,
On Mon, Jul 30, 2018 at 10:57 AM Dmitry Torokhov <[email protected]> wrote:
>
> Hi Todd,
>
> On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <[email protected]> wrote:
> > @@ -1064,7 +1067,8 @@ 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);
> > + struct pid_namespace *pid_ns = task_active_pid_ns(current);
> > + int is_root = ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN);
>
> ns_capable() returns bool, why did you make is_root an integer?

Gaah, I forgot to change the type of the existing var. Will fix, thanks -- Todd

>
> Thanks,
> Dmitry

2018-07-31 06:20:39

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used

On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <[email protected]> wrote:
>
> From: Todd Poynor <[email protected]>
>
> Hold a reference on the struct pci_dev while a pointer to it is held in
> the gasket data structures.
>
> 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 2b484d067c38a..b832a4f529f27 100644
> --- a/drivers/staging/gasket/gasket_core.c
> +++ b/drivers/staging/gasket/gasket_core.c
> @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
> internal_desc->devs[gasket_dev->dev_idx] = NULL;
> mutex_unlock(&internal_desc->mutex);
> put_device(gasket_dev->dev);
> + pci_dev_put(gasket_dev->pci_dev);

gasket_free_dev() is called only from driver PCI probe and remove
function. I can assure you that that pci_dev structure is not going
anywhere, there is no need to take this additional reference.

Thanks,
Dmitry

2018-07-31 06:31:10

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH 01/13] staging: gasket: core: hold reference to pci_dev while used

On Mon, Jul 30, 2018 at 11:19 PM Dmitry Torokhov <[email protected]> wrote:
>
> On Sun, Jul 29, 2018 at 12:37 PM Todd Poynor <[email protected]> wrote:
> >
> > From: Todd Poynor <[email protected]>
> >
> > Hold a reference on the struct pci_dev while a pointer to it is held in
> > the gasket data structures.
> >
> > 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 2b484d067c38a..b832a4f529f27 100644
> > --- a/drivers/staging/gasket/gasket_core.c
> > +++ b/drivers/staging/gasket/gasket_core.c
> > @@ -488,6 +488,7 @@ static void gasket_free_dev(struct gasket_dev *gasket_dev)
> > internal_desc->devs[gasket_dev->dev_idx] = NULL;
> > mutex_unlock(&internal_desc->mutex);
> > put_device(gasket_dev->dev);
> > + pci_dev_put(gasket_dev->pci_dev);
>
> gasket_free_dev() is called only from driver PCI probe and remove
> function. I can assure you that that pci_dev structure is not going
> anywhere, there is no need to take this additional reference.

WIll fix, thanks.

>
> Thanks,
> Dmitry