From: Greg Kurz <groug@kaod.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
"Kevin Wolf" <kwolf@redhat.com>,
qemu-block@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Jason Wang" <jasowang@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
"Hanna Reitz" <hreitz@redhat.com>
Subject: Re: [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
Date: Tue, 13 Dec 2022 11:50:19 +0100 [thread overview]
Message-ID: <20221213115019.4adfc326@bahia> (raw)
In-Reply-To: <20221212230517.28872-8-philmd@linaro.org>
On Tue, 13 Dec 2022 00:05:14 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> Since the device endianness doesn't change at runtime,
> use the cached value instead of evaluating it on each call.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/virtio/virtio-access.h | 44 +++++++++++++++----------------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index 07aae69042..985f39fe16 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -43,7 +43,7 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
For x86, virtio_access_is_big_endian() expands to :
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
return false;
}
When I added these memory accessors, there was a strong requirement from MST
that x86 shouldn't have to pay for an out-of-line check when it is known that
everything is always little endian. Not sure exactly what you're trying to
achieve with VirtIODevice::access_is_big_endian but this shouldn't mess with
this fast path.
> return lduw_be_phys(dma_as, pa);
> }
> return lduw_le_phys(dma_as, pa);
> @@ -53,7 +53,7 @@ static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldl_be_phys(dma_as, pa);
> }
> return ldl_le_phys(dma_as, pa);
> @@ -63,7 +63,7 @@ static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldq_be_phys(dma_as, pa);
> }
> return ldq_le_phys(dma_as, pa);
> @@ -74,7 +74,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stw_be_phys(dma_as, pa, value);
> } else {
> stw_le_phys(dma_as, pa, value);
> @@ -86,7 +86,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stl_be_phys(dma_as, pa, value);
> } else {
> stl_le_phys(dma_as, pa, value);
> @@ -95,7 +95,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
>
> static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stw_be_p(ptr, v);
> } else {
> stw_le_p(ptr, v);
> @@ -104,7 +104,7 @@ static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
>
> static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stl_be_p(ptr, v);
> } else {
> stl_le_p(ptr, v);
> @@ -113,7 +113,7 @@ static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
>
> static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stq_be_p(ptr, v);
> } else {
> stq_le_p(ptr, v);
> @@ -122,7 +122,7 @@ static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
>
> static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return lduw_be_p(ptr);
> } else {
> return lduw_le_p(ptr);
> @@ -131,7 +131,7 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
>
> static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldl_be_p(ptr);
> } else {
> return ldl_le_p(ptr);
> @@ -140,7 +140,7 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
>
> static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldq_be_p(ptr);
> } else {
> return ldq_le_p(ptr);
> @@ -150,9 +150,9 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
> static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
> {
> #if HOST_BIG_ENDIAN
> - return virtio_access_is_big_endian(vdev) ? s : bswap16(s);
> + return vdev->access_is_big_endian ? s : bswap16(s);
> #else
> - return virtio_access_is_big_endian(vdev) ? bswap16(s) : s;
> + return vdev->access_is_big_endian ? bswap16(s) : s;
> #endif
> }
>
> @@ -160,7 +160,7 @@ static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
> MemoryRegionCache *cache,
> hwaddr pa)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return lduw_be_phys_cached(cache, pa);
> }
> return lduw_le_phys_cached(cache, pa);
> @@ -170,7 +170,7 @@ static inline uint32_t virtio_ldl_phys_cached(VirtIODevice *vdev,
> MemoryRegionCache *cache,
> hwaddr pa)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldl_be_phys_cached(cache, pa);
> }
> return ldl_le_phys_cached(cache, pa);
> @@ -180,7 +180,7 @@ static inline uint64_t virtio_ldq_phys_cached(VirtIODevice *vdev,
> MemoryRegionCache *cache,
> hwaddr pa)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> return ldq_be_phys_cached(cache, pa);
> }
> return ldq_le_phys_cached(cache, pa);
> @@ -190,7 +190,7 @@ static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
> MemoryRegionCache *cache,
> hwaddr pa, uint16_t value)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stw_be_phys_cached(cache, pa, value);
> } else {
> stw_le_phys_cached(cache, pa, value);
> @@ -201,7 +201,7 @@ static inline void virtio_stl_phys_cached(VirtIODevice *vdev,
> MemoryRegionCache *cache,
> hwaddr pa, uint32_t value)
> {
> - if (virtio_access_is_big_endian(vdev)) {
> + if (vdev->access_is_big_endian) {
> stl_be_phys_cached(cache, pa, value);
> } else {
> stl_le_phys_cached(cache, pa, value);
> @@ -216,9 +216,9 @@ static inline void virtio_tswap16s(VirtIODevice *vdev, uint16_t *s)
> static inline uint32_t virtio_tswap32(VirtIODevice *vdev, uint32_t s)
> {
> #if HOST_BIG_ENDIAN
> - return virtio_access_is_big_endian(vdev) ? s : bswap32(s);
> + return vdev->access_is_big_endian ? s : bswap32(s);
> #else
> - return virtio_access_is_big_endian(vdev) ? bswap32(s) : s;
> + return vdev->access_is_big_endian ? bswap32(s) : s;
> #endif
> }
>
> @@ -230,9 +230,9 @@ static inline void virtio_tswap32s(VirtIODevice *vdev, uint32_t *s)
> static inline uint64_t virtio_tswap64(VirtIODevice *vdev, uint64_t s)
> {
> #if HOST_BIG_ENDIAN
> - return virtio_access_is_big_endian(vdev) ? s : bswap64(s);
> + return vdev->access_is_big_endian ? s : bswap64(s);
> #else
> - return virtio_access_is_big_endian(vdev) ? bswap64(s) : s;
> + return vdev->access_is_big_endian ? bswap64(s) : s;
> #endif
> }
>
next prev parent reply other threads:[~2022-12-13 10:51 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-12 23:05 [RFC PATCH-for-8.0 00/10] hw/virtio: Build most objects as target independent units Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 01/10] hw/virtio: Add missing "hw/core/cpu.h" include Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 02/10] hw/virtio: Rename virtio_ss[] -> specific_virtio_ss[] Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 03/10] hw/virtio: Constify qmp_virtio_feature_map_t[] Philippe Mathieu-Daudé
2022-12-13 0:02 ` Richard Henderson
2022-12-13 7:35 ` Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 04/10] hw/virtio: Extract config read/write accessors to virtio-config.c Philippe Mathieu-Daudé
2022-12-12 23:05 ` [PATCH-for-8.0 05/10] hw/virtio: Extract QMP related code virtio-qmp.c Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state Philippe Mathieu-Daudé
2022-12-13 0:14 ` Richard Henderson
2022-12-13 7:30 ` Philippe Mathieu-Daudé
2022-12-13 8:03 ` Thomas Huth
2022-12-13 8:32 ` Philippe Mathieu-Daudé
2022-12-13 8:47 ` Thomas Huth
2022-12-13 8:22 ` Philippe Mathieu-Daudé
2022-12-13 15:41 ` Richard Henderson
2022-12-12 23:05 ` [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian Philippe Mathieu-Daudé
2022-12-13 10:50 ` Greg Kurz [this message]
2022-12-12 23:05 ` [PATCH-for-8.0 08/10] hw/virtio: Un-inline virtio_access_is_big_endian() Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 09/10] hw/virtio: Extract vhost_user_ram_slots_max() to vhost-user-target.c Philippe Mathieu-Daudé
2025-04-10 12:14 ` Philippe Mathieu-Daudé
2025-04-10 14:36 ` Pierrick Bouvier
2025-04-10 17:21 ` Philippe Mathieu-Daudé
2025-04-10 17:29 ` Pierrick Bouvier
2025-04-11 10:15 ` Philippe Mathieu-Daudé
2022-12-12 23:05 ` [RFC PATCH-for-8.0 10/10] hw/virtio: Make most of virtio devices target-independent Philippe Mathieu-Daudé
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=20221213115019.4adfc326@bahia \
--to=groug@kaod.org \
--cc=alex.bennee@linaro.org \
--cc=hreitz@redhat.com \
--cc=jasowang@redhat.com \
--cc=kwolf@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu_oss@crudebyte.com \
--cc=stefanha@redhat.com \
--cc=thuth@redhat.com \
/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.