Re: [PATCH 2/3] hw/ssi: fixup coverity issue
On Fri, Aug 12, 2022 at 02:21:40AM +, Wilfred Mallawa wrote:
> On Thu, 2022-08-11 at 16:23 +0200, Andrew Jones wrote:
> > On Thu, Aug 11, 2022 at 09:02:00AM +1000, Wilfred Mallawa wrote:
> > > From: Wilfred Mallawa
> > >
> > > This patch addresses the coverity issues specified in [1],
> > > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> > > implemented to clean up the code.
> > >
> > > Additionally, the `EVENT_ENABLE` register is correctly updated
> > > to addr of `0x34`.
> >
> > This sounds like separate patch material.
> >
> > >
> > > [1]
> > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
> > >
> > > Fixes: Coverity CID 1488107
> > >
> > > Signed-off-by: Wilfred Mallawa
> > > ---
> > > hw/ssi/ibex_spi_host.c | 141 +++--
> > >
> > > 1 file changed, 78 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > > index 601041d719..8c35bfa95f 100644
> > > --- a/hw/ssi/ibex_spi_host.c
> > > +++ b/hw/ssi/ibex_spi_host.c
> > > @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
> > > FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
> > > FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
> > > FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
> > > -REG32(EVENT_ENABLE, 0x30)
> > > +REG32(EVENT_ENABLE, 0x34)
> > > FIELD(EVENT_ENABLE, RXFULL, 0, 1)
> > > FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
> > > FIELD(EVENT_ENABLE, RXWM, 2, 1)
> > > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t
> > > dividend)
> > >
> > > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> > > {
> > > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > > /* Empty the RX FIFO and assert RXEMPTY */
> > > fifo8_reset(>rx_fifo);
> > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> > > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> > > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > > }
> > >
> > > static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> > > {
> > > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > > /* Empty the TX FIFO and assert TXEMPTY */
> > > fifo8_reset(>tx_fifo);
> > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> > > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> > > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > > }
> > >
> > > static void ibex_spi_host_reset(DeviceState *dev)
> > > @@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState
> > > *dev)
> > > */
> > > static void ibex_spi_host_irq(IbexSPIHostState *s)
> > > {
> > > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > > - & R_INTR_ENABLE_ERROR_MASK;
> > > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > > - & R_INTR_ENABLE_SPI_EVENT_MASK;
> > > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > > - & R_INTR_STATE_ERROR_MASK;
> > > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > > - & R_INTR_STATE_SPI_EVENT_MASK;
> > > + bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > > + INTR_ENABLE, ERROR);
> > > +
> > > + bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > > + INTR_ENABLE, SPI_EVENT);
> > > +
> > > + bool err_pending = FIELD_EX32(s-
> > > >regs[IBEX_SPI_HOST_INTR_STATE],
> > > + INTR_STATE, ERROR);
> > > +
> > > + bool status_pending = FIELD_EX32(s-
> > > >regs[IBEX_SPI_HOST_INTR_STATE],
> > > + INTR_STATE, SPI_EVENT);
> >
> > uint32_t reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
> > bool error_en = FIELD_EX32(reg, INTR_ENABLE, ERROR);
> > bool event_en = FIELD_EX32(reg, INTR_ENABLE, SPI_EVENT);
> > ...
> >
> > would like nicer, IMHO.
> as per the comment below, do you mean to make all the conditions
> variables here? and substitute those for the if statements instead of
> the `FIELD_..` macros?
For the above code, the suggestion I gave is what I'm thinking. For the
below code, keep the FIELD macros in place, but shrink the length of the
line by doing
uint32_t enable = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
uint32_t status = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
at the top, and then
} else if (FIELD_EX32(enable, ERROR_ENABLE, CMDBUSY) &&
FIELD_EX32(status, ERROR_STATUS, CMDBUSY)) {
...
> >
> >
> > > +
> > > int err_irq = 0, event_irq = 0;
> > >
> > > /* Error IRQ enabled and Error IRQ Cleared */
> > > if (error_en && !err_pending) {
> > > /* Event enabled, Interrupt Test Error */
> > > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] &
> > > R_INTR_TEST_ERROR_MASK) {
> > > + if
Re: [PATCH 2/3] hw/ssi: fixup coverity issue
On Thu, 2022-08-11 at 10:55 +0800, Bin Meng wrote:
> On Thu, Aug 11, 2022 at 8:58 AM Wilfred Mallawa
> wrote:
> >
> > From: Wilfred Mallawa
> >
> > This patch addresses the coverity issues specified in [1],
> > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> > implemented to clean up the code.
> >
> > Additionally, the `EVENT_ENABLE` register is correctly updated
> > to addr of `0x34`.
> >
> > [1]
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
> >
> > Fixes: Coverity CID 1488107
> >
> > Signed-off-by: Wilfred Mallawa
>
> nits: please add "ibex_spi" to the tag, like hw/ssi: ibex_spi:
will add in v2 :)
>
> > ---
> > hw/ssi/ibex_spi_host.c | 141 +++--
> >
> > 1 file changed, 78 insertions(+), 63 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index 601041d719..8c35bfa95f 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
> > FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
> > FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
> > FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
> > -REG32(EVENT_ENABLE, 0x30)
> > +REG32(EVENT_ENABLE, 0x34)
> > FIELD(EVENT_ENABLE, RXFULL, 0, 1)
> > FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
> > FIELD(EVENT_ENABLE, RXWM, 2, 1)
> > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t
> > dividend)
> >
> > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the RX FIFO and assert RXEMPTY */
> > fifo8_reset(>rx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the TX FIFO and assert TXEMPTY */
> > fifo8_reset(>tx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_host_reset(DeviceState *dev)
> > @@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState
> > *dev)
> > */
> > static void ibex_spi_host_irq(IbexSPIHostState *s)
> > {
> > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_ERROR_MASK;
> > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_SPI_EVENT_MASK;
> > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_ERROR_MASK;
> > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_SPI_EVENT_MASK;
> > + bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > + INTR_ENABLE, ERROR);
> > +
> > + bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > + INTR_ENABLE, SPI_EVENT);
> > +
> > + bool err_pending = FIELD_EX32(s-
> > >regs[IBEX_SPI_HOST_INTR_STATE],
> > + INTR_STATE, ERROR);
> > +
> > + bool status_pending = FIELD_EX32(s-
> > >regs[IBEX_SPI_HOST_INTR_STATE],
> > + INTR_STATE, SPI_EVENT);
> > +
> > int err_irq = 0, event_irq = 0;
> >
> > /* Error IRQ enabled and Error IRQ Cleared */
> > if (error_en && !err_pending) {
> > /* Event enabled, Interrupt Test Error */
> > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] &
> > R_INTR_TEST_ERROR_MASK) {
> > + if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST],
> > INTR_TEST, ERROR)) {
> > err_irq = 1;
> > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> > - & R_ERROR_ENABLE_CMDBUSY_MASK) &&
> > - s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> > - & R_ERROR_STATUS_CMDBUSY_MASK) {
> > + } else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> > + ERROR_ENABLE, CMDBUSY) &&
> > + FIELD_EX32(s-
> > >regs[IBEX_SPI_HOST_ERROR_STATUS],
> > + ERROR_STATUS, CMDBUSY)) {
> > /* Wrote to COMMAND when not READY */
> > err_irq = 1;
> > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> > - & R_ERROR_ENABLE_CMDINVAL_MASK) &&
> > - s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> > - & R_ERROR_STATUS_CMDINVAL_MASK) {
> > + } else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> > + ERROR_ENABLE, CMDINVAL) &&
> > +
Re: [PATCH 2/3] hw/ssi: fixup coverity issue
On Thu, Aug 11, 2022 at 09:02:00AM +1000, Wilfred Mallawa wrote:
> From: Wilfred Mallawa
>
> This patch addresses the coverity issues specified in [1],
> as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> implemented to clean up the code.
>
> Additionally, the `EVENT_ENABLE` register is correctly updated
> to addr of `0x34`.
This sounds like separate patch material.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
>
> Fixes: Coverity CID 1488107
>
> Signed-off-by: Wilfred Mallawa
> ---
> hw/ssi/ibex_spi_host.c | 141 +++--
> 1 file changed, 78 insertions(+), 63 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 601041d719..8c35bfa95f 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
> FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
> FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
> FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
> -REG32(EVENT_ENABLE, 0x30)
> +REG32(EVENT_ENABLE, 0x34)
> FIELD(EVENT_ENABLE, RXFULL, 0, 1)
> FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
> FIELD(EVENT_ENABLE, RXWM, 2, 1)
> @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
>
> static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> /* Empty the RX FIFO and assert RXEMPTY */
> fifo8_reset(>rx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> /* Empty the TX FIFO and assert TXEMPTY */
> fifo8_reset(>tx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_host_reset(DeviceState *dev)
> @@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState *dev)
> */
> static void ibex_spi_host_irq(IbexSPIHostState *s)
> {
> -bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_ERROR_MASK;
> -bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_SPI_EVENT_MASK;
> -bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_ERROR_MASK;
> -bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_SPI_EVENT_MASK;
> +bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> + INTR_ENABLE, ERROR);
> +
> +bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> + INTR_ENABLE, SPI_EVENT);
> +
> +bool err_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
> + INTR_STATE, ERROR);
> +
> +bool status_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
> + INTR_STATE, SPI_EVENT);
uint32_t reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
bool error_en = FIELD_EX32(reg, INTR_ENABLE, ERROR);
bool event_en = FIELD_EX32(reg, INTR_ENABLE, SPI_EVENT);
...
would like nicer, IMHO.
> +
> int err_irq = 0, event_irq = 0;
>
> /* Error IRQ enabled and Error IRQ Cleared */
> if (error_en && !err_pending) {
> /* Event enabled, Interrupt Test Error */
> -if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
> +if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST], INTR_TEST, ERROR))
> {
> err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CMDBUSY_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDBUSY_MASK) {
> +} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> + ERROR_ENABLE, CMDBUSY) &&
> +FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
> + ERROR_STATUS, CMDBUSY)) {
> /* Wrote to COMMAND when not READY */
> err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CMDINVAL_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDINVAL_MASK) {
> +} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> + ERROR_ENABLE, CMDINVAL) &&
> +FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
> + ERROR_STATUS, CMDINVAL)) {
> /* Invalid command
Re: [PATCH 2/3] hw/ssi: fixup coverity issue
On Thu, Aug 11, 2022 at 8:58 AM Wilfred Mallawa
wrote:
>
> From: Wilfred Mallawa
>
> This patch addresses the coverity issues specified in [1],
> as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> implemented to clean up the code.
>
> Additionally, the `EVENT_ENABLE` register is correctly updated
> to addr of `0x34`.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
>
> Fixes: Coverity CID 1488107
>
> Signed-off-by: Wilfred Mallawa
nits: please add "ibex_spi" to the tag, like hw/ssi: ibex_spi:
> ---
> hw/ssi/ibex_spi_host.c | 141 +++--
> 1 file changed, 78 insertions(+), 63 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 601041d719..8c35bfa95f 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
> FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
> FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
> FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
> -REG32(EVENT_ENABLE, 0x30)
> +REG32(EVENT_ENABLE, 0x34)
> FIELD(EVENT_ENABLE, RXFULL, 0, 1)
> FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
> FIELD(EVENT_ENABLE, RXWM, 2, 1)
> @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
>
> static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> /* Empty the RX FIFO and assert RXEMPTY */
> fifo8_reset(>rx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> /* Empty the TX FIFO and assert TXEMPTY */
> fifo8_reset(>tx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_host_reset(DeviceState *dev)
> @@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState *dev)
> */
> static void ibex_spi_host_irq(IbexSPIHostState *s)
> {
> -bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_ERROR_MASK;
> -bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_SPI_EVENT_MASK;
> -bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_ERROR_MASK;
> -bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_SPI_EVENT_MASK;
> +bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> + INTR_ENABLE, ERROR);
> +
> +bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> + INTR_ENABLE, SPI_EVENT);
> +
> +bool err_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
> + INTR_STATE, ERROR);
> +
> +bool status_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
> + INTR_STATE, SPI_EVENT);
> +
> int err_irq = 0, event_irq = 0;
>
> /* Error IRQ enabled and Error IRQ Cleared */
> if (error_en && !err_pending) {
> /* Event enabled, Interrupt Test Error */
> -if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
> +if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST], INTR_TEST, ERROR))
> {
> err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CMDBUSY_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDBUSY_MASK) {
> +} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> + ERROR_ENABLE, CMDBUSY) &&
> +FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
> + ERROR_STATUS, CMDBUSY)) {
> /* Wrote to COMMAND when not READY */
> err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CMDINVAL_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDINVAL_MASK) {
> +} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
> + ERROR_ENABLE, CMDINVAL) &&
> +FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
> + ERROR_STATUS, CMDINVAL)) {
> /* Invalid command segment */
> err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
> -
[PATCH 2/3] hw/ssi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
Additionally, the `EVENT_ENABLE` register is correctly updated
to addr of `0x34`.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 141 +++--
1 file changed, 78 insertions(+), 63 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..8c35bfa95f 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
-REG32(EVENT_ENABLE, 0x30)
+REG32(EVENT_ENABLE, 0x34)
FIELD(EVENT_ENABLE, RXFULL, 0, 1)
FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
FIELD(EVENT_ENABLE, RXWM, 2, 1)
@@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
+ INTR_ENABLE, ERROR);
+
+bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
+ INTR_ENABLE, SPI_EVENT);
+
+bool err_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
+ INTR_STATE, ERROR);
+
+bool status_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
+ INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST], INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
+ ERROR_ENABLE, CMDBUSY) &&
+FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
+ ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
+ ERROR_ENABLE, CMDINVAL) &&
+FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
+ ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
+ ERROR_ENABLE, CSIDINVAL) &&
+FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
+ ERROR_STATUS, CSIDINVAL)) {
