Garrett D'Amore wrote:
> Alan Perry wrote:
>> I am sponsoring this self-review case.
>>
>> This case documents additional changes to an existing case. I believe
>> that
>> this case qualifies for self-review because the interfaces are
>> Consolidation
>> Private and backwards compatible with the existing interfaces.
>>
>
> This is not the only criteria for self-review. Self-review cases must
> also be so obvious and self explanatory that no further review is
> desired or required. They should usually should not be introducing new
> architecture.
>
> I believe this case exceed this threshold, and I would like to make sure
> it is properly reviewed. Please convert this to a fast track with a one
> week timer.
This case does not introduce new architecture. It is making changes to
an architecture introduced in PSARC/2004/779.
As far as the obvious criteria, obvious to whom? I find the interface
changes fairly obvious. I did have to read about how SATA port
multipliers work first and think about ways to implement support for
them across the sata framework/sata HBA driver split.
alan
>
> -- Garrett
>> Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI
>> This information is Copyright 2009 Sun Microsystems
>> 1. Introduction
>> 1.1. Project/Component Working Name:
>> SATA Framework Port Multiplier Support
>> 1.2. Name of Document Author/Supplier:
>> Author: Xiaoyu Zhang
>> 1.3 Date of This Document:
>> 14 July, 2009
>> 4. Technical Description
>>
>> 4.1. Background
>> ---------------
>> PSARC/2004/779 [1] established the SATA HBA Framework interface.
>> PSARC/2005/679 [2] expanded this interface as needed to suport the
>> marvell88sx
>> and si3124 SATA HBA drivers. PSARC 2007/274 [3] expanded the interface
>> further
>> to support Native Command Queuing (NCQ) and ATAPI devices.
>> PSARC/2007/448 [4]
>> expanded the interface again to support releasing DMA framework-allocated
>> resources associated with a command's buffer.
>>
>> This fast-track describes further interface changes required to
>> support SATA
>> port multipliers.
>>
>> 4.2. The Problem
>> ----------------
>>
>> 4.2.1. Required to support READ/WRITE PORTMULT command
>> ------------------------------------------------------
>> According to the SATA Specification 2.6 [5] and the Port Multiplier
>> Specification [6], READ PORT MULTIPLIER and WRITE PORT MULIPLIER are
>> used to
>> access the registers of the port multiplier. These two commands are
>> necessary
>> to the successful enumeration of the the port multiplier.
>>
>> Both SATA HBA drivers as well as the SATA module have internal functions
>> operating on sata_pkt rather than isolated SATA commands. Therefore,
>> READ/WRITE PORTMULT commands should be delivered to SATA HBA drivers in
>> sata_pkt structures.
>>
>> 4.2.2. The insufficient sata_device interface
>> ---------------------------------------------
>> The sata_device is used as a parameter to the HBA probe entry point.
>> Existing
>> HBA implementations update only SATA Control Registers values in
>> response to
>> probe port operation. The port multiplier has its own Global Status &
>> Control
>> Registers, in which the parameters and status of the port multiplier
>> itself
>> are stored. The sata module needs this register information, so there
>> needs
>> to be a method for the SATA HBA driver to provide this information.
>>
>> 4.2.3. Required to handle port multiplier quirks
>> ------------------------------------------------
>> The SATA specification 2.6 [5] defines the registers and enumeration
>> process
>> for port multipliers, however, some existing port multiplier models
>> have quirks
>> and might break the general enumeration or initialization process. The
>> HBA
>> driver should have idea of these quirks and determine its reaction.
>> Additions
>> to the interface are required to handle Port Multiplier behavior that is
>> different the specification.
>>
>>
>> 4.3. The Proposal
>> -----------------
>>
>> 4.3.1. Summary
>> --------------
>> Revise the SATA interface to support port multiplier. Port multiplier
>> support
>> was partially designed and implemented inside SATA module. The proposal
>> defines new interface functions necessary to support new SATA commands
>> and new
>> fields in sata_device structure to support port multiplier device. A
>> blacklist
>> structure is defined is added to to dealing with port multiplier
>> discrepancies
>> from the SATA specification.
>>
>> The initial consumer of the modified interface will be AHCI driver
>> implementing SATA port multiplier support.
>>
>> Since the existing SATA HBA drivers and SATA module do not currently
>> implement
>> the structure version checking (except for sata_hba_tran structure), the
>> proposed change will maintain the binary compatibility of the modified
>> interface structure (sata_device).
>>
>> A SATA module supporting these interface changes will operate with
>> SATA HBA
>> drivers using the unmodified interface as well as SATA HBA drivers
>> using the
>> modified interface.
>>
>> 4.3.2. Interface Modifications
>> ------------------------------
>> a) Add new structure sata_gscr.
>> (See section 5.2.1 for more details)
>>
>> b) Modified sata_device structure. Add new field satadev_pmult_gscr
>> for port
>> multiplier device.
>> (See section 5.3.1 for more details)
>>
>> c) The sata_device structure version (SATA_DEVICE_REV) will be
>> increased to
>> indicate added functionality. Current veriosn level is 1 - it
>> will be increased to 2.
>> (See section 5.3.1 for more details).
>>
>> d) The sata_hba_tran structure version (SATA_TRAN_HBA_REV) will be
>> increased
>> to 3 to indicate new functionality level of the entire SATA framework
>> interface.
>> (See section 5.3.2 for more details)
>>
>> e) Add three new SATA module interface functions.
>> + sata_get_rdwr_pmult_pkt();
>> + sata_free_rdwr_pmult_pkt();
>> + sata_check_pmult_blacklist();
>> (See section 5.4 for more details)
>>
>> 4.5. Stability level
>> --------------------
>> The stability level of the new interfaces will be the same as the
>> other interfaces between SATA HBA Framework (SATA module) and
>> SATA HBA driver, i.e. Consolidation Private.
>> The requested release binding is micro release and patch release.
>>
>>
>> 5. Interface Table
>> ==================
>>
>> 5.1. Exported Interfaces
>> ------------------------
>>
>> ------------------------------------------------------------------------
>> Interface Level Comments
>> ------------------------------------------------------------------------
>> SATA_DEVICE_REV Consolidation Symbol
>> Private (redefine)
>> SATA_DEVICE_REV_2 Consolidation sata_device version
>> Private (new)
>> SATA_TRAN_HBA_REV Consolidation Symbol
>> Private (redefine)
>> SATA_TRAN_HBA_REV_3 Consolidation sata_hba_tran version
>> Private (new)
>> sata_pmult_gscr Consolidation Interface structure
>> Private (new)
>> sata_device Consolidation Interface structure
>> Private (modified)
>> satadev_gscr Consolidation Interface structure
>> Private (new)
>> sata_get_rdwr_pmult_pkt Consolidation Interface function
>> Private (new)
>> sata_free_rdwr_pmult_pkt Consolidation Interface function
>> Private (new)
>> sata_check_pmult_blacklist Consolidation Interface function
>> Private (new)
>>
>> 5.2. New structures
>> -------------------
>>
>> 5.2.1. New structure: sata_pmult_gscr
>> -------------------------------------
>> struct sata_pmult_gscr {
>> uint32_t gscr0; /* Product Identifier register */
>> uint32_t gscr1; /* Resrved Information register */
>> uint32_t gscr2; /* Port Information register */
>> uint32_t gscr64; /* Feature register */
>> };
>>
>>
>> 5.3. Redefined symbols
>> ----------------------
>>
>> 5.3.1. Redefine: SATA_DEVICE_REV & sata_device
>> ----------------------------------------------
>> Modified field is indicated by a change bar.
>>
>> Old definition:
>> #define SATA_DEVICE_REV_1 1
>> |#define SATA_DEVICE_REV SATA_DEVICE_REV_1
>>
>> struct sata_device
>> {
>> int satadev_rev; /* structure version */
>> struct sata_address satadev_addr; /* sata port/device address */
>> uint32_t satadev_state; /* Port or device state */
>> uint32_t satadev_type; /* Attached device type */
>> struct sata_port_scr satadev_scr; /* Port status and ctrl regs */
>> uint32_t satadev_add_info; /* additional information, */
>> /* function specific */
>> };
>>
>> New definition:
>> #define SATA_DEVICE_REV_1 1
>> |#define SATA_DEVICE_REV_2 2
>> |#define SATA_DEVICE_REV SATA_DEVICE_REV_1
>>
>> struct sata_device
>> {
>> int satadev_rev; /* structure version */
>> struct sata_address satadev_addr; /* sata port/device address */
>> uint32_t satadev_state; /* Port or device state */
>> uint32_t satadev_type; /* Attached device type */
>> struct sata_port_scr satadev_scr; /* Port status and ctrl regs */
>> uint32_t satadev_add_info; /* additional information, */
>> /* function specific */
>> | struct sata_pmult_gscr satadev_gscr; /* Port multiplier specific
>> | global status and control
>> | registers */
>> };
>>
>> Implementation Notes:
>>
>> The satadev_gscr block is added for port multiplier's global status and
>> control registers.
>>
>>
>> 5.3.2. SATA_TRAN_HBA_REV redefinition
>> -------------------------------------
>> Old definition:
>> #define SATA_TRAN_HBA_REV SATA_TRAN_HBA_REV_2
>>
>> New definitions:
>> #define SATA_TRAN_HBA_REV_3 3
>> #define SATA_TRAN_HBA_REV SATA_TRAN_HBA_REV_3
>>
>> Only version level of the sata_hba_tran structure is modified to indicate
>> new functionality level of the entire SATA framework interface.
>> New functionality includes:
>> a) SATA module functions to get and free READ/WRITE PORTMULT sata
>> packets.
>> b) SATA port multiplier blacklist in SATA module
>> c) Enable sata_device structure to support port multiplier device.
>> New interface version (SATA_TRAN_HBA_REV_3) level also implies version
>> 2 of
>> the sata_device structure definition.
>>
>>
>> 5.4. New Interface Functions
>> ----------------------------
>>
>> 5.4.1. sata_get_rdwr_pmult_pkt
>> ------------------------------
>> #define SATA_RDWR_PMULT_PKT_TYPE_READ 1
>> #define SATA_RDWR_PMULT_PKT_TYPE_WRITE 2
>>
>> NAME
>>
>> sata_get_rdwr_pmult_pkt - get sata packet to execute READ/WRITE
>> PORTMULT
>> command
>>
>> SYNOPSIS
>>
>> #include <sys/sata/impl/sata_hba.h>
>>
>> sata_pkt_t *sata_get_rdwr_pmult_pkt(dev_info_t *dip,
>> sata_device_t *sata_device, uint8_t regn,
>> uint32_t regv, uint32_t type);
>>
>> INTERFACE LEVEL
>>
>> Consolidation Private
>>
>> PARAMETERS
>>
>> dip
>> Pointer to a dev_info_t structure, referring to the HBA device
>> instance.
>>
>> sata_device
>> Pointer to the structure specifying the SATA device address.
>>
>> regn
>> Register that is supposed to be read/write.
>>
>> regv
>> The value of the target register. This parameter is only used
>> when the type parameter is set to SATA_RDWR_PMULT_PKT_TYPE_WRITE.
>>
>> type
>> SATA_RDWR_PMULT_PKT_TYPE_READ Returned sata_pkt structure should
>> contain
>> READ PORTMULT command.
>>
>> SATA_RDWR_PMULT_PKT_TYPE_WRITE Returned sata_pkt structure should
>> contain
>> WRITE PORTMULT command.
>>
>> DESCRIPTION
>>
>> The sata_get_rdwr_pmult_pkt function is called by SATA HBA driver
>> to obtain
>> a fully initialized sata_pkt containing a READ/WRITE PORTMULT
>> command, as
>> well as a DMA-capable data buffer and DMA resources for the data
>> buffer.
>> The data buffer will satisfy HBA DMA attributes restrictions. The
>> same
>> data buffer could be also used for programmed I/O.
>>
>> The target register is specified by the regn argument. The command
>> type is
>> specified by type argument.
>>
>> The initialized sata packet does not specify any completion callback
>> routine. No packet completion reason nor packet status is to be
>> returned
>> to SATA module. Once the SATA HBA completes sata_pkt usage, it
>> should call
>> sata_free_rdwr_pmult_pkt() function to free the packet and allocated
>> resources.
>>
>> RETURN VALUES
>> Returns a pointer to initialized sata_pkt if the function succeeds,
>> and returns Null, if packet could not be allocated and/or
>> initialized.
>>
>> CONTEXT
>> This function cannot be called from the interrupt context.
>>
>>
>> 5.4.2. sata_free_rdwr_pmult_pkt
>> -------------------------------
>>
>> NAME
>> sata_free_rdwr_pmult_pkt - free sata packet allocated
>> for READ/WRITE PORTMULT command
>>
>> SYNOPSIS
>> #include <sys/sata/impl/sata_hba.h>
>>
>> void sata_free_rdwr_pmult_pkt(sata_pkt_t *sata_pkt);
>>
>> INTERFACE LEVEL
>>
>> Consolidation Private
>>
>> PARAMETERS
>> sata_pkt sata_pkt allocated previoulsy by the
>> sata_get_rdwr_pmult_pkt().
>>
>> DESCRIPTION
>> sata_free_rdwr_pmult_pkt function is called by the SATA HBA driver in
>> order to release the sata_pkt structure allocated previously by
>> sata_get_rdwr_pmult_pkt(). All resources associated with the
>> packet are
>> freed. After calling this function, the SATA HBA driver should not
>> attempt
>> to access any field and/or data buffer associated with the freed
>> sata_pkt.
>>
>> RETURN VALUES
>> Void
>>
>> CONTEXT
>> This function may be called from the interrupt context.
>>
>> 5.4.3. sata_check_pmult_blacklist
>> ---------------------------------
>>
>> NAME
>> sata_check_pmult_blacklist - check if a port multiplier is on the
>> blacklist
>>
>> SYNOPSIS
>> #include <sys/sata/impl/sata_hba.h>
>>
>> int sata_check_pmult_blacklist(sata_device_t *sata_device);
>>
>> INTERFACE LEVEL
>>
>> Consolidation Private
>>
>> PARAMETERS
>> sata_device Pointer to the sata_device structure that contains
>> the global
>> status and control register values of a port
>> multiplier.
>>
>> DESCRIPTION
>> sata_check_pmult_blacklist function is called by the SATA HBA
>> driver in order to check if a port multiplier has any quirk.
>>
>> Some port multipliers have quirks, e.x. register values are not
>> correctly
>> configured. The SATA framework maintains a blacklist hence these port
>> multiplier could be identified and properly handled.
>>
>> The model of a port multiplier can be uniquely identified by its
>> read-only
>> Global Status and Control Registers (GSCR[0,1,2]). In case a port
>> multiplier
>> is on the blacklist, this function will write the corresponding
>> flags into
>> satadev_add_info.
>>
>> RETURN VALUES
>> SATA_SUCCESS The device is found on the blacklist and the
>> sata_device
>> structure is successfully updated.
>>
>> SATA_FAILURE The device is not on the blacklist.
>>
>> CONTEXT
>> This function may be called from the interrupt context.
>>
>>
>> 5.5. Release Summary
>>
>> S11, S10 Update
>>
>> 5.6. Packaging Changes
>>
>> 5.6.1. Binaries Modified
>> ----------------------
>> /kernel/misc/sata
>> /kernel/misc/amd64/sata
>> /usr/include/sys/sata/sata_hba.h
>>
>> 5.6.2. Packages Affected
>> ----------------------
>> SUNWckr
>> SUNWhea
>>
>> 5.7. References
>>
>> [1] PSARC/2004/779 - SATA Framework Support
>> [2] PSARC/2005/679 - SATA Framework Support (Updated)
>> [3] PSARC/2007/274 - SATA Framework Interface Revision
>> [4] PSARC/2008/448 - SATA Framework Addition
>> [5] SATA Specification 2.6, Serial ATA International Organization
>> [6] SATA Port Multiplier Specification 1.2, Serial ATA International
>> Organization
>>
>> 6. Resources and Schedule
>> 6.4. Steering Committee requested information
>> 6.4.1. Consolidation C-team Name:
>> ON
>> 6.5. ARC review type: Automatic
>> 6.6. ARC Exposure: open
>>
>>
>
