| Message ID | 20230424165053.1428857-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: allwinner: fix endianness bugs in sdhost and sun8i-emac | expand |
On 24/04/2023 18.50, Peter Maydell wrote: > In allwinner_sdhost_process_desc() we just read directly from > guest memory into a host TransferDescriptor struct and back. > This only works on little-endian hosts. Abstract the reading > and writing of descriptors into functions that handle the > byte-swapping so that TransferDescriptor structs as seen by > the rest of the code are always in host-order. > > This fixes a failure of one of the avocado tests on s390. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/sd/allwinner-sdhost.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c > index 51e5e908307..92a0f42708d 100644 > --- a/hw/sd/allwinner-sdhost.c > +++ b/hw/sd/allwinner-sdhost.c > @@ -302,6 +302,30 @@ static void allwinner_sdhost_auto_stop(AwSdHostState *s) > } > } > > +static void read_descriptor(AwSdHostState *s, hwaddr desc_addr, > + TransferDescriptor *desc) > +{ > + uint32_t desc_words[4]; > + dma_memory_read(&s->dma_as, desc_addr, &desc_words, sizeof(desc_words), > + MEMTXATTRS_UNSPECIFIED); > + desc->status = le32_to_cpu(desc_words[0]); > + desc->size = le32_to_cpu(desc_words[1]); > + desc->addr = le32_to_cpu(desc_words[2]); > + desc->next = le32_to_cpu(desc_words[3]); > +} > + > +static void write_descriptor(AwSdHostState *s, hwaddr desc_addr, > + const TransferDescriptor *desc) > +{ > + uint32_t desc_words[4]; > + desc_words[0] = cpu_to_le32(desc->status); > + desc_words[1] = cpu_to_le32(desc->size); > + desc_words[2] = cpu_to_le32(desc->addr); > + desc_words[3] = cpu_to_le32(desc->next); > + dma_memory_write(&s->dma_as, desc_addr, &desc_words, sizeof(desc_words), > + MEMTXATTRS_UNSPECIFIED); > +} > + > static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, > hwaddr desc_addr, > TransferDescriptor *desc, > @@ -312,9 +336,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, > uint32_t num_bytes = max_bytes; > uint8_t buf[1024]; > > - /* Read descriptor */ > - dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc), > - MEMTXATTRS_UNSPECIFIED); > + read_descriptor(s, desc_addr, desc); > if (desc->size == 0) { > desc->size = klass->max_desc_size; > } else if (desc->size > klass->max_desc_size) { > @@ -356,8 +378,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, > > /* Clear hold flag and flush descriptor */ > desc->status &= ~DESC_STATUS_HOLD; > - dma_memory_write(&s->dma_as, desc_addr, desc, sizeof(*desc), > - MEMTXATTRS_UNSPECIFIED); > + write_descriptor(s, desc_addr, desc); > > return num_done; > } Reviewed-by: Thomas Huth <thuth@redhat.com>
Peter Maydell <peter.maydell@linaro.org> writes: > In allwinner_sdhost_process_desc() we just read directly from > guest memory into a host TransferDescriptor struct and back. > This only works on little-endian hosts. Abstract the reading > and writing of descriptors into functions that handle the > byte-swapping so that TransferDescriptor structs as seen by > the rest of the code are always in host-order. I couldn't find any datasheets on the sdhost hardware but the kernel certainly explicitly sets the endianess of the descriptors so: Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 24/4/23 18:50, Peter Maydell wrote: > In allwinner_sdhost_process_desc() we just read directly from > guest memory into a host TransferDescriptor struct and back. > This only works on little-endian hosts. Abstract the reading > and writing of descriptors into functions that handle the > byte-swapping so that TransferDescriptor structs as seen by > the rest of the code are always in host-order. > > This fixes a failure of one of the avocado tests on s390. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/sd/allwinner-sdhost.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c index 51e5e908307..92a0f42708d 100644 --- a/hw/sd/allwinner-sdhost.c +++ b/hw/sd/allwinner-sdhost.c @@ -302,6 +302,30 @@ static void allwinner_sdhost_auto_stop(AwSdHostState *s) } } +static void read_descriptor(AwSdHostState *s, hwaddr desc_addr, + TransferDescriptor *desc) +{ + uint32_t desc_words[4]; + dma_memory_read(&s->dma_as, desc_addr, &desc_words, sizeof(desc_words), + MEMTXATTRS_UNSPECIFIED); + desc->status = le32_to_cpu(desc_words[0]); + desc->size = le32_to_cpu(desc_words[1]); + desc->addr = le32_to_cpu(desc_words[2]); + desc->next = le32_to_cpu(desc_words[3]); +} + +static void write_descriptor(AwSdHostState *s, hwaddr desc_addr, + const TransferDescriptor *desc) +{ + uint32_t desc_words[4]; + desc_words[0] = cpu_to_le32(desc->status); + desc_words[1] = cpu_to_le32(desc->size); + desc_words[2] = cpu_to_le32(desc->addr); + desc_words[3] = cpu_to_le32(desc->next); + dma_memory_write(&s->dma_as, desc_addr, &desc_words, sizeof(desc_words), + MEMTXATTRS_UNSPECIFIED); +} + static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, hwaddr desc_addr, TransferDescriptor *desc, @@ -312,9 +336,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, uint32_t num_bytes = max_bytes; uint8_t buf[1024]; - /* Read descriptor */ - dma_memory_read(&s->dma_as, desc_addr, desc, sizeof(*desc), - MEMTXATTRS_UNSPECIFIED); + read_descriptor(s, desc_addr, desc); if (desc->size == 0) { desc->size = klass->max_desc_size; } else if (desc->size > klass->max_desc_size) { @@ -356,8 +378,7 @@ static uint32_t allwinner_sdhost_process_desc(AwSdHostState *s, /* Clear hold flag and flush descriptor */ desc->status &= ~DESC_STATUS_HOLD; - dma_memory_write(&s->dma_as, desc_addr, desc, sizeof(*desc), - MEMTXATTRS_UNSPECIFIED); + write_descriptor(s, desc_addr, desc); return num_done; }
In allwinner_sdhost_process_desc() we just read directly from guest memory into a host TransferDescriptor struct and back. This only works on little-endian hosts. Abstract the reading and writing of descriptors into functions that handle the byte-swapping so that TransferDescriptor structs as seen by the rest of the code are always in host-order. This fixes a failure of one of the avocado tests on s390. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/sd/allwinner-sdhost.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-)