Hi Peter,
On 7/17/23 15:26, Peter Maydell wrote:
> The implementation of the SMMUv3 has multiple places where it reads a
> data structure from the guest and directly operates on it without
> doing a guest-to-host endianness conversion. Since all SMMU data
> structures are little-endian, this means that the SMMU doesn't work
> on a big-endian host. In particular, this causes the Avocado test
> machine_aarch64_virt.py:Aarch64VirtMachine.test_alpine_virt_tcg_gic_max
> to fail on an s390x host.
>
> Add appropriate byte-swapping on reads and writes of guest in-memory
> data structures so that the device works correctly on big-endian
> hosts.
>
> As part of this we constrain queue_read() to operate only on Cmd
> structs and queue_write() on Evt structs, because in practice these
> are the only data structures the two functions are used with, and we
> need to know what the data structure is to be able to byte-swap its
> parts correctly.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> Cc: qemu-sta...@nongnu.org
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Thanks
Eric
> ---
> hw/arm/smmu-common.c | 3 +--
> hw/arm/smmuv3.c | 39 +++++++++++++++++++++++++++++++--------
> 2 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 5ab9d45d58a..f35ae9aa22c 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -216,8 +216,7 @@ static int get_pte(dma_addr_t baseaddr, uint32_t index,
> uint64_t *pte,
> dma_addr_t addr = baseaddr + index * sizeof(*pte);
>
> /* TODO: guarantee 64-bit single-copy atomicity */
> - ret = dma_memory_read(&address_space_memory, addr, pte, sizeof(*pte),
> - MEMTXATTRS_UNSPECIFIED);
> + ret = ldq_le_dma(&address_space_memory, addr, pte,
> MEMTXATTRS_UNSPECIFIED);
>
> if (ret != MEMTX_OK) {
> info->type = SMMU_PTW_ERR_WALK_EABT;
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 932f0096974..1e9be8e89af 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -102,20 +102,34 @@ static void smmuv3_write_gerrorn(SMMUv3State *s,
> uint32_t new_gerrorn)
> trace_smmuv3_write_gerrorn(toggled & pending, s->gerrorn);
> }
>
> -static inline MemTxResult queue_read(SMMUQueue *q, void *data)
> +static inline MemTxResult queue_read(SMMUQueue *q, Cmd *cmd)
> {
> dma_addr_t addr = Q_CONS_ENTRY(q);
> + MemTxResult ret;
> + int i;
>
> - return dma_memory_read(&address_space_memory, addr, data, q->entry_size,
> - MEMTXATTRS_UNSPECIFIED);
> + ret = dma_memory_read(&address_space_memory, addr, cmd, sizeof(Cmd),
> + MEMTXATTRS_UNSPECIFIED);
> + if (ret != MEMTX_OK) {
> + return ret;
> + }
> + for (i = 0; i < ARRAY_SIZE(cmd->word); i++) {
> + le32_to_cpus(&cmd->word[i]);
> + }
> + return ret;
> }
>
> -static MemTxResult queue_write(SMMUQueue *q, void *data)
> +static MemTxResult queue_write(SMMUQueue *q, Evt *evt_in)
> {
> dma_addr_t addr = Q_PROD_ENTRY(q);
> MemTxResult ret;
> + Evt evt = *evt_in;
> + int i;
>
> - ret = dma_memory_write(&address_space_memory, addr, data, q->entry_size,
> + for (i = 0; i < ARRAY_SIZE(evt.word); i++) {
> + cpu_to_le32s(&evt.word[i]);
> + }
> + ret = dma_memory_write(&address_space_memory, addr, &evt, sizeof(Evt),
> MEMTXATTRS_UNSPECIFIED);
> if (ret != MEMTX_OK) {
> return ret;
> @@ -298,7 +312,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
> static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
> SMMUEventInfo *event)
> {
> - int ret;
> + int ret, i;
>
> trace_smmuv3_get_ste(addr);
> /* TODO: guarantee 64-bit single-copy atomicity */
> @@ -311,6 +325,9 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr,
> STE *buf,
> event->u.f_ste_fetch.addr = addr;
> return -EINVAL;
> }
> + for (i = 0; i < ARRAY_SIZE(buf->word); i++) {
> + le32_to_cpus(&buf->word[i]);
> + }
> return 0;
>
> }
> @@ -320,7 +337,7 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t
> ssid,
> CD *buf, SMMUEventInfo *event)
> {
> dma_addr_t addr = STE_CTXPTR(ste);
> - int ret;
> + int ret, i;
>
> trace_smmuv3_get_cd(addr);
> /* TODO: guarantee 64-bit single-copy atomicity */
> @@ -333,6 +350,9 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t
> ssid,
> event->u.f_ste_fetch.addr = addr;
> return -EINVAL;
> }
> + for (i = 0; i < ARRAY_SIZE(buf->word); i++) {
> + le32_to_cpus(&buf->word[i]);
> + }
> return 0;
> }
>
> @@ -569,7 +589,7 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid,
> STE *ste,
> return -EINVAL;
> }
> if (s->features & SMMU_FEATURE_2LVL_STE) {
> - int l1_ste_offset, l2_ste_offset, max_l2_ste, span;
> + int l1_ste_offset, l2_ste_offset, max_l2_ste, span, i;
> dma_addr_t l1ptr, l2ptr;
> STEDesc l1std;
>
> @@ -593,6 +613,9 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid,
> STE *ste,
> event->u.f_ste_fetch.addr = l1ptr;
> return -EINVAL;
> }
> + for (i = 0; i < ARRAY_SIZE(l1std.word); i++) {
> + le32_to_cpus(&l1std.word[i]);
> + }
>
> span = L1STD_SPAN(&l1std);
>
