Re: [PATCH v3 03/11] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 7/13/20 6:36 PM, Philippe Mathieu-Daudé wrote:
> On 6/15/20 4:06 PM, Peter Maydell wrote:
>> On Fri, 5 Jun 2020 at 11:25, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>> Only move the state machine to ReceivingData if there is no
>>> pending error.  This avoids later OOB access while processing
>>> commands queued.
>>>
>>>   "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"
>>>
>>>   4.3.3 Data Read
>>>
>>>   Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
>>>   occurred and no data transfer is performed.
>>>
>>>   4.3.4 Data Write
>>>
>>>   Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
>>>   occurred and no data transfer is performed.
>>
>> It's not clear from the spec that this should also
>> apply to WP_VIOLATION errors. The text about WP_VIOLATION
>> suggests that it is handled by aborting the data transfer
>> (ie set the error bit, stay in receive-data state, wait for
>> a stop command, but ignore all further data transfer),
>> which is I think distinct from "rejecting" the command.
>>
>> If that theory is right then moving the check for the
>> ADDRESS_ERROR in this patch is correct but the WP_VIOLATION
>> tests should stay as they are, I think.
> 
> I found the correct behavior in table '4.10.1 Card Status':
> 
> * OUT_OF_RANGE
>   ============
>   Type: E R X
> 
>   The command's argument was out of the allowed range for this card.
> 
> * ADDRESS_ERROR
>   =============
>   Type: E R X
> 
>   A misaligned address which did not match the block length was
>   used in the command.
> 
> * WP_VIOLATION
>   ============
>   Type: E R X
> 
>   Set when the host attempts to write to a protected block or to
>   the temporary or permanent write protected card.
> 
> With 'Type':
> 
> - E: Error bit.
> - R: Detected and set for the actual command response.
> - X: Detected and set during command execution. The host can get
>      the status by issuing a command with R1 response.
> 
> Block Read
> ==========
> [...]
> When the last block of user area is read using CMD18, the host should
> ignore OUT_OF_RANGE error that may occur even the sequence is correct.
> If the host uses partial blocks whose accumulated length is not block
> aligned and block misalignment is not allowed, the card shall detect
> a block misalignment at the beginning of the first misaligned block,
> set the ADDRESS_ERROR error bit in the status register, abort
> transmission and wait in the Data State for a stop command.
> 
> 
> So I understand we want OUT_OF_RANGE (returned via R1).

We always returned ADDRESS_ERROR and guests never complained, so I don't
plan to modify this bit for 5.1. What matters is "command is rejected
if error occurred and no data transfer is performed".