On 2/4/20 3:28 PM, Guenter Roeck wrote:
> On 2/4/20 12:53 AM, Cédric Le Goater wrote:
>> On 2/3/20 7:09 PM, Guenter Roeck wrote:
>>> Always report 6 bytes of JEDEC data. Fill remaining data with 0.
>>>
>>> For unsupported commands, keep sending a value of 0 until the chip
>>> is deselected.
>>>
>>> Both changes avoid attempts to decode random commands. Up to now this
>>> happened if the reported Jedec data was shorter than 6 bytes but the
>>> host read 6 bytes, and with all unsupported commands.
>>
>> Do you have a concrete example for that ? machine and flash model.
>>
>
> I noticed it while tracking down the bug fixed in patch 3 of the series,
> ie with AST2500 evb using w25q256 flash, but it happens with all machines
> using SPI NOR flash (ie all aspeed bmc machines) when running the Linux
> kernel. Most of the time it doesn't cause harm, unless the host sends
> an "address" as part of an unsupported command which happens to include
> a valid command code.
ok. we will need to model SFDP one day.
Are you using the OpenBMC images or do you have your own ?
>
>>> Signed-off-by: Guenter Roeck <li...@roeck-us.net>
>>> ---
>>> hw/block/m25p80.c | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>> index 63e050d7d3..aca75edcc1 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -1040,8 +1040,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>> for (i = 0; i < s->pi->id_len; i++) {
>>> s->data[i] = s->pi->id[i];
>>> }
>>> + for (; i < SPI_NOR_MAX_ID_LEN; i++) {
>>> + s->data[i] = 0;
>>> + }
>>
>> It seems that data should be reseted in m25p80_cs() also.
>>
> Are you sure ?
>
> The current implementation sets s->data[] as needed when command decode
> is complete. That seems less costly to me.
ok.
Reviewed-by: Cédric Le Goater <c...@kaod.org>
Thanks,
C.
> Thanks,
> Guenter
>
>>> - s->len = s->pi->id_len;
>>> + s->len = SPI_NOR_MAX_ID_LEN;
>>> s->pos = 0;
>>> s->state = STATE_READING_DATA;
>>> break;
>>> @@ -1158,6 +1161,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>> s->quad_enable = false;
>>> break;
>>> default:
>>> + s->pos = 0;
>>> + s->len = 1;
>>> + s->state = STATE_READING_DATA;
>>> + s->data_read_loop = true;
>>> + s->data[0] = 0;
>>> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>> break;
>>> }
>>>
>>
>
