07 Jan, 2016

6 commits

  • This drop enables a future card with a device id of 0x0600 to be
    recognized by the cxlflash driver.

    As per the design, the Accelerator Function Unit (AFU) for this new IBM
    CXL Flash Adapter retains the same host interface as the previous
    generation. For the early prototypes of the new card, the driver with
    this change behaves exactly as the driver prior to this behaved with the
    earlier generation card. Therefore, no card specific programming has
    been added. These card specific changes can be staged in later if
    needed.

    Signed-off-by: Manoj N. Kumar
    Acked-by: Matthew R. Ochs
    Reviewed-by: Andrew Donnellan
    Signed-off-by: Martin K. Petersen

    Manoj Kumar
     
  • If an async error interrupt is generated, and the error requires the FC
    link to be reset, it cannot be performed in the interrupt context. So a
    work element is scheduled to complete the link reset in a process
    context. If either an EEH event or an escalation occurs in between when
    the interrupt is generated and the scheduled work is started, the MMIO
    space may no longer be available. This will cause an oops in the worker
    thread.

    [ 606.806583] NIP kthread_data+0x28/0x40
    [ 606.806633] LR wq_worker_sleeping+0x30/0x100
    [ 606.806694] Call Trace:
    [ 606.806721] 0x50 (unreliable)
    [ 606.806796] wq_worker_sleeping+0x30/0x100
    [ 606.806884] __schedule+0x69c/0x8a0
    [ 606.806959] schedule+0x44/0xc0
    [ 606.807034] do_exit+0x770/0xb90
    [ 606.807109] die+0x300/0x460
    [ 606.807185] bad_page_fault+0xd8/0x150
    [ 606.807259] handle_page_fault+0x2c/0x30
    [ 606.807338] wait_port_offline.constprop.12+0x60/0x130 [cxlflash]

    To prevent the problem space area from being unmapped, when there is
    pending work, a mapcount (using the kref mechanism) is held. The
    mapcount is released only when the work is completed. The last
    reference release is tied to the unmapping service.

    Signed-off-by: Manoj N. Kumar
    Acked-by: Matthew R. Ochs
    Reviewed-by: Uma Krishnan
    Signed-off-by: Martin K. Petersen

    Manoj Kumar
     
  • After a few iterations of resetting the card, either during EEH
    recovery, or a host_reset the following is seen in the logs. cxlflash
    0008:00: cxlflash_queuecommand: could not get a free command

    At every reset of the card, the commands that are outstanding are being
    leaked. No effort is being made to reap these commands. A few more
    resets later, the above error message floods the logs and the card is
    rendered totally unusable as no free commands are available.

    Iterated through the 'cmd' queue and printed out the 'free' counter and
    found that on each reset certain commands were in-use and stayed in-use
    through subsequent resets.

    To resolve this issue, when the card is reset, reap all the commands
    that are active/outstanding.

    Signed-off-by: Manoj N. Kumar
    Acked-by: Matthew R. Ochs
    Reviewed-by: Andrew Donnellan
    Signed-off-by: Martin K. Petersen

    Manoj Kumar
     
  • Having a date for the driver requires it to be updated quite
    often. Removing the date which is not necessary. Also made
    use of the existing symbol to print the driver name.

    Signed-off-by: Uma Krishnan
    Reviewed-by: Andrew Donnellan
    Acked-by: Matthew R. Ochs
    Signed-off-by: Martin K. Petersen

    Uma Krishnan
     
  • Applications which use virtual LUN's that are backed by a physical LUN
    over both adapter ports may experience an I/O failure in the event of a
    link loss (e.g. cable pull).

    Virtual LUNs may be accessed through one or both ports of the adapter.
    This access is encoded in the translation entries that comprise the
    virtual LUN and used by the AFU for load-balancing I/O and handling
    failover scenarios. In a link loss scenario, even though the AFU is able
    to maintain connectivity to the LUN, it is up to the application to
    retry the failed I/O. When applications are unaware of the virtual LUN's
    underlying topology, they are unable to make a sound decision of when to
    retry an I/O and therefore are forced to make their reaction to a failed
    I/O absolute. The result is either a failure to retry I/O or increased
    latency for scenarios where a retry is pointless.

    To remedy this scenario, provide feedback back to the application on
    virtual LUN creation as to which ports the LUN may be accessed. LUN's
    spanning both ports are candidates for a retry in a presence of an I/O
    failure.

    Signed-off-by: Matthew R. Ochs
    Acked-by: Manoj Kumar
    Reviewed-by: Uma Krishnan
    Signed-off-by: Martin K. Petersen

    Matthew R. Ochs
     
  • The original fix to escalate a 'login timed out' error to a LINK_RESET
    was only made for one of the two ports on the card. This fix resolves
    the same issue for the second port (port 1).

    Signed-off-by: Manoj N. Kumar
    Acked-by: Matthew R. Ochs
    Reviewed-by: Uma Krishnan
    Signed-off-by: Martin K. Petersen

    Manoj Kumar
     

11 Dec, 2015

2 commits


30 Oct, 2015

32 commits

  • Contexts may be skipped over for cleanup in situations where contention
    for the adapter's table-list mutex is experienced in the presence of a
    signal during the execution of the release handler.

    This can lead to two known issues:

    - A hang condition on remove as that path tries to wait for users to
    cleanup - something that will never complete should this scenario play
    out as the user has already cleaned up from their perspective.

    - An Oops in the unmap_mapping_range() call that is made as part of
    the user waiting mechanism that is invoked on remove when contexts
    are found to still exist.

    The root cause of this issue can be found in get_context() and how the
    table-list mutex is acquired. As this code path is shared by several
    different access points within the driver, a decision was made during
    the development cycle to acquire this mutex in this location using the
    interruptible version of the mutex locking service. In almost all of
    the use-cases and environmental scenarios this holds up, even when the
    mutex is contended. However, for critical system threads (such as the
    release handler), failing to acquire the mutex and bailing with the
    intention of the user being able to try again later is unacceptable.

    In such a scenario, the context _must_ be derived as it is on an
    irreversible path to being freed. Without being able to derive the
    context, the code mistakenly assumes that it has already been freed
    and proceeds to free up the underlying CXL context resources. From
    this point on, any usage of [the now stale] CXL context resources
    will result in undefined behavior. This is root cause of the Oops
    mentioned as the second known issue as the mapping passed to the
    unmap_mapping_range() service is owned by the CXL context.

    To fix this problem, acquisition of the table-list mutex within
    get_context() is simply changed to use the uninterruptible version
    of the mutex locking service. This is safe as the timing windows for
    holding this mutex are short and also protected against blocking.

    Signed-off-by: Matthew R. Ochs
    Acked-by: Manoj Kumar
    Reviewed-by: Andrew Donnellan
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • When running with lock instrumentation (e.g. lockdep), some of the
    instrumentation can become disabled at probe time for a cxlflash
    adapter. This is due to a missing lock registration for the tmf_slock.

    The fix is to call spin_lock_init() for the tmf_slock during probe.

    Signed-off-by: Matthew R. Ochs
    Acked-by: Manoj Kumar
    Reviewed-by: Andrew Donnellan
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The port selection mask of a LUN can be corrupted when the manage LUN
    ioctl (DK_CXLFLASH_MANAGE_LUN) is issued more than once for any device.

    This mask indicates to the AFU which port[s] can be used for a data
    transfer to/from a particular LUN. The mask is critical to ensuring the
    correct behavior when using the virtual LUN function of this adapter.
    When the mask is configured for both ports, an I/O may be sent to either
    port as the AFU assumes that each port has access to the same physical
    device (specified by LUN ID in the port LUN table).

    In a situation where the mask becomes incorrectly configured to reflect
    access to both ports when in fact there is only access through a single
    port, an I/O can be targeted to the wrong physical device. This can lead
    to data corruption among other ill effects (e.g. security leaks).

    The cause for this corruption is the assumption that the ioctl will only
    be called a second time for a LUN when it is being configured for access
    via a second port. A boolean 'newly_created' variable is used to
    differentiate between a LUN that was created (and subsequently configured
    for single port access) and one that is destined for access across both
    ports. While initially set to 'true', this sticky boolean is toggled to
    the 'false' state during a lookup on any next ioctl performed on a device
    with a matching WWN/WWID. The code fails to realize that the match could
    in fact be the same device calling in again. From here, an assumption is
    made that any LUN with 'newly_created' set to 'false' is configured for
    access over both ports and the port selection mask is set to reflect this.
    Any future attempts to use this LUN for hosting a virtual LUN will result
    in the port LUN table being incorrectly programmed.

    As a remedy, the 'newly_created' concept was removed entirely and replaced
    with code that always constructs the port selection mask based upon the
    SCSI channel of the LUN being accessed. The bits remain sticky, therefore
    allowing for a device to be accessed over both ports when that is in fact
    the correct physical configuration.

    Also included in this commit are a few minor related changes to enhance
    the fix and provide better debug information for port selection mask and
    port LUN table bugs in the future. These include renaming refresh_local()
    to lookup_local(), tracing the WWN/WWID as a big-endian entity, and
    tracing the port selection mask, SCSI channel, and LUN ID each time the
    port LUN table is programmed.

    Signed-off-by: Matthew R. Ochs
    Acked-by: Manoj Kumar
    Reviewed-by: Andrew Donnellan
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • A 'login timed out' asynchronous error interrupt is generated if no
    response is seen to a FLOGI within 2 seconds. If the time out error
    is not escalated to a LINK_RESET the port will not be available for
    use. This fix provides the required escalation.

    Signed-off-by: Manoj N. Kumar
    Acked-by: Matthew R. Ochs
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Manoj Kumar
     
  • When running with an unsupported AFU, the cxlflash driver fails
    the probe. When the driver is removed, the following Oops is
    encountered on a show_interrupts() thread:

    Call Trace:
    [c000001fba5a7a10] [0000000000000003] 0x3 (unreliable)
    [c000001fba5a7a60] [c00000000053dcf4] vsnprintf+0x204/0x4c0
    [c000001fba5a7ae0] [c00000000030045c] seq_vprintf+0x5c/0xd0
    [c000001fba5a7b20] [c00000000030051c] seq_printf+0x4c/0x60
    [c000001fba5a7b50] [c00000000013e140] show_interrupts+0x370/0x4f0
    [c000001fba5a7c10] [c0000000002ff898] seq_read+0xe8/0x530
    [c000001fba5a7ca0] [c00000000035d5c0] proc_reg_read+0xb0/0x110
    [c000001fba5a7cf0] [c0000000002ca74c] __vfs_read+0x6c/0x180
    [c000001fba5a7d90] [c0000000002cb464] vfs_read+0xa4/0x1c0
    [c000001fba5a7de0] [c0000000002cc51c] SyS_read+0x6c/0x110
    [c000001fba5a7e30] [c000000000009204] system_call+0x38/0xb4

    The Oops is due to not cleaning up correctly on the unsupported
    AFU error path, leaving various allocated and registered resources.
    In this case, interrupts are in a semi-allocated/registered state,
    which the show_interrupts() thread attempts to use.

    To fix, the cleanup logic in init_afu() is consolidated to error
    gates at the bottom of the function and the appropriate goto is
    added to each error path. As a mini side fix while refactoring
    in this routine, the else statement following the AFU version
    evaluation is eliminated as it is not needed.

    Signed-off-by: Matthew R. Ochs
    Acked-by: Manoj Kumar
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Ioctl threads that use scsi_execute() can run for an excessive amount
    of time due to the fact that they have lengthy timeouts and retry logic
    built in. Under normal operation this is not an issue. However, once EEH
    enters the picture, a long execution time coupled with the possibility
    that a timeout can trigger entry to the driver via registered reset
    callbacks becomes a liability.

    In particular, a deadlock can occur when an EEH event is encountered
    while in running in scsi_execute(). As part of the recovery, the EEH
    handler drains all currently running ioctls, waiting until they have
    completed before proceeding with a reset. As the scsi_execute()'s are
    situated on the ioctl path, the EEH handler will wait until they (and
    the remainder of the ioctl handler they're associated with) have
    completed. Normally this would not be much of an issue aside from the
    longer recovery period. Unfortunately, the scsi_execute() triggers a
    reset when it times out. The reset handler will see that the device is
    already being reset and wait until that reset completed. This creates
    a condition where the EEH handler becomes stuck, infinitely waiting for
    the ioctl thread to complete.

    To avoid this behavior, temporarily unmark the scsi_execute() threads
    as an ioctl thread by releasing the ioctl read semaphore. This allows
    the EEH handler to proceed with a recovery while the thread is still
    running. Once the scsi_execute() returns, the ioctl read semaphore is
    reacquired and the adapter state is rechecked in case it changed while
    inside of scsi_execute(). The state check will wait if the adapter is
    still being recovered or returns a failure if the recovery failed. In
    the event that the adapter reset failed, the failure is simply returned
    as the ioctl would be unable to continue.

    Reported-by: Brian King
    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The trace following the failure of alloc_mem() incorrectly identifies
    which function failed. This can lead to misdiagnosing a failure.

    Fix the string to correctly indicate that alloc_mem() failed.

    Reported-by: Brian King
    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The fops owned by the adapter can be corrupted in certain scenarios,
    opening a window where certain fops are temporarily NULLed before being
    reset to their proper value. This can potentially lead software to make
    incorrect decisions, leaving the user with the inability to function as
    intended.

    An example of this behavior can be observed when there are a number of
    users with a high rate of turn around (attach to LUN, perform an I/O,
    detach from LUN, repeat). Every so often a user is given a valid
    context and adapter file descriptor, but the file associated with the
    descriptor lacks the correct read permission bit (FMODE_CAN_READ) and
    thus the read system call bails before calling the valid read fop.

    Background:

    The fops is stored in the adapter structure to provide the ability to
    lookup the adapter structure from within the fop handler. CXL services
    use the file's private_data and at present, the CXL context does not
    have a private section. In an effort to limit areas of the cxlflash
    driver with code specific the superpipe function, a design choice was
    made to keep the details of the fops situated away from the legacy
    portions of the driver. This drove the behavior that the adapter fops
    is set at the beginning of the disk attach ioctl handler when there
    are no users present.

    The corruption that this fix remedies is due to the fact that the fops
    is initially defaulted to values found within a static structure. When
    the fops is handed down to the CXL services later in the attach path,
    certain services are patched. The fops structure remains correct until
    the user count drops to 0 and the fops is reset, triggering the process
    to repeat again. The user counts are tightly coupled with the creation
    and deletion of the user context. If multiple users perform a disk
    attach at the same time, when the user count is currently 0, some users
    can be in the middle of obtaining a file descriptor and have not yet
    reached the context creation code that [in addition to creating the
    context] increments the user count. Subsequent users coming in to
    perform the attach see that the user count is still 0, and reinitialize
    the fops, temporarily removing the patched fops. The users that are in
    the middle obtaining their file descriptor may then receive an invalid
    descriptor.

    The fix simply removes the user count altogether and moves the fops
    initialization to probe time such that it is only performed one time
    for the life of the adapter. In the future, if the CXL services adopt
    a private member for their context, that could be used to store the
    adapter structure reference and cxlflash could revert to a model that
    does not require an embedded fops.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The operator used to double the master context response delay
    is incorrect and does not result in delay doubling.

    To fix, use a left shift instead of the XOR operator.

    Reported-by: Tomas Henzl
    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Manoj Kumar
     
  • Following an adapter reset, the AFU RRQ that resides in host memory
    holds stale data. This can lead to a condition where the RRQ interrupt
    handler tries to process stale entries and/or endlessly loops due to an
    out of sync generation bit.

    To fix, the AFU RRQ in host memory needs to be cleared after each reset.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • There are several spelling and grammar mistakes throughout the
    driver. Additionally there are a handful of places where there
    are extra lines and unnecessary variables/statements. These are
    a nuisance and pollute the driver.

    Fix spelling and grammar issues. Update some comments for clarity and
    consistency. Remove extra lines and a few unneeded variables/statements.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The process_sense() routine can perform a read capacity which
    can take some time to complete. If an EEH occurs while waiting
    on the read capacity, the EEH handler will wait to obtain the
    context's mutex in order to put the context in an error state.
    The EEH handler will sit and wait until the context is free,
    but this wait can potentially last forever (deadlock) if the
    scsi_execute() that performs the read capacity experiences a
    timeout and calls into the reset callback. When that occurs,
    the reset callback sees that the device is already being reset
    and waits for the reset to complete. This leaves two threads
    waiting on the other.

    To address this issue, make the context unavailable to new,
    non-system owned threads and release the context while calling
    into process_sense(). After returning from process_sense() the
    context mutex is reacquired and the context is made available
    again. The context can be safely moved to the error state if
    needed during the unavailable window as no other threads will
    hold its reference.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Sparse uncovered several errors with MMIO operations (accessing
    directly) and handling endianness. These can cause issues when
    running in different environments.

    Introduce __iomem and proper endianness tags/swaps where
    appropriate to make driver sparse clean.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Several function prologs have incorrect parameter names and return
    code descriptions. This can lead to confusion when reviewing the
    source and creates inaccurate documentation.

    To remedy, update the function prologs to properly reflect parameter
    names and return codes.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The host reset handler is called with I/O already blocked, thus
    there is no need to explicitly block and unblock I/O in the handler.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • When the device reset handler is entered while a reset operation
    is taking place, the handler exits without actually sending a
    reset (TMF) to the targeted device. This behavior is incorrect
    as the device is not reset. Further complicating matters is the
    fact that a success is returned even when the TMF was not sent.

    To fix, the state is rechecked after coming out of the reset
    state. When the state is normal, a TMF will be sent out.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The workq can process work in parallel with a remove event, leading
    to a condition where the workq handler can access freed memory.

    To remedy, the workq should be terminated prior to freeing memory. Move
    the termination call earlier in remove and use cancel_work_sync() instead
    of flush_work() as there is not a need to process any scheduled work when
    shutting down.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Currently, scsi_host_put() is being called prematurely in the
    remove path and is missing entirely in an error cleanup path.
    The former can lead to memory being freed too early with
    subsequent access potentially corrupting data whilst the former
    would result in a memory leak.

    Move the usage on remove to be the last cleanup action taken
    and introduce a call to scsi_host_put() in the one initialization
    error path that does not use remove to cleanup.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The AFU version is stored as a non-terminated string of bytes within
    a 64-bit little-endian register. Presently the value is read directly
    (no MMIO accessor) and is stored in a buffer that is not big enough
    to contain a NULL terminator. Additionally the version obtained is not
    evaluated against a known value to prevent usage with unsupported AFUs.
    All of these deficiencies can lead to a variety of problems.

    To remedy, use the correct MMIO accessor to read the version value into
    a null-terminated buffer and add a check to prevent an incompatible AFU
    from being used with this driver.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • At present, both ports must be online for the device to
    configure properly. Remove this dependency and the unnecessary
    internal LUN override logic as well. Additionally, as a refactoring
    measure, change the return code variable name to match that used
    throughout the driver.

    With this change, the card will be able to configure even when the
    link is down. At some later point when the link is transitioned to
    'up', a link state change interrupt will trigger the port configuration.
    Note that despite its void-like behavior, the function was left with a
    return code for right now in case its behavior needs to be altered again
    in the near future based on testing.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • A bug was introduced earlier in the development cycle when cleaning
    up logic statements. Instead of skipping bits that are not set, set
    bits are skipped, causing async interrupts to not be handled correctly.

    To fix, simply add back in the proper evaluation for an unset bit.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Following a link up event, the LUNs available to the host may
    have changed. Without rescanning the host, the LUN topology is
    unknown to the user. In such a state, the user would be unable
    to locate provisioned resources.

    To remedy, the host should be rescanned after a link up event.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The resid is incorrectly set which can lead to unnecessary retry
    attempts by the stack. This is due to resid _always_ being set
    using a value returned from the adapter. Instead, the value
    should only be interpreted and set when in an underrun scenario.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Borrowing the TMF waitq's spinlock causes a stall condition when
    waiting for the TMF to complete. To remedy, introduce our own spin
    lock to serialize TMF and use the appropriate wait services.

    Also add a timeout while waiting for a TMF completion. When a TMF
    times out, report back a failure such that a bigger hammer reset
    can occur.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • During run-time the driver can be very chatty and spam the system
    kernel log. Various print statements can be limited and/or moved
    to development-only mode. Additionally, numerous prints can be
    converted to trace the corresponding device. Lastly, one spelling
    correction was made: 'entra' to 'extra'.

    The following changes were made:
    - pr_debug to pr_devel
    - pr_debug to pr_debug_ratelimited
    - pr_err to dev_err
    - pr_debug to dev_dbg

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Andrew Donnellan
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Implement the following suggestions and add two new attributes
    to allow for debugging the port LUN table.

    - use scnprintf() instead of snprintf()
    - use DEVICE_ATTR_RO and DEVICE_ATTR_RW

    Suggested-by: Shane Seymour
    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Found during code inspection, that the following functions are not
    being used outside of the file where they are defined. Make them static.

    int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
    void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
    int cxlflash_afu_reset(struct cxlflash_cfg *);
    struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
    void cxlflash_cmd_checkin(struct afu_cmd *);
    void init_pcr(struct cxlflash_cfg *);
    int init_global(struct cxlflash_cfg *);

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Limbo is not an accurate representation of this state and is
    also not consistent with the terminology that other drivers
    use to represent this concept. Rename the state and and its
    associated waitq to 'reset'.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • During an EEH freeze event, certain CXL services should not be
    called until after the hardware reset has taken place. Doing so
    can result in unnecessary failures and possibly cause other ill
    effects by triggering hardware accesses. This translates to a
    requirement to quiesce all threads that may potentially use CXL
    runtime service during this window. In particular, multiple ioctls
    make use of the CXL services when acting on contexts on behalf of
    the user. Thus, it is essential to 'drain' running ioctls _before_
    proceeding with handling the EEH freeze event.

    Create the ability to drain ioctls by wrapping the ioctl handler
    call in a read semaphore and then implementing a small routine that
    obtains the write semaphore, effectively creating a wait point for
    all currently executing ioctls.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • The context encode mask covers more than 32-bits, making it
    a long integer. This should be noted by appending the ULL
    width suffix to the mask.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • Using sizeof(bool) is considered poor form for various reasons and
    sparse warns us of that. Correct by changing type from bool to u8.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Daniel Axtens
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs
     
  • If the same virtual LUN is accessed over multiple cards, only accesses
    made over the first card will be valid. Accesses made over the second
    card will go to the wrong LUN causing data corruption.

    This is because the global LUN's mode word was being used to determine
    whether the LUN table for that card needs to be programmed. The mode
    word would be setup by the first card, causing the LUN table for the
    second card to not be programmed.

    By unconditionally initializing the LUN table (not depending on the
    mode word), the problem is avoided.

    Signed-off-by: Matthew R. Ochs
    Signed-off-by: Manoj N. Kumar
    Reviewed-by: Brian King
    Reviewed-by: Tomas Henzl
    Signed-off-by: James Bottomley

    Matthew R. Ochs