Re: [PATCH for-5.2 v2 1/4] hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer

[Pavel Pisa]

Hello Peter,

On Tuesday 10 of November 2020 19:24:03 Peter Maydell wrote:
> For unaligned accesses, for 6.0, I think the code for doing
> them to the txbuff at least is straightforward:
>
>    if (buff_num < CTUCAN_CORE_TXBUF_NUM &&
>        (addr + size) < CTUCAN_CORE_MSG_MAX_LEN) {
>       stn_le_p(s->tx_buffer[buff_num].data + addr, size, val);
>    }
>
> (stn_le_p takes care of doing an appropriate-width write.)

Thanks, great to know, I like that much.
Only small nitpicking, it should be (addr + size) <= CTUCAN_CORE_MSG_MAX_LEN

So whole code I am testing now

    if (addr >= CTU_CAN_FD_TXTB1_DATA_1) {
        int buff_num;
        addr -= CTU_CAN_FD_TXTB1_DATA_1;
        buff_num = addr / CTUCAN_CORE_TXBUFF_SPAN;
        addr %= CTUCAN_CORE_TXBUFF_SPAN;
        if ((buff_num < CTUCAN_CORE_TXBUF_NUM) &&
            ((addr + size) <= sizeof(s->tx_buffer[buff_num].data))) {
            stn_le_p(s->tx_buffer[buff_num].data + addr, size, val);
        }
    } else {

So I have applied you whole series with above update. All works correctly
on x86_64 Linux host and with Linux x86_64 and MIPS big endian guests.

Please update to this combination.
I do not expect to have byte writes in our drivers but real core
supports byte enable bus signals.

Thanks much for teaching me QEMU stn_le_p.
In the fact, we are discussion about similar slution of peripherals
access for our https://github.com/cvut/QtMips/ education emulator
(performance vise a total toy when compared to QEMU).

It would worth to enable byte writes into registers as well.
But I would not do it before release. It would be more complex.
The reads supports bytes by reading 32/bit word and then shifting
and masking right bits into result. Cross word unaligned reads
are not supported. Again no reason for them now.

You can add

Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

to whole series.

Thanks,

Pavel