All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Silva <afscoelho@gmail.com>
To: qemu-devel@nongnu.org
Cc: mst@redhat.com
Subject: [PATCH] virtio: Prevent double swap due to target pre 1.0 VirtIO
Date: Wed,  8 Jan 2020 13:16:18 -0300	[thread overview]
Message-ID: <20200108161618.221116-1-afscoelho@gmail.com> (raw)

Remove the bswap function calls after reading and before writing
memory bytes in virtio_pci_config_read and virtio_pci_config_write
because they are reverting back an already swapped bytes.

Consider the table below in the context of virtio_pci_config_read
function.

Host   Target  virtio-config-read[wl]
               swap?                   virtio-is-big-endian?   extra bswap?   Should be   Final result   Final result ok?
----- ------- ------------------------ ----------------------- -------------- ----------- -------------- ------------------
LE     BE      s(x)                    true                    s(s(x))        s(x)        x              No
LE     LE      x                       false                   -              x           x              Yes
BE     LE      s(x)                    false                   -              s(x)        s(x)           Yes
BE     BE      x                       true                    s(x)           x           s(x)           No

In table above, when target is big endian and VirtIO is pre 1.0,
function virtio_is_big_endian would return true and the extra
swap would be executed, reverting the previous swap made by
virtio_config_read[wl].

The 's(x)' means that a swap function was applied at
address x. 'LE' is little endian and 'BE' is big endian. The
'Final result' column is the returned value from
virtio_pci_config_read, considering a target Virtio pre 1.0.
'x' means that target's value was not swapped in Qemu, 's(x)' means
that Qemu will use a swapped value.

If we remove the extra swap made in virtio_pci_config_read we will
have the correct result in any host/target combination, both for
VirtIO pre 1.0 or later versions.

The same reasoning applies to virtio_pci_config_write.

Signed-off-by: Andre Silva <afscoelho@gmail.com>
---
 hw/virtio/virtio-pci.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c6b47a9c73..4ba9e847f3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -431,15 +431,9 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
         break;
     case 2:
         val = virtio_config_readw(vdev, addr);
-        if (virtio_is_big_endian(vdev)) {
-            val = bswap16(val);
-        }
         break;
     case 4:
         val = virtio_config_readl(vdev, addr);
-        if (virtio_is_big_endian(vdev)) {
-            val = bswap32(val);
-        }
         break;
     }
     return val;
@@ -465,15 +459,9 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
         virtio_config_writeb(vdev, addr, val);
         break;
     case 2:
-        if (virtio_is_big_endian(vdev)) {
-            val = bswap16(val);
-        }
         virtio_config_writew(vdev, addr, val);
         break;
     case 4:
-        if (virtio_is_big_endian(vdev)) {
-            val = bswap32(val);
-        }
         virtio_config_writel(vdev, addr, val);
         break;
     }
-- 
2.24.1



             reply	other threads:[~2020-01-08 16:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 16:16 Andre Silva [this message]
2020-01-08 17:51 ` [PATCH] virtio: Prevent double swap due to target pre 1.0 VirtIO Peter Maydell
2020-01-08 19:37   ` André Silva
2020-01-09 10:50 ` Michael S. Tsirkin
2020-01-09 12:25   ` André Silva
2020-01-09 12:39     ` Michael S. Tsirkin
2020-01-09 16:06       ` Greg Kurz
2020-01-09 21:18         ` André Silva
2020-01-10  8:55           ` Greg Kurz
2020-01-10 12:00             ` André Silva
2020-01-10 14:50               ` Greg Kurz
2020-01-10 17:09                 ` André Silva
  -- strict thread matches above, loose matches on Subject: below --
2020-01-08 12:56 Andre Silva
2020-01-08 12:56 ` Andre Silva
2020-01-08 15:49   ` no-reply

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=20200108161618.221116-1-afscoelho@gmail.com \
    --to=afscoelho@gmail.com \
    --cc=mst@redhat.com \
    --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 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.