subject:"\[Qemu\-devel\] \[PATCH v3 07\/30\] imx_fec\: Add support for multiple Tx DMA rings"

[Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

2017-11-06 Thread Andrey Smirnov

More recent version of the IP block support more than one Tx DMA ring,
so add the code implementing that feature.

Cc: Peter Maydell 
Cc: Jason Wang 
Cc: Philippe Mathieu-Daudé 
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: yurov...@gmail.com
Signed-off-by: Andrey Smirnov 
---
 hw/net/imx_fec.c | 106 ++-
 include/hw/net/imx_fec.h |  18 +++-
 2 files changed, 102 insertions(+), 22 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 131e7fd734..38d8c27dcd 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -198,13 +198,13 @@ static const char *imx_eth_reg_name(IMXFECState *s, 
uint32_t index)
 
 static const VMStateDescription vmstate_imx_eth = {
 .name = TYPE_IMX_FEC,
-.version_id = 2,
-.minimum_version_id = 2,
+.version_id = 3,
+.minimum_version_id = 3,
 .fields = (VMStateField[]) {
 VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
 VMSTATE_UINT32(rx_descriptor, IMXFECState),
-VMSTATE_UINT32(tx_descriptor, IMXFECState),
-
+VMSTATE_UINT32_ARRAY(tx_descriptor, IMXFECState, ENET_TX_RING_NUM),
+VMSTATE_UINT32(tx_ring_num, IMXFECState),
 VMSTATE_UINT32(phy_status, IMXFECState),
 VMSTATE_UINT32(phy_control, IMXFECState),
 VMSTATE_UINT32(phy_advertise, IMXFECState),
@@ -407,7 +407,7 @@ static void imx_fec_do_tx(IMXFECState *s)
 int frame_size = 0, descnt = 0;
 uint8_t frame[ENET_MAX_FRAME_SIZE];
 uint8_t *ptr = frame;
-uint32_t addr = s->tx_descriptor;
+uint32_t addr = s->tx_descriptor[0];
 
 while (descnt++ < IMX_MAX_DESC) {
 IMXFECBufDesc bd;
@@ -448,17 +448,47 @@ static void imx_fec_do_tx(IMXFECState *s)
 }
 }
 
-s->tx_descriptor = addr;
+s->tx_descriptor[0] = addr;
 
 imx_eth_update(s);
 }
 
-static void imx_enet_do_tx(IMXFECState *s)
+static void imx_enet_do_tx(IMXFECState *s, uint32_t index)
 {
 int frame_size = 0, descnt = 0;
 uint8_t frame[ENET_MAX_FRAME_SIZE];
 uint8_t *ptr = frame;
-uint32_t addr = s->tx_descriptor;
+uint32_t addr, int_txb, int_txf, tdsr;
+size_t ring;
+
+switch (index) {
+case ENET_TDAR:
+ring= 0;
+int_txb = ENET_INT_TXB;
+int_txf = ENET_INT_TXF;
+tdsr= ENET_TDSR;
+break;
+case ENET_TDAR1:
+ring= 1;
+int_txb = ENET_INT_TXB1;
+int_txf = ENET_INT_TXF1;
+tdsr= ENET_TDSR1;
+break;
+case ENET_TDAR2:
+ring= 2;
+int_txb = ENET_INT_TXB2;
+int_txf = ENET_INT_TXF2;
+tdsr= ENET_TDSR2;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: bogus value for index %x\n",
+  __func__, index);
+abort();
+break;
+}
+
+addr = s->tx_descriptor[ring];
 
 while (descnt++ < IMX_MAX_DESC) {
 IMXENETBufDesc bd;
@@ -502,32 +532,32 @@ static void imx_enet_do_tx(IMXFECState *s)
 ptr = frame;
 frame_size = 0;
 if (bd.option & ENET_BD_TX_INT) {
-s->regs[ENET_EIR] |= ENET_INT_TXF;
+s->regs[ENET_EIR] |= int_txf;
 }
 }
 if (bd.option & ENET_BD_TX_INT) {
-s->regs[ENET_EIR] |= ENET_INT_TXB;
+s->regs[ENET_EIR] |= int_txb;
 }
 bd.flags &= ~ENET_BD_R;
 /* Write back the modified descriptor.  */
 imx_enet_write_bd(&bd, addr);
 /* Advance to the next descriptor.  */
 if ((bd.flags & ENET_BD_W) != 0) {
-addr = s->regs[ENET_TDSR];
+addr = s->regs[tdsr];
 } else {
 addr += sizeof(bd);
 }
 }
 
-s->tx_descriptor = addr;
+s->tx_descriptor[ring] = addr;
 
 imx_eth_update(s);
 }
 
-static void imx_eth_do_tx(IMXFECState *s)
+static void imx_eth_do_tx(IMXFECState *s, uint32_t index)
 {
 if (!s->is_fec && (s->regs[ENET_ECR] & ENET_ECR_EN1588)) {
-imx_enet_do_tx(s);
+imx_enet_do_tx(s, index);
 } else {
 imx_fec_do_tx(s);
 }
@@ -585,7 +615,7 @@ static void imx_eth_reset(DeviceState *d)
 }
 
 s->rx_descriptor = 0;
-s->tx_descriptor = 0;
+memset(s->tx_descriptor, 0, sizeof(s->tx_descriptor));
 
 /* We also reset the PHY */
 phy_reset(s);
@@ -791,6 +821,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, 
uint64_t value,
unsigned size)
 {
 IMXFECState *s = IMX_FEC(opaque);
+const bool single_tx_ring = s->tx_ring_num != 3;
 uint32_t index = offset >> 2;
 
 FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_eth_reg_name(s, index),
@@ -813,10 +844,18 @@ static void imx_eth_write(void *opaque, hwaddr offset, 
uint64_t value,
 s->regs[index] = 0;
 }
 break;
-case ENET_TDAR:
+case ENET_TDAR1:/* FALLTHROUGH */
+case ENET_TDAR2:/* FALLTHROUGH */
+if (unlikely(si

Re: [Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

2017-11-21 Thread Peter Maydell

On 6 November 2017 at 15:47, Andrey Smirnov  wrote:
> More recent version of the IP block support more than one Tx DMA ring,
> so add the code implementing that feature.
>
> Cc: Peter Maydell 
> Cc: Jason Wang 
> Cc: Philippe Mathieu-Daudé 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: yurov...@gmail.com
> Signed-off-by: Andrey Smirnov 

>  static const VMStateDescription vmstate_imx_eth = {
>  .name = TYPE_IMX_FEC,
> -.version_id = 2,
> -.minimum_version_id = 2,
> +.version_id = 3,
> +.minimum_version_id = 3,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
>  VMSTATE_UINT32(rx_descriptor, IMXFECState),
> -VMSTATE_UINT32(tx_descriptor, IMXFECState),
> -
> +VMSTATE_UINT32_ARRAY(tx_descriptor, IMXFECState, ENET_TX_RING_NUM),
> +VMSTATE_UINT32(tx_ring_num, IMXFECState),
>  VMSTATE_UINT32(phy_status, IMXFECState),
>  VMSTATE_UINT32(phy_control, IMXFECState),
>  VMSTATE_UINT32(phy_advertise, IMXFECState),

tx_ring_num is constant for any particular instantiation of the device,
so you don't need to put it in the vmstate.

It's pretty trivial to make this vmstate compatible with the old
ones for the existing single-tx-descriptor devices, so we might as well:

/* Versions of this device with more than one TX descriptor
 * save the 2nd and 3rd descriptors in a subsection, to maintain
 * migration compatibility with previous versions of the device
 * that only supported a single descriptor.
 */
static bool txdescs_needed(void *opaque) {
IMXFECState *s = opaque;

return s->tx_ring_num > 1;
}

static const VMStateDescription vmstate_imx_eth_txdescs = {
.name = "imx.fec/txdescs",
.version_id = 1,
.minimum_version_id = 1,
.needed = txdescs_needed,
.fields = (VMStateField[]) {
 VMSTATE_UINT32(tx_descriptor[1], IMXFECState),
 VMSTATE_UINT32(tx_descriptor[2], IMXFECState),
 VMSTATE_END_OF_LIST()
}
};

and then add this to the vmx_state_eth at the end:
.subsections = (const VMStateDescription*[]) {
 &vmstate_imx_eth_txdescs,
 NULL
}

Then you don't need to bump version_id/minimum_version_id on the
vmstate_imx_eth struct.

> @@ -791,6 +821,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, 
> uint64_t value,
> unsigned size)
>  {
>  IMXFECState *s = IMX_FEC(opaque);
> +const bool single_tx_ring = s->tx_ring_num != 3;

This looks odd -- I would have expected "single_tx_ring =
s->tx_ring_num == 1" ...

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

2017-11-22 Thread Andrey Smirnov

On Tue, Nov 21, 2017 at 9:44 AM, Peter Maydell  wrote:
> On 6 November 2017 at 15:47, Andrey Smirnov  wrote:
>> More recent version of the IP block support more than one Tx DMA ring,
>> so add the code implementing that feature.
>>
>> Cc: Peter Maydell 
>> Cc: Jason Wang 
>> Cc: Philippe Mathieu-Daudé 
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-...@nongnu.org
>> Cc: yurov...@gmail.com
>> Signed-off-by: Andrey Smirnov 
>
>>  static const VMStateDescription vmstate_imx_eth = {
>>  .name = TYPE_IMX_FEC,
>> -.version_id = 2,
>> -.minimum_version_id = 2,
>> +.version_id = 3,
>> +.minimum_version_id = 3,
>>  .fields = (VMStateField[]) {
>>  VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
>>  VMSTATE_UINT32(rx_descriptor, IMXFECState),
>> -VMSTATE_UINT32(tx_descriptor, IMXFECState),
>> -
>> +VMSTATE_UINT32_ARRAY(tx_descriptor, IMXFECState, ENET_TX_RING_NUM),
>> +VMSTATE_UINT32(tx_ring_num, IMXFECState),
>>  VMSTATE_UINT32(phy_status, IMXFECState),
>>  VMSTATE_UINT32(phy_control, IMXFECState),
>>  VMSTATE_UINT32(phy_advertise, IMXFECState),
>
> tx_ring_num is constant for any particular instantiation of the device,
> so you don't need to put it in the vmstate.
>
> It's pretty trivial to make this vmstate compatible with the old
> ones for the existing single-tx-descriptor devices, so we might as well:
>
> /* Versions of this device with more than one TX descriptor
>  * save the 2nd and 3rd descriptors in a subsection, to maintain
>  * migration compatibility with previous versions of the device
>  * that only supported a single descriptor.
>  */
> static bool txdescs_needed(void *opaque) {
> IMXFECState *s = opaque;
>
> return s->tx_ring_num > 1;
> }
>
> static const VMStateDescription vmstate_imx_eth_txdescs = {
> .name = "imx.fec/txdescs",
> .version_id = 1,
> .minimum_version_id = 1,
> .needed = txdescs_needed,
> .fields = (VMStateField[]) {
>  VMSTATE_UINT32(tx_descriptor[1], IMXFECState),
>  VMSTATE_UINT32(tx_descriptor[2], IMXFECState),
>  VMSTATE_END_OF_LIST()
> }
> };
>
> and then add this to the vmx_state_eth at the end:
> .subsections = (const VMStateDescription*[]) {
>  &vmstate_imx_eth_txdescs,
>  NULL
> }
>
> Then you don't need to bump version_id/minimum_version_id on the
> vmstate_imx_eth struct.
>

Cool, sounds good. Will add that to the patch in v4.

>> @@ -791,6 +821,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, 
>> uint64_t value,
>> unsigned size)
>>  {
>>  IMXFECState *s = IMX_FEC(opaque);
>> +const bool single_tx_ring = s->tx_ring_num != 3;
>
> This looks odd -- I would have expected "single_tx_ring =
> s->tx_ring_num == 1" ...

AFAIK the HW that's out there will have either 3 or 1 Tx ring, so
that's why I wrote it this way. I'll change the logic to the way your
suggest to avoid surprising the reader.

Thanks,
Andrey Smirnov

[Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

Re: [Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

Re: [Qemu-devel] [PATCH v3 07/30] imx_fec: Add support for multiple Tx DMA rings

3 matches

Site Navigation

Mail list logo

Footer information