Re: [RFC PATCH 00/12] exec/memory: Experimental API to catch unaligned accesses

[Philippe Mathieu-Daudé]

Hi,

If there is any feedback, should I discard this experiment?

On 5/20/21 1:09 PM, Philippe Mathieu-Daudé wrote:
> This series is an experiment after chatting with Stefan and having
> received review from Peter / Richard on an orthogonal series aiming
> to handle unaligned pointers (atomically):
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg808954.html
> 
> Here we don't aim to fix unatomic accesses, however we are interested
> in catching malicious unaligned accesses from guests.
> 
> For that we introduce the MemTxAttrs::aligned field which allow
> accessors to return MEMTX_UNALIGNED_ERROR early enough, instead
> of trying the unaligned access which can potentially trigger a
> SIGBUS and abort the process.
> 
> To be able to pass/return alignment information we modify the
> memory load/store cached API, then add the
> virtio_ld/st*_phys_cached_with_attrs() handler (we only implement
> lduw for this experiment).
> 
> Finaly we modify vring_avail_flags() to return whether the guest
> requested an illegal (unaligned) memory access.
> 
> Note: The current virtio_ld/st*_phys_cached_with_attrs() API returns
> the value, and take the MemTxResult as argument, so I choose to
> return -1 (marked with /* XXX */ comment. We should switch to using
> an API which returns a MemTxResult and takes the value accessed as
> argument, this way we don't have to return random meaningless value.
> 
> But this is beyond the scope of this experiment, here we want to
> emphasize the introduction of the MemTxAttrs::aligned field and the
> MEMTX_UNALIGNED_ERROR return value.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (12):
>   exec/memory_ldst: Use correct type sizes
>   exec/memattrs: Add attribute/error for address alignment
>   exec/memory_ldst: Return MEMTX_UNALIGNED_ERROR for unaligned addresses
>   exec/memory_ldst_cached: Sort declarations
>   exec/memory_ldst_cached: Use correct type size
>   exec/memory_ldst_cached: Set MemTxResult on success
>   exec/memory_ldst_cached: Document aligned addresses are expected
>   exec/memory_ldst_cached: Check address alignment if requested
>   hw/virtio: Use correct type sizes
>   hw/virtio: Extract virtio_lduw_phys_cached_with_attrs()
>   hw/virtio: Have vring_avail_flags() return a boolean value
>   hw/virtio: Display error if vring flag field is not aligned
> 
>  include/exec/memattrs.h               |   3 +
>  include/hw/virtio/virtio-access.h     |  39 +++++++--
>  include/exec/memory_ldst.h.inc        |  16 ++--
>  include/exec/memory_ldst_cached.h.inc | 114 ++++++++++++++++++++------
>  hw/virtio/virtio.c                    |  22 ++++-
>  memory_ldst.c.inc                     |  69 +++++++++++++---
>  6 files changed, 211 insertions(+), 52 deletions(-)
>