On Wed, Jun 01, 2022 at 05:51:36PM +0100, Andre Przywara wrote:
> The handlers for accessing the virtio-mmio header tried to be very
> clever, by modelling the internal data structure to look exactly like
> the protocol header, so that address offsets can "reused".
>
> This requires using a packed structure, which creates other problems,
> and seems to be totally unnecessary in this case.
>
> Replace the offset-based access hacks to the structure with proper
> compiler visible accesses, to avoid unaligned accesses and make the code
> more robust.
>
> This fixes UBSAN complaints about unaligned accesses.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
> include/kvm/virtio-mmio.h | 2 +-
> virtio/mmio.c | 19 +++++++++++++++----
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/include/kvm/virtio-mmio.h b/include/kvm/virtio-mmio.h
> index 13dcccb6..aa4cab3c 100644
> --- a/include/kvm/virtio-mmio.h
> +++ b/include/kvm/virtio-mmio.h
> @@ -39,7 +39,7 @@ struct virtio_mmio_hdr {
> u32 interrupt_ack;
> u32 reserved_5[2];
> u32 status;
> -} __attribute__((packed));
> +};
Does this mean that the previous patch is no longer required?
>
> struct virtio_mmio {
> u32 addr;
> diff --git a/virtio/mmio.c b/virtio/mmio.c
> index 3782d55a..c9ad8ee7 100644
> --- a/virtio/mmio.c
> +++ b/virtio/mmio.c
> @@ -135,12 +135,22 @@ static void virtio_mmio_config_in(struct kvm_cpu *vcpu,
>
> switch (addr) {
> case VIRTIO_MMIO_MAGIC_VALUE:
> + memcpy(data, &vmmio->hdr.magic, sizeof(vmmio->hdr.magic));
Hmm, this is a semantic change as we used to treat the magic as a u32 by
passing it to ioport__write32(), which would in turn do the swab for
big-endian machines.
I don't think we should be using raw memcpy() here.
Will
