From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.86_2) id 1hpaZt-0000Xw-8A for mharc-qemu-riscv@gnu.org; Mon, 22 Jul 2019 11:50:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53531) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpaXo-0001rG-8H for qemu-riscv@nongnu.org; Mon, 22 Jul 2019 11:48:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpaXm-0007YV-M4 for qemu-riscv@nongnu.org; Mon, 22 Jul 2019 11:48:08 -0400 Received: from smtpe1.intersmtp.com ([62.239.224.234]:43242) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpaXl-0007WK-Pe; Mon, 22 Jul 2019 11:48:06 -0400 Received: from tpw09926dag18g.domain1.systemhost.net (10.9.212.34) by RDW083A012ED68.bt.com (10.187.98.38) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 22 Jul 2019 16:47:47 +0100 Received: from tpw09926dag18e.domain1.systemhost.net (10.9.212.18) by tpw09926dag18g.domain1.systemhost.net (10.9.212.34) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 22 Jul 2019 16:48:01 +0100 Received: from tpw09926dag18e.domain1.systemhost.net ([fe80::a946:6348:ccf4:fa6c]) by tpw09926dag18e.domain1.systemhost.net ([fe80::a946:6348:ccf4:fa6c%12]) with mapi id 15.00.1395.000; Mon, 22 Jul 2019 16:48:00 +0100 From: To: CC: , , , , , , , , , , , , , , , , , , , , , , , , Thread-Topic: [Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp Thread-Index: AQHVQKTX+Ks5rP6+AkSy2PJ+oMCFiQ== Date: Mon, 22 Jul 2019 15:48:00 +0000 Message-ID: <1563810480347.95681@bt.com> References: In-Reply-To: Accept-Language: en-AU, en-GB, en-US Content-Language: en-AU X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.187.101.37] Content-Type: multipart/alternative; boundary="_000_156381048034795681btcom_" MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 62.239.224.234 X-Mailman-Approved-At: Mon, 22 Jul 2019 11:50:10 -0400 Subject: [Qemu-riscv] [Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp X-BeenThere: qemu-riscv@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jul 2019 15:48:10 -0000 --_000_156381048034795681btcom_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable On 17/07/19 08:06, Paolo Bonzini wrote: > My main concern is that MO_BE/MO_LE/MO_TE do not really apply to the > memory.c paths. MO_BSWAP is never passed into the MemOp, even if target > endianness !=3D host endianness. > > Therefore, you could return MO_TE | MO_{8,16,32,64} from this function, > and change memory_region_endianness_inverted to test > HOST_WORDS_BIGENDIAN instead of TARGET_WORDS_BIGENDIAN. Then the two > MO_BSWAPs (one from MO_TE, one from adjust_endianness because > memory_region_endianness_inverted returns true) cancel out if the > memory region's endianness is the same as the host's but different > from the target's. > > Some care is needed in virtio_address_space_write and zpci_write_bar. I > think the latter is okay, while the former could do something like this: > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ce928f2429..61885f020c 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -541,16 +541,16 @@ void virtio_address_space_write(VirtIOPCIProxy *pro= xy, > hwaddr addr, > val =3D pci_get_byte(buf); > break; > case 2: > - val =3D cpu_to_le16(pci_get_word(buf)); > + val =3D pci_get_word(buf); > break; > case 4: > - val =3D cpu_to_le32(pci_get_long(buf)); > + val =3D pci_get_long(buf); > break; > default: > /* As length is under guest control, handle illegal values. */ > return; > } > - memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIF= IED); > + memory_region_dispatch_write(mr, addr, val, size_memop(len) & ~MO_BS= WAP, > MEMTXATTRS_UNSPECIFIED); > } > > static void Sorry Paolo, I noted the need to take care in virtio_address_space_write an= d zpci_write_bar but did not understand. > Some care is needed in virtio_address_space_write and zpci_write_bar. Is this advice for my v1 implementation, or in the case of the MO_TE | MO_{8,16,32,64} idead suggested in the paragraph before? Signed-off-by: Tony Nguyen --- hw/virtio/virtio-pci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ce928f2..265f066 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -17,6 +17,7 @@ #include "qemu/osdep.h" +#include "exec/memop.h" #include "standard-headers/linux/virtio_pci.h" #include "hw/virtio/virtio.h" #include "hw/pci/pci.h" @@ -550,7 +551,8 @@ void virtio_address_space_write(VirtIOPCIProxy *proxy, = hwaddr addr, /* As length is under guest control, handle illegal values. */ return; } - memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIE= D); + memory_region_dispatch_write(mr, addr, val, SIZE_MEMOP(len), + MEMTXATTRS_UNSPECIFIED); } static void @@ -573,7 +575,8 @@ virtio_address_space_read(VirtIOPCIProxy *proxy, hwaddr= addr, /* Make sure caller aligned buf properly */ assert(!(((uintptr_t)buf) & (len - 1))); - memory_region_dispatch_read(mr, addr, &val, len, MEMTXATTRS_UNSPECIFIE= D); + memory_region_dispatch_read(mr, addr, &val, SIZE_MEMOP(len), + MEMTXATTRS_UNSPECIFIED); switch (len) { case 1: pci_set_byte(buf, val); -- 1.8.3.1 --_000_156381048034795681btcom_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

On 17/07/19 08:06, Paolo Bonzini wrot= e:

> My main concern is that MO_BE/MO_LE/MO_TE do not really apply to = the 
> memory.c paths.  MO_BSWAP is never passed into the MemOp, ev= en if target 
> endianness !=3D host endianness.
> 
> Therefore, you could return MO_TE | MO_{8,16,32,64} from this fun= ction, 
> and change memory_region_endianness_inverted to test 
> HOST_WORDS_BIGENDIAN instead of TARGET_WORDS_BIGENDIAN.  The= n the two
> MO_BSWAPs (one from MO_TE, one from adjust_endianness because
> memory_region_endianness_inverted returns true) cancel out if the=
> memory region's endianness is the same as the host's but differen= t
> from the target's.
> 
> Some care is needed in virtio_address_space_write and zpci_write_= bar.  I 
> think the latter is okay, while the former could do something lik= e this:
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index ce928f2429..61885f020c 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -541,16 +541,16 @@ void virtio_address_space_write(VirtIOP= CIProxy *proxy, 
> hwaddr addr,
>          val =3D pci_get_byte(buf);
>          break;
>      case 2:
> -        val =3D cpu_to_le16(pci_get_word(buf= ));
> +        val =3D pci_get_word(buf);
>          break;
>      case 4:
> -        val =3D cpu_to_le32(pci_get_long(buf= ));
> +        val =3D pci_get_long(buf);
>          break;
>      default:
>          /* As length is under guest con= trol, handle illegal values. */
>          return;
>      }
> -    memory_region_dispatch_write(mr, addr, val, len, M= EMTXATTRS_UNSPECIFIED);
> +    memory_region_dispatch_write(mr, addr, val, si= ze_memop(len) & ~MO_BSWAP, 
> MEMTXATTRS_UNSPECIFIED);
>  }
>  
>  static void

Sorry Paolo, I noted the need to take care in virtio_address_space_wri= te and
zpci_write_bar but did not understand.

> Some care is needed in virtio_address_space_write and zpci_write_= bar.
Is this advice for my v1 implementation, or in the case of the
MO_TE | MO_{8,16,32,64} idead suggested in the paragraph before?

Signed-off-by: Tony Nguyen <tony.nguyen@bt.com>
---
 hw/virtio/virtio-pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ce928f2..265f066 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -17,6 +17,7 @@
 
 #include "qemu/osdep.h"
 
+#include "exec/memop.h"
 #include "standard-headers/linux/virtio_pci.h"
 #include "hw/virtio/virtio.h"
 #include "hw/pci/pci.h"
@@ -550,7 +551,8 @@ void virtio_address_space_write(VirtIOPCIProxy= *proxy, hwaddr addr,
         /* As length is under guest control,= handle illegal values. */
         return;
     }
-    memory_region_dispatch_write(mr, addr, val, len, MEMTXA= TTRS_UNSPECIFIED);
+    memory_region_dispatch_write(mr, addr, val, SIZE_ME= MOP(len),
+                   &= nbsp;             MEMTXATTRS_UNSPECIFIED);
 }
 
 static void
@@ -573,7 +575,8 @@ virtio_address_space_read(VirtIOPCIProxy *prox= y, hwaddr addr,
     /* Make sure caller aligned buf properly */
     assert(!(((uintptr_t)buf) & (len - 1)));
 
-    memory_region_dispatch_read(mr, addr, &val, len, ME= MTXATTRS_UNSPECIFIED);
+    memory_region_dispatch_read(mr, addr, &val, SIZ= E_MEMOP(len),
+                   &= nbsp;            MEMTXATTRS_UNSPECIFIED);
     switch (len) {
     case 1:
         pci_set_byte(buf, val);
-- 
1.8.3.1



--_000_156381048034795681btcom_-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a5d:51d0:0:0:0:0:0 with SMTP id n16csp7042963wrv; Mon, 22 Jul 2019 08:48:39 -0700 (PDT) X-Google-Smtp-Source: APXvYqz1WbywSN8WvCIHj6KbotWtCq14OhZweZ/WJ61LOEcGa8eo9crTKoHp55rmqXVc/IPwBFm5 X-Received: by 2002:a1f:b48e:: with SMTP id d136mr25779645vkf.57.1563810519139; Mon, 22 Jul 2019 08:48:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563810519; cv=none; d=google.com; s=arc-20160816; b=PxAHhoKdeJbTy8RdAIZDd1xap0uyriR7Q13bkfcvTzjmDKZNLHnzzalFCvhjDj8Pmc Zef298MSjeO7iEVuvtHgrzU8sdha/fwJZsQD8zTewR3D6QhFwDdOf85gZmQx6Zv72/tu WuYd83xqi+EESIen/2EBle8PfN1cFnOAtfSdXVeTCgDMGndzm6p7C7l77wfnypNw+P5A 650A3W0s3r9Br7a/wN062Csl8K6NJ7H35pojTPJkAhi56ras8pvWAlCVOIv+P7ji/VMO lmPqelDI3aEUMZOMIuALP/dhJsxqZyEC7rHlMLurMqfkf3wWsVoOoLYpo280PEmkEjij YJRw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:content-language :accept-language:in-reply-to:references:message-id:date:thread-index :thread-topic:to:from; bh=zf3xdgvgL56FpRZLeZetkq9F1u7/uFRhGu0wLYE+jAE=; b=QQHrELVYCq1e7AWP8bTk2E3ayY3x4UyB6fLZWA8JCsQLrWUPh1oF2H3VkHu1dED+cn WR2lveIwC6mys42ap3UELFL3DaN3yGjt6xj23zcpmL4OtwyLhH9N0lSkRu8IdO1k0MAS mCox4cMPuTxvHyIJ9ZjRpqNdMyT0sWqnwL/uqrKj38z3iGuxHuHunDWWS1K72UGJFL5W lCF3aj789Kdfl2z4Az8cR3PFyS0i+WWuke/v5ROx+Mnr/puzReSiXWJUB2YEWXj9R5F+ yoWrK3GcCkweMd6SqFgRXkfB6wNYq97Zo6ZC4LAsEwrY9ZOPoCsMSPkK0Tdp7JUX95xG 6cuA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=bt.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id x128si9183989vsc.386.2019.07.22.08.48.38 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 22 Jul 2019 08:48:39 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=bt.com Received: from localhost ([::1]:34954 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpaYI-0003BE-9A for alex.bennee@linaro.org; Mon, 22 Jul 2019 11:48:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53579) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpaXu-0002D1-2J for qemu-devel@nongnu.org; Mon, 22 Jul 2019 11:48:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpaXs-0007bm-N0 for qemu-devel@nongnu.org; Mon, 22 Jul 2019 11:48:13 -0400 Received: from smtpe1.intersmtp.com ([62.239.224.234]:43242) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpaXl-0007WK-Pe; Mon, 22 Jul 2019 11:48:06 -0400 Received: from tpw09926dag18g.domain1.systemhost.net (10.9.212.34) by RDW083A012ED68.bt.com (10.187.98.38) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 22 Jul 2019 16:47:47 +0100 Received: from tpw09926dag18e.domain1.systemhost.net (10.9.212.18) by tpw09926dag18g.domain1.systemhost.net (10.9.212.34) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 22 Jul 2019 16:48:01 +0100 Received: from tpw09926dag18e.domain1.systemhost.net ([fe80::a946:6348:ccf4:fa6c]) by tpw09926dag18e.domain1.systemhost.net ([fe80::a946:6348:ccf4:fa6c%12]) with mapi id 15.00.1395.000; Mon, 22 Jul 2019 16:48:00 +0100 From: To: Thread-Topic: [Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp Thread-Index: AQHVQKTX+Ks5rP6+AkSy2PJ+oMCFiQ== Date: Mon, 22 Jul 2019 15:48:00 +0000 Message-ID: <1563810480347.95681@bt.com> References: In-Reply-To: Accept-Language: en-AU, en-GB, en-US Content-Language: en-AU X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.187.101.37] MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 62.239.224.234 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.23 Subject: [Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, walling@linux.ibm.com, mst@redhat.com, palmer@sifive.com, mark.cave-ayland@ilande.co.uk, Alistair.Francis@wdc.com, arikalo@wavecomp.com, david@redhat.com, pasic@linux.ibm.com, borntraeger@de.ibm.com, rth@twiddle.net, atar4qemu@gmail.com, ehabkost@redhat.com, sw@weilnetz.de, qemu-s390x@nongnu.org, qemu-arm@nongnu.org, david@gibson.dropbear.id.au, qemu-riscv@nongnu.org, cohuck@redhat.com, claudio.fontana@huawei.com, alex.williamson@redhat.com, qemu-ppc@nongnu.org, amarkovic@wavecomp.com, pbonzini@redhat.com, aurelien@aurel32.net Errors-To: qemu-devel-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-devel" X-TUID: 53dMYhTutwq1 On 17/07/19 08:06, Paolo Bonzini wrote: > My main concern is that MO_BE/MO_LE/MO_TE do not really apply to the > memory.c paths. MO_BSWAP is never passed into the MemOp, even if target > endianness !=3D host endianness. > > Therefore, you could return MO_TE | MO_{8,16,32,64} from this function, > and change memory_region_endianness_inverted to test > HOST_WORDS_BIGENDIAN instead of TARGET_WORDS_BIGENDIAN. Then the two > MO_BSWAPs (one from MO_TE, one from adjust_endianness because > memory_region_endianness_inverted returns true) cancel out if the > memory region's endianness is the same as the host's but different > from the target's. > > Some care is needed in virtio_address_space_write and zpci_write_bar. I > think the latter is okay, while the former could do something like this: > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ce928f2429..61885f020c 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -541,16 +541,16 @@ void virtio_address_space_write(VirtIOPCIProxy *pro= xy, > hwaddr addr, > val =3D pci_get_byte(buf); > break; > case 2: > - val =3D cpu_to_le16(pci_get_word(buf)); > + val =3D pci_get_word(buf); > break; > case 4: > - val =3D cpu_to_le32(pci_get_long(buf)); > + val =3D pci_get_long(buf); > break; > default: > /* As length is under guest control, handle illegal values. */ > return; > } > - memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIF= IED); > + memory_region_dispatch_write(mr, addr, val, size_memop(len) & ~MO_BS= WAP, > MEMTXATTRS_UNSPECIFIED); > } > > static void Sorry Paolo, I noted the need to take care in virtio_address_space_write an= d zpci_write_bar but did not understand. > Some care is needed in virtio_address_space_write and zpci_write_bar. Is this advice for my v1 implementation, or in the case of the MO_TE | MO_{8,16,32,64} idead suggested in the paragraph before? Signed-off-by: Tony Nguyen --- hw/virtio/virtio-pci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ce928f2..265f066 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -17,6 +17,7 @@ #include "qemu/osdep.h" +#include "exec/memop.h" #include "standard-headers/linux/virtio_pci.h" #include "hw/virtio/virtio.h" #include "hw/pci/pci.h" @@ -550,7 +551,8 @@ void virtio_address_space_write(VirtIOPCIProxy *proxy, = hwaddr addr, /* As length is under guest control, handle illegal values. */ return; } - memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIE= D); + memory_region_dispatch_write(mr, addr, val, SIZE_MEMOP(len), + MEMTXATTRS_UNSPECIFIED); } static void @@ -573,7 +575,8 @@ virtio_address_space_read(VirtIOPCIProxy *proxy, hwaddr= addr, /* Make sure caller aligned buf properly */ assert(!(((uintptr_t)buf) & (len - 1))); - memory_region_dispatch_read(mr, addr, &val, len, MEMTXATTRS_UNSPECIFIE= D); + memory_region_dispatch_read(mr, addr, &val, SIZE_MEMOP(len), + MEMTXATTRS_UNSPECIFIED); switch (len) { case 1: pci_set_byte(buf, val); -- 1.8.3.1