On Wed, Feb 05, 2020 at 11:08:07AM +0100, Cédric Le Goater wrote:
> 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 ?
>
I am running images built from upstream/stable kernel branches.
Guenter
> >
> >>> 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;
> >>> }
> >>>
> >>
> >
>
