qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Jason Wang" <jasowang@redhat.com>,
	"Vikram Garhwal" <fnu.vikram@xilinx.com>,
	"Ondrej Ille" <ondrej.ille@gmail.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Jan Charvát" <charvj10@fel.cvut.cz>
Subject: Re: [PATCH for-5.2 v2 1/4] hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer
Date: Tue, 10 Nov 2020 20:30:58 +0100	[thread overview]
Message-ID: <202011102030.58186.pisa@cmp.felk.cvut.cz> (raw)
In-Reply-To: <CAFEAcA8ujRALffz8KfvxdVb6NTcNLNdR4tF6picWnF6wk3R64A@mail.gmail.com>

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


  reply	other threads:[~2020-11-10 19:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 17:06 [PATCH for-5.2 v2 0/4] hw/net/can/ctucan: fix Coverity and other issues Peter Maydell
2020-11-10 17:06 ` [PATCH for-5.2 v2 1/4] hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer Peter Maydell
2020-11-10 18:01   ` Pavel Pisa
2020-11-10 18:24     ` Peter Maydell
2020-11-10 19:30       ` Pavel Pisa [this message]
2020-11-10 21:18         ` Peter Maydell
2020-11-10 22:02           ` Pavel Pisa
2020-11-10 17:06 ` [PATCH for-5.2 v2 2/4] hw/net/can/ctucan: Avoid unused value in ctucan_send_ready_buffers() Peter Maydell
2020-11-10 19:36   ` Pavel Pisa
2020-11-10 21:05     ` Peter Maydell
2020-11-10 17:06 ` [PATCH for-5.2 v2 3/4] hw/net/can/ctucan_core: Handle big-endian hosts Peter Maydell
2020-11-10 19:37   ` Pavel Pisa
2020-11-10 17:06 ` [PATCH for-5.2 v2 4/4] hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers Peter Maydell
2020-11-10 18:03   ` Pavel Pisa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202011102030.58186.pisa@cmp.felk.cvut.cz \
    --to=pisa@cmp.felk.cvut.cz \
    --cc=charvj10@fel.cvut.cz \
    --cc=fnu.vikram@xilinx.com \
    --cc=jasowang@redhat.com \
    --cc=ondrej.ille@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).