1. 24 Sep, 2024 2 commits
    • Damien Le Moal's avatar
      ata: libata-scsi: Fix ata_msense_control() CDL page reporting · 0e9a2990
      Damien Le Moal authored
      When the user requests the ALL_SUB_MPAGES mode sense page,
      ata_msense_control() adds the CDL_T2A_SUB_MPAGE twice instead of adding
      the CDL_T2A_SUB_MPAGE and CDL_T2B_SUB_MPAGE pages information. Correct
      the second call to ata_msense_control_spgt2() to report the
      CDL_T2B_SUB_MPAGE page.
      
      Fixes: 673b2fe6 ("scsi: ata: libata-scsi: Add support for CDL pages mode sense")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      0e9a2990
    • Damien Le Moal's avatar
      ata: libata-scsi: Fix ata_msense_control_spgt2() · 03a9cfc1
      Damien Le Moal authored
      ata_msense_control_spgt2() can be called even for devices that do not
      support CDL when the user requests the ALL_SUB_MPAGES mode sense page,
      but for such device, this will cause a NULL pointer dereference as
      dev->cdl is NULL. Similarly, we should not return any data if
      ata_msense_control_spgt2() is called when the user requested the
      CDL_T2A_SUB_MPAGE or CDL_T2B_SUB_MPAGE pages for a device that does not
      support CDL.
      
      Avoid this potential NULL pointer dereference by checking if the device
      support CDL on entry to ata_msense_control_spgt2() and return 0 if it
      does not support CDL.
      
      Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
      Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
      Fixes: 602bcf21 ("ata: libata: Improve CDL resource management")
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      03a9cfc1
  2. 10 Sep, 2024 1 commit
    • Niklas Cassel's avatar
      ata: libata: Clear DID_TIME_OUT for ATA PT commands with sense data · e5dd410a
      Niklas Cassel authored
      When ata_qc_complete() schedules a command for EH using
      ata_qc_schedule_eh(), blk_abort_request() will be called, which leads to
      req->q->mq_ops->timeout() / scsi_timeout() being called.
      
      scsi_timeout(), if the LLDD has no abort handler (libata has no abort
      handler), will set host byte to DID_TIME_OUT, and then call
      scsi_eh_scmd_add() to add the command to EH.
      
      Thus, when commands first enter libata's EH strategy_handler, all the
      commands that have been added to EH will have DID_TIME_OUT set.
      
      libata has its own flag (AC_ERR_TIMEOUT), that it sets for commands that
      have not received a completion at the time of entering EH.
      
      Thus, libata doesn't really care about DID_TIME_OUT at all, and currently
      clears the host byte at the end of EH, in ata_scsi_qc_complete(), before
      scsi_eh_finish_cmd() is called.
      
      However, this clearing in ata_scsi_qc_complete() is currently only done
      for commands that are not ATA passthrough commands.
      
      Since the host byte is visible in the completion that we return to user
      space for ATA passthrough commands, for ATA passthrough commands that got
      completed via EH (commands with sense data), the user will incorrectly see:
      ATA pass-through(16): transport error: Host_status=0x03 [DID_TIME_OUT]
      
      Fix this by moving the clearing of the host byte (which is currently only
      done for commands that are not ATA passthrough commands) from
      ata_scsi_qc_complete() to the start of EH (regardless if the command is
      ATA passthrough or not).
      
      While at it, use the proper helper function to clear the host byte, rather
      than open coding the clearing.
      
      This will make sure that we:
      -Correctly clear DID_TIME_OUT for both ATA passthrough commands and
       commands that are not ATA passthrough commands.
      -Do not needlessly clear the host byte for commands that did not go via EH.
       ata_scsi_qc_complete() is called both for commands that are completed
       normally (without going via EH), and for commands that went via EH,
       however, only commands that went via EH will have DID_TIME_OUT set.
      
      Fixes: 24aeebbf ("scsi: ata: libata: Change ata_eh_request_sense() to not set CHECK_CONDITION")
      Reported-by: default avatarIgor Pylypiv <ipylypiv@google.com>
      Closes: https://lore.kernel.org/linux-ide/ZttIN8He8TOZ7Lct@google.com/Signed-off-by: default avatarNiklas Cassel <cassel@kernel.org>
      Tested-by: default avatarIgor Pylypiv <ipylypiv@google.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      e5dd410a
  3. 07 Sep, 2024 10 commits
    • Damien Le Moal's avatar
      ata: libata: Fix W=1 compilation warning · d647bdf4
      Damien Le Moal authored
      Commit c494708d ("ata: libata: Cleanup libata-transport")
      inadvertently changed the name of the link argument to ata_link in the
      kdoc description of ata_tlink_add(), causing warnings to be issue when
      compiling with W=1:
      
      drivers/ata/libata-transport.c:690: warning: Function parameter or
      struct member 'link' not described in 'ata_tlink_add'
      drivers/ata/libata-transport.c:690: warning: Excess function parameter
      'ata_link' description in 'ata_tlink_add'
      
      Change the kdoc argument name to "link" to avoid these warnings.
      
      Fixes: c494708d ("ata: libata: Cleanup libata-transport")
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      d647bdf4
    • Damien Le Moal's avatar
      ata: libata: Improve CDL resource management · 602bcf21
      Damien Le Moal authored
      The ncq_sense_buf buffer field of struct ata_port is allocated and used
      only for devices that support the Command Duration Limits (CDL) feature.
      However, the cdl buffer of struct ata_device, which is used to cache the
      command duration limits log page for devices supporting CDL is always
      allocated as part of struct ata_device, which is wasteful of memory for
      devices that do not support this feature.
      
      Clean this up by defining both buffers as part of the new ata_cdl
      structure and allocating this structure only for devices that support
      the CDL feature. This new structure is attached to struct ata_device
      using the cdl pointer.
      
      The functions ata_dev_init_cdl_resources() and
      ata_dev_cleanup_cdl_resources() are defined to manage this new structure
      allocation, initialization and freeing when a port is removed or a
      device disabled. ata_dev_init_cdl_resources() is called from
      ata_dev_config_cdl() only for devices that support CDL.
      ata_dev_cleanup_cdl_resources() is called from ata_dev_free_resources()
      to free the ata_cdl structure when a device is being disabled by EH.
      
      Note that the name of the former cdl log buffer of struct ata_device is
      changed to desc_log_buf to make it clearer that it is a buffer for the
      limit descriptors log page.
      
      This change reduces the size of struct ata_device, thus reducing memory
      usage for ATA devices that do not support the CDL feature.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      602bcf21
    • Damien Le Moal's avatar
      ata: libata: Introduce ata_dev_free_resources · 5f8319c4
      Damien Le Moal authored
      Introduce the function ata_dev_free_resources() to free the resources
      allocated to support a device features. For now, this function is
      reduced to calling zpodd_exit() for devices that have this feature
      enabled.
      
      ata_dev_free_resources() is called from ata_eh_dev_disable() as this
      function is always called for all devices attached to a port that is
      being detached and for devices that are being disabled due to being
      removed (detached) from the system or due to errors.
      
      With this change, the call to zpodd_exit() done in ata_port_detach()
      and ata_scsi_handle_link_detach() are removed as these functions
      remove all devices attached to the link or port using libata EH, thus
      resulting in ata_eh_dev_disable() being called and the zpodd_exit()
      function being executed.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      5f8319c4
    • Damien Le Moal's avatar
      ata: libata: Move sector_buf from struct ata_port to struct ata_device · da65bbdd
      Damien Le Moal authored
      The 512B buffer sector_buf field of struct ata_port is used for scanning
      devices as well as during error recovery with ata EH. This buffer is
      thus useless if a port does not have a device connected to it.
      And also given that commands using this buffer are issued to devices,
      and not to ports, move this buffer definition from struct ata_port to
      struct ata_device.
      
      This change slightly increases system memory usage for systems using a
      port-multiplier as in that case we do not need a per-device buffer for
      scanning devices (PMP does not allow parallel scanning) nor for EH (as
      when entering EH we are guaranteed that all commands to all devices
      connected to the PMP have completed or have been aborted). However,
      this change reduces memory usage on systems that have many ports with
      only few devices rives connected, which is a much more common use case
      than the PMP use case.
      Suggested-by: default avatarNiklas Cassel <cassel@kernel.org>
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      da65bbdd
    • Damien Le Moal's avatar
      ata: libata: Rename ata_eh_read_sense_success_ncq_log() · 10e80763
      Damien Le Moal authored
      The function ata_eh_read_sense_success_ncq_log() does more that just
      reading the sense data for successful NCQ commands log page as it also
      sets the sense data for all commands listed in the log page.
      
      Rename this function to ata_eh_get_ncq_success_sense() to better
      describe what the function does. Furthermore, since this function is
      only called from ata_eh_get_success_sense() in libata-eh.c, there is no
      need to export it and its declaration can be moved to
      drivers/ata/libata.h.
      
      To be consistent with this change, the function
      ata_eh_read_sense_success_non_ncq() is also renamed to
      ata_eh_get_non_ncq_success_sense().
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      10e80763
    • Damien Le Moal's avatar
      ata: libata: Move sata_std_hardreset() definition to libata-sata.c · 78f76b09
      Damien Le Moal authored
      Unlike ata_std_prereset() and ata_std_postreset(), the function
      sata_std_hardreset() applies only to SATA devices, as its name implies.
      So move its definition to libata-sata.c.
      
      Together with this, also move the definition of sata_port_ops to
      libata-sata.c, where it belongs.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      78f76b09
    • Damien Le Moal's avatar
      ata: libata: Move sata_down_spd_limit() to libata-sata.c · b642212d
      Damien Le Moal authored
      Move the definition of the function sata_down_spd_limit() to
      libata-sata.c where it belongs, together with sata_set_spd().
      The helper function ata_sstatus_online() is also changed to be an
      inline function defined in drivers/ata/libata.h.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      b642212d
    • Damien Le Moal's avatar
      ata: libata: Improve __ata_qc_complete() · 5bb52d92
      Damien Le Moal authored
      The function __ata_qc_complete() is always called with a qc that already
      has been dereferenced and so is guaranteed to be non-NULL (as otherwise
      the kernel would have crashed). So remove the warning for a NULL qc as
      it is useless.
      
      Furthermore, the qc passed to __ata_qc_complete() must always be marked
      as active with the ATA_QCFLAG_ACTIVE flag. If that is not the case, in
      addition to the existing warning, return early so that we do not attempt
      to complete an invalid qc.
      
      Finally, fix the comment related to clearing the qc active flag as that
      operation applies to all devices, not just ATAPI ones.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      5bb52d92
    • Damien Le Moal's avatar
      ata: libata-scsi: Improve ata_scsi_handle_link_detach() · a1695151
      Damien Le Moal authored
      A struct ata_device flags should always be set and cleared with the
      device port locked. Testing for a flag should thus also be done while
      holding the device port lock. In accordance to this principle, modify
      ata_scsi_handle_link_detach() to test and clear the ATA_DFLAG_DETACHED
      flag while holding the device port lock.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      a1695151
    • Damien Le Moal's avatar
      ata: libata: Cleanup libata-transport · c494708d
      Damien Le Moal authored
      Move the ATA link transport device related functions after the ATA
      transport device related functions to avoid the need for forward
      declaring ata_tdev_add() and ata_tdev_delete().
      
      And while at it, do the following:
      1) Change ata_is_ata_dev() and ata_is_link() to return a boolean
      2) Fix a pointer declaration style in ata_is_ata_dev()
      3) Improve the kdoc comments for ata_tdev_free(), ata_tdev_delete(),
         ata_tdev_add(), ata_tlink_delete() and ata_tlink_add()
      
      No functional changes are introduced by this cleanup.
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
      Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
      c494708d
  4. 01 Sep, 2024 3 commits
  5. 29 Aug, 2024 1 commit
    • Niklas Cassel's avatar
      ata: libata: Add helper ata_eh_decide_disposition() · 9526dec2
      Niklas Cassel authored
      Every time I see libata code calling scsi_check_sense(), I get confused
      why the code path that is working fine for SCSI code, is not sufficient
      for libata code.
      
      The reason is that SCSI usually gets the sense data as part of the
      completion, and will thus automatically call scsi_check_sense(), which
      will set the SCSI ML byte (if any).
      
      However, for libata queued commands, we always need to fetch the sense
      data via SCSI EH, and thus do not get the luxury of having
      scsi_check_sense() called automatically.
      
      Add a new helper, ata_eh_decide_disposition(), that has a ata_eh_ prefix
      to more clearly highlight that this is only needed for code called by EH,
      while also having a similar name to scsi_decide_disposition(), such that
      it is easier to compare the libata code with the equivalent SCSI code.
      
      Also add a big kdoc comment explaining why this helper is called/needed in
      the first place.
      Signed-off-by: default avatarNiklas Cassel <cassel@kernel.org>
      Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
      9526dec2
  6. 28 Aug, 2024 1 commit
  7. 26 Aug, 2024 1 commit
  8. 15 Aug, 2024 2 commits
  9. 12 Aug, 2024 5 commits
  10. 02 Aug, 2024 2 commits
  11. 29 Jul, 2024 12 commits