[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 14/25] qemu/bswap: Introduce load/store for aligned point
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC PATCH 14/25] qemu/bswap: Introduce load/store for aligned pointer |
Date: |
Wed, 19 May 2021 10:08:55 +0100 |
On Wed, May 19, 2021 at 07:56:51AM +0200, Philippe Mathieu-Daudé wrote:
> On 5/18/21 10:15 PM, Peter Maydell wrote:
> > On Tue, 18 May 2021 at 19:38, Philippe Mathieu-Daudé <philmd@redhat.com>
> > wrote:
> >>
> >> When the pointer alignment is known to be safe, we can
> >> directly swap the data in place, without having to rely
> >> on the compiler builtin code.
> >>
> >> Load/store methods expecting aligned pointer use the 'a'
> >> infix. For example to read a 16-bit unsigned value stored
> >> in little endianess at an unaligned pointer:
> >>
> >> val = lduw_le_p(&unaligned_ptr);
> >>
> >> then to store it in big endianess at an aligned pointer:
> >>
> >> stw_be_ap(&aligned_ptr, val);
> >
> > It sounded from the bug report as if the desired effect
> > was "this access is atomic". Nothing in the documentation here
> > makes that guarantee of the implementation -- it merely imposes
> > an extra requirement on the caller that the pointer alignment
> > is "safe" (which term it does not define...) and a valid
> > implementation would be to implement the "aligned" versions
> > identically to the "unaligned" versions...
> >
> > Q: should the functions at the bottom of this stack of APIs
> > be using something from the atomic.h header? If not, why not?
> > Do we need any of the other atomic primitives ?
>
> I'll defer this question to Stefan/Paolo...
Stricly speaking qatomic_read() and qatomic_set() are necessary for
__ATOMIC_RELAXED semantics.
The comment in atomic.h mentions this generally has no effect on
generated code. That's probably because aligned 16/32/64-bit accesses
don't tear on modern CPUs so no special instructions are needed. This is
why not using atomic.h usually works.
Even qatomic_read()/qatomic_set() is too strong since the doc comment
mentions it prevents the compiler moving other loads/stores past the
atomic operation. The hw/virtio/ code already has explicit memory
barriers and doesn't need the additional compiler barrier.
For safety I suggest using qatomic_read()/qatomic_set().
Stefan
signature.asc
Description: PGP signature
- Re: [RFC PATCH 08/25] qemu/bswap: Use ST_CONVERT() macro to emit 16-bit load/store functions, (continued)
- [RFC PATCH 07/25] qemu/bswap: Introduce ST_CONVERT() macro, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 09/25] qemu/bswap: Introduce LD_CONVERT() macro, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 10/25] qemu/bswap: Use LD_CONVERT macro to emit 16-bit signed load/store code, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 11/25] qemu/bswap: Use LD_CONVERT macro to emit 16-bit unsigned load/store code, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 12/25] qemu/bswap: Use LDST_CONVERT macro to emit 32-bit load/store functions, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 13/25] qemu/bswap: Use LDST_CONVERT macro to emit 64-bit load/store functions, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 14/25] qemu/bswap: Introduce load/store for aligned pointer, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 16/25] exec/memory: Add methods for aligned pointer access (physical memory), Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 15/25] exec/memory: Add methods for aligned pointer access (address space), Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 17/25] hw/virtio: Use correct type sizes, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 18/25] hw/virtio: Introduce VIRTIO_LD_CONVERT() macro, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 19/25] hw/virtio: Use LD_CONVERT macro to emit 16-bit unsigned load/store code, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 20/25] hw/virtio: Introduce VIRTIO_ST_CONVERT() macro, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 21/25] hw/virtio: Use ST_CONVERT() macro to emit 16-bit load/store functions, Philippe Mathieu-Daudé, 2021/05/18
- [RFC PATCH 22/25] hw/virtio: Use LDST_CONVERT macro to emit 32-bit load/store functions, Philippe Mathieu-Daudé, 2021/05/18