On Wed, Oct 25, 2017 at 2:15 PM, Darren Kenny <darren.ke...@oracle.com> wrote:
> LGTM now, thanks. > > Reviewed-by: Darren Kenny <darren.ke...@oracle.com> > Thanks Darren, Sundeep > > Thanks, > > Darren. > > > On Wed, Oct 25, 2017 at 07:59:04AM +0530, Subbaraya Sundeep wrote: > >> Fixed incorrect frame size mask, validated maximum frame >> size in spi_write and removed dead code. >> >> Signed-off-by: Subbaraya Sundeep <sundeep.l...@gmail.com> >> --- >> v4: >> changed %d to %u while logging frame size error. >> v3: >> Added comment that [31:6] bits are reserved in R_SPI_DFSIZE >> register and logged incorrect value too in guest error(suggested >> by Darren). >> v2: >> else if -> else in set_fifodepth >> log guest error when frame size is more than 32 >> >> hw/ssi/mss-spi.c | 18 ++++++++++++++---- >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c >> index 5a8e308..d60daba 100644 >> --- a/hw/ssi/mss-spi.c >> +++ b/hw/ssi/mss-spi.c >> @@ -76,9 +76,10 @@ >> #define C_BIGFIFO (1 << 29) >> #define C_RESET (1 << 31) >> >> -#define FRAMESZ_MASK 0x1F >> +#define FRAMESZ_MASK 0x3F >> #define FMCOUNT_MASK 0x00FFFF00 >> #define FMCOUNT_SHIFT 8 >> +#define FRAMESZ_MAX 32 >> >> static void txfifo_reset(MSSSpiState *s) >> { >> @@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s) >> s->fifo_depth = 32; >> } else if (size <= 16) { >> s->fifo_depth = 16; >> - } else if (size <= 32) { >> - s->fifo_depth = 8; >> } else { >> - s->fifo_depth = 4; >> + s->fifo_depth = 8; >> } >> } >> >> @@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr, >> if (s->enabled) { >> break; >> } >> + /* >> + * [31:6] bits are reserved bits and for future use. >> + * [5:0] are for frame size. Only [5:0] bits are validated >> + * during write, [31:6] bits are untouched. >> + */ >> + if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) { >> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %u >> provided." >> + "Maximum frame size is %u\n", >> + __func__, value & FRAMESZ_MASK, FRAMESZ_MAX); >> + break; >> + } >> s->regs[R_SPI_DFSIZE] = value; >> break; >> >> -- >> 2.5.0 >> >> >>