All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Alexandra Diupina <adiupina@astralinux.ru>
Cc: Alistair Francis <alistair@alistair23.me>,
	"Konrad, Frederic" <Frederic.Konrad@amd.com>,
	 "Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	sdl.qemu@linuxtesting.org
Subject: Re: [PATCH v2 RFC] fix host-endianness bug and prevent overflow
Date: Wed, 24 Apr 2024 17:04:27 +0100	[thread overview]
Message-ID: <CAFEAcA-=kk_TQVRLLQvH96DC-ffmDqd_hU5=z=Og8ntYGxPUeg@mail.gmail.com> (raw)
In-Reply-To: <20240424125324.1628-1-adiupina@astralinux.ru>

On Wed, 24 Apr 2024 at 13:53, Alexandra Diupina <adiupina@astralinux.ru> wrote:
>
> Add a type cast and use extract64() instead of extract32()
> to avoid integer overflow on addition. Fix bit fields
> extraction according to documentation.
> Also fix host-endianness bug by swapping desc fields from guest
> memory order to host memory order after dma_memory_read().

Thanks for this patch. Could you split it into two, please?
The error in handling of the descriptor extension fields is a
separate problem from the endianness bug, so they should be
fixed in separate patches.

> @@ -660,6 +660,24 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel,
>              break;
>          }
>
> +        /* Convert from LE into host endianness.  */
> +        desc.control = le32_to_cpu(desc.control);
> +        desc.descriptor_id = le32_to_cpu(desc.descriptor_id);
> +        desc.xfer_size = le32_to_cpu(desc.xfer_size);
> +        desc.line_size_stride = le32_to_cpu(desc.line_size_stride);
> +        desc.timestamp_lsb = le32_to_cpu(desc.timestamp_lsb);
> +        desc.timestamp_msb = le32_to_cpu(desc.timestamp_msb);
> +        desc.address_extension = le32_to_cpu(desc.address_extension);
> +        desc.next_descriptor = le32_to_cpu(desc.next_descriptor);
> +        desc.source_address = le32_to_cpu(desc.source_address);
> +        desc.address_extension_23 = le32_to_cpu(desc.address_extension_23);
> +        desc.address_extension_45 = le32_to_cpu(desc.address_extension_45);
> +        desc.source_address2 = le32_to_cpu(desc.source_address2);
> +        desc.source_address3 = le32_to_cpu(desc.source_address3);
> +        desc.source_address4 = le32_to_cpu(desc.source_address4);
> +        desc.source_address5 = le32_to_cpu(desc.source_address5);
> +        desc.crc = le32_to_cpu(desc.crc);
> +
>          xlnx_dpdma_update_desc_info(s, channel, &desc);
>
>  #ifdef DEBUG_DPDMA

I would suggest factoring out a function like

static MemTxResult xlnx_dpdma_read_descriptor(XlnxDPDMAState *s,
uint64_t desc_addr,
                                              DPDMADescriptor *desc)

which wraps up "read the descriptor from desc_addr and fill in desc"
as a single operation (by calling dma_memory_read() and then
doing the byteswap).

thanks
-- PMM


  reply	other threads:[~2024-04-24 16:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12  8:13 [PATCH RFC] prevent overflow in xlnx_dpdma_desc_get_source_address() Alexandra Diupina
2024-04-12 10:06 ` Peter Maydell
2024-04-16 17:56   ` Alexandra Diupina
2024-04-16 18:30     ` Edgar E. Iglesias
2024-04-17 10:05   ` Konrad, Frederic
2024-04-23 10:23     ` Alexandra Diupina
2024-04-23 10:51       ` Peter Maydell
2024-04-24 12:53         ` [PATCH v2 RFC] fix host-endianness bug and prevent overflow Alexandra Diupina
2024-04-24 16:04           ` Peter Maydell [this message]
2024-04-24 18:13             ` [PATCH] fix host-endianness bug Alexandra Diupina
2024-04-25  9:26               ` Peter Maydell
2024-04-25 10:07                 ` [PATCH v2] " Alexandra Diupina
2024-04-25 10:42                   ` Philippe Mathieu-Daudé
2024-04-25 13:41                     ` [PATCH v3] fix endianness bug Alexandra Diupina
2024-04-25 15:24                       ` Richard Henderson
2024-04-24 18:13             ` [PATCH] fix bit fields extraction and prevent overflow Alexandra Diupina
2024-04-25 19:25               ` Peter Maydell

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='CAFEAcA-=kk_TQVRLLQvH96DC-ffmDqd_hU5=z=Og8ntYGxPUeg@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=Frederic.Konrad@amd.com \
    --cc=adiupina@astralinux.ru \
    --cc=alistair@alistair23.me \
    --cc=edgar.iglesias@gmail.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sdl.qemu@linuxtesting.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.