| Message ID | 20230424165053.1428857-3-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-sun8i-emac we just read directly from guest memory into > a host FrameDescriptor struct and back. This only works on > little-endian hosts. Reading and writing of descriptors is already > abstracted into functions; make those functions also handle the > byte-swapping so that TransferDescriptor structs as seen by the rest > of the code are always in host-order, and fix two places that were > doing ad-hoc descriptor reading without using the functions. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/net/allwinner-sun8i-emac.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c > index b861d8ff352..fac4405f452 100644 > --- a/hw/net/allwinner-sun8i-emac.c > +++ b/hw/net/allwinner-sun8i-emac.c > @@ -350,8 +350,13 @@ static void allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s, > FrameDescriptor *desc, > uint32_t phys_addr) > { > - dma_memory_read(&s->dma_as, phys_addr, desc, sizeof(*desc), > + uint32_t desc_words[4]; > + dma_memory_read(&s->dma_as, phys_addr, &desc_words, sizeof(desc_words), > MEMTXATTRS_UNSPECIFIED); > + desc->status = le32_to_cpu(desc_words[0]); > + desc->status2 = le32_to_cpu(desc_words[1]); > + desc->addr = le32_to_cpu(desc_words[2]); > + desc->next = le32_to_cpu(desc_words[3]); > } > > static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s, > @@ -400,10 +405,15 @@ static uint32_t allwinner_sun8i_emac_tx_desc(AwSun8iEmacState *s, > } > > static void allwinner_sun8i_emac_flush_desc(AwSun8iEmacState *s, > - FrameDescriptor *desc, > + const FrameDescriptor *desc, > uint32_t phys_addr) > { > - dma_memory_write(&s->dma_as, phys_addr, desc, sizeof(*desc), > + uint32_t desc_words[4]; > + desc_words[0] = cpu_to_le32(desc->status); > + desc_words[1] = cpu_to_le32(desc->status2); > + desc_words[2] = cpu_to_le32(desc->addr); > + desc_words[3] = cpu_to_le32(desc->next); > + dma_memory_write(&s->dma_as, phys_addr, &desc_words, sizeof(desc_words), > MEMTXATTRS_UNSPECIFIED); > } > > @@ -638,8 +648,7 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, > break; > case REG_TX_CUR_BUF: /* Transmit Current Buffer */ > if (s->tx_desc_curr != 0) { > - dma_memory_read(&s->dma_as, s->tx_desc_curr, &desc, sizeof(desc), > - MEMTXATTRS_UNSPECIFIED); > + allwinner_sun8i_emac_get_desc(s, &desc, s->tx_desc_curr); > value = desc.addr; > } else { > value = 0; > @@ -652,8 +661,7 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, > break; > case REG_RX_CUR_BUF: /* Receive Current Buffer */ > if (s->rx_desc_curr != 0) { > - dma_memory_read(&s->dma_as, s->rx_desc_curr, &desc, sizeof(desc), > - MEMTXATTRS_UNSPECIFIED); > + allwinner_sun8i_emac_get_desc(s, &desc, s->rx_desc_curr); > value = desc.addr; > } else { > value = 0; Reviewed-by: Thomas Huth <thuth@redhat.com>
Peter Maydell <peter.maydell@linaro.org> writes: > In allwinner-sun8i-emac we just read directly from guest memory into > a host FrameDescriptor struct and back. This only works on > little-endian hosts. Reading and writing of descriptors is already > abstracted into functions; make those functions also handle the > byte-swapping so that TransferDescriptor structs as seen by the rest > of the code are always in host-order, and fix two places that were > doing ad-hoc descriptor reading without using the functions. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 24/4/23 18:50, Peter Maydell wrote: > In allwinner-sun8i-emac we just read directly from guest memory into > a host FrameDescriptor struct and back. This only works on > little-endian hosts. Reading and writing of descriptors is already > abstracted into functions; make those functions also handle the > byte-swapping so that TransferDescriptor structs as seen by the rest > of the code are always in host-order, and fix two places that were > doing ad-hoc descriptor reading without using the functions. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/net/allwinner-sun8i-emac.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c index b861d8ff352..fac4405f452 100644 --- a/hw/net/allwinner-sun8i-emac.c +++ b/hw/net/allwinner-sun8i-emac.c @@ -350,8 +350,13 @@ static void allwinner_sun8i_emac_get_desc(AwSun8iEmacState *s, FrameDescriptor *desc, uint32_t phys_addr) { - dma_memory_read(&s->dma_as, phys_addr, desc, sizeof(*desc), + uint32_t desc_words[4]; + dma_memory_read(&s->dma_as, phys_addr, &desc_words, sizeof(desc_words), MEMTXATTRS_UNSPECIFIED); + desc->status = le32_to_cpu(desc_words[0]); + desc->status2 = le32_to_cpu(desc_words[1]); + desc->addr = le32_to_cpu(desc_words[2]); + desc->next = le32_to_cpu(desc_words[3]); } static uint32_t allwinner_sun8i_emac_next_desc(AwSun8iEmacState *s, @@ -400,10 +405,15 @@ static uint32_t allwinner_sun8i_emac_tx_desc(AwSun8iEmacState *s, } static void allwinner_sun8i_emac_flush_desc(AwSun8iEmacState *s, - FrameDescriptor *desc, + const FrameDescriptor *desc, uint32_t phys_addr) { - dma_memory_write(&s->dma_as, phys_addr, desc, sizeof(*desc), + uint32_t desc_words[4]; + desc_words[0] = cpu_to_le32(desc->status); + desc_words[1] = cpu_to_le32(desc->status2); + desc_words[2] = cpu_to_le32(desc->addr); + desc_words[3] = cpu_to_le32(desc->next); + dma_memory_write(&s->dma_as, phys_addr, &desc_words, sizeof(desc_words), MEMTXATTRS_UNSPECIFIED); } @@ -638,8 +648,7 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, break; case REG_TX_CUR_BUF: /* Transmit Current Buffer */ if (s->tx_desc_curr != 0) { - dma_memory_read(&s->dma_as, s->tx_desc_curr, &desc, sizeof(desc), - MEMTXATTRS_UNSPECIFIED); + allwinner_sun8i_emac_get_desc(s, &desc, s->tx_desc_curr); value = desc.addr; } else { value = 0; @@ -652,8 +661,7 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset, break; case REG_RX_CUR_BUF: /* Receive Current Buffer */ if (s->rx_desc_curr != 0) { - dma_memory_read(&s->dma_as, s->rx_desc_curr, &desc, sizeof(desc), - MEMTXATTRS_UNSPECIFIED); + allwinner_sun8i_emac_get_desc(s, &desc, s->rx_desc_curr); value = desc.addr; } else { value = 0;
In allwinner-sun8i-emac we just read directly from guest memory into a host FrameDescriptor struct and back. This only works on little-endian hosts. Reading and writing of descriptors is already abstracted into functions; make those functions also handle the byte-swapping so that TransferDescriptor structs as seen by the rest of the code are always in host-order, and fix two places that were doing ad-hoc descriptor reading without using the functions. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/allwinner-sun8i-emac.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)