Add a property "jumbo-max-len", which can be configured for jumbo frame size
up to 16,383 bytes, and also introduce new register GEM_JUMBO_MAX_LEN.
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
hw/net/cadence_gem.c | 21 +++++++++++++++++++--
include/hw/net/cadence_gem.h | 1 +
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 5ccec1a..45c50ab 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -61,6 +61,7 @@
#define GEM_TXPAUSE (0x0000003C/4) /* TX Pause Time reg */
#define GEM_TXPARTIALSF (0x00000040/4) /* TX Partial Store and Forward */
#define GEM_RXPARTIALSF (0x00000044/4) /* RX Partial Store and Forward */
+#define GEM_JUMBO_MAX_LEN (0x00000048 / 4) /* Max Jumbo Frame Size */
#define GEM_HASHLO (0x00000080/4) /* Hash Low address reg */
#define GEM_HASHHI (0x00000084/4) /* Hash High address reg */
#define GEM_SPADDR1LO (0x00000088/4) /* Specific addr 1 low reg */
@@ -314,7 +315,8 @@
#define GEM_MODID_VALUE 0x00020118
-#define MAX_FRAME_SIZE 2048
+#define MAX_JUMBO_FRAME_SIZE_MASK 0x3FFF
+#define MAX_FRAME_SIZE MAX_JUMBO_FRAME_SIZE_MASK
static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
{
@@ -1343,9 +1345,10 @@ static void gem_reset(DeviceState *d)
s->regs[GEM_RXPARTIALSF] = 0x000003ff;
s->regs[GEM_MODID] = s->revision;
s->regs[GEM_DESCONF] = 0x02500111;
- s->regs[GEM_DESCONF2] = 0x2ab13fff;
+ s->regs[GEM_DESCONF2] = 0x2ab10000 | s->jumbo_max_len;
s->regs[GEM_DESCONF5] = 0x002f2045;
s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
+ s->regs[GEM_JUMBO_MAX_LEN] = s->jumbo_max_len;
if (s->num_priority_queues > 1) {
queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
@@ -1420,6 +1423,9 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
DB_PRINT("lowering irqs on ISR read\n");
/* The interrupts get updated at the end of the function. */
break;
+ case GEM_JUMBO_MAX_LEN:
+ retval = s->jumbo_max_len;
+ break;
case GEM_PHYMNTNC:
if (retval & GEM_PHYMNTNC_OP_R) {
uint32_t phy_addr, reg_num;
@@ -1516,6 +1522,9 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
s->regs[GEM_IMR] &= ~val;
gem_update_int_status(s);
break;
+ case GEM_JUMBO_MAX_LEN:
+ s->jumbo_max_len = val & MAX_JUMBO_FRAME_SIZE_MASK;
+ break;
case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &= ~val;
gem_update_int_status(s);
@@ -1611,6 +1620,12 @@ static void gem_realize(DeviceState *dev, Error **errp)
s->nic = qemu_new_nic(&net_gem_info, &s->conf,
object_get_typename(OBJECT(dev)), dev->id, s);
+ if (s->jumbo_max_len > MAX_FRAME_SIZE) {
+ g_warning("jumbo-max-len is grater than %d",
+ MAX_FRAME_SIZE);
+ s->jumbo_max_len = MAX_FRAME_SIZE;
+ }
+
s->tx_packet = g_new0(uint8_t, MAX_FRAME_SIZE);
s->rx_packet = g_new0(uint8_t, MAX_FRAME_SIZE);
}
@@ -1670,6 +1685,8 @@ static Property gem_properties[] = {
num_type1_screeners, 4),
DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState,
num_type2_screeners, 4),
+ DEFINE_PROP_UINT16("jumbo-max-len", CadenceGEMState,
+ jumbo_max_len, 10240),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 8dbbaa3..ef85737 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -82,6 +82,7 @@ typedef struct CadenceGEMState {
uint8_t *tx_packet;
uint8_t *rx_packet;
+ uint16_t jumbo_max_len;
uint32_t rx_desc[MAX_PRIORITY_QUEUES][DESC_MAX_NUM_WORDS];
bool sar_active[4];
--
2.7.4
On Fri, May 08, 2020 at 04:30:41PM +0530, Sai Pavan Boddu wrote:
> Add a property "jumbo-max-len", which can be configured for jumbo frame size
> up to 16,383 bytes, and also introduce new register GEM_JUMBO_MAX_LEN.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
> hw/net/cadence_gem.c | 21 +++++++++++++++++++--
> include/hw/net/cadence_gem.h | 1 +
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 5ccec1a..45c50ab 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -61,6 +61,7 @@
> #define GEM_TXPAUSE (0x0000003C/4) /* TX Pause Time reg */
> #define GEM_TXPARTIALSF (0x00000040/4) /* TX Partial Store and Forward */
> #define GEM_RXPARTIALSF (0x00000044/4) /* RX Partial Store and Forward */
> +#define GEM_JUMBO_MAX_LEN (0x00000048 / 4) /* Max Jumbo Frame Size */
Would be nice to align this in the same way as all the others...
> #define GEM_HASHLO (0x00000080/4) /* Hash Low address reg */
> #define GEM_HASHHI (0x00000084/4) /* Hash High address reg */
> #define GEM_SPADDR1LO (0x00000088/4) /* Specific addr 1 low reg */
> @@ -314,7 +315,8 @@
>
> #define GEM_MODID_VALUE 0x00020118
>
> -#define MAX_FRAME_SIZE 2048
> +#define MAX_JUMBO_FRAME_SIZE_MASK 0x3FFF
> +#define MAX_FRAME_SIZE MAX_JUMBO_FRAME_SIZE_MASK
>
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
> {
> @@ -1343,9 +1345,10 @@ static void gem_reset(DeviceState *d)
> s->regs[GEM_RXPARTIALSF] = 0x000003ff;
> s->regs[GEM_MODID] = s->revision;
> s->regs[GEM_DESCONF] = 0x02500111;
> - s->regs[GEM_DESCONF2] = 0x2ab13fff;
> + s->regs[GEM_DESCONF2] = 0x2ab10000 | s->jumbo_max_len;
> s->regs[GEM_DESCONF5] = 0x002f2045;
> s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
> + s->regs[GEM_JUMBO_MAX_LEN] = s->jumbo_max_len;
>
> if (s->num_priority_queues > 1) {
> queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
> @@ -1420,6 +1423,9 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
> DB_PRINT("lowering irqs on ISR read\n");
> /* The interrupts get updated at the end of the function. */
> break;
> + case GEM_JUMBO_MAX_LEN:
> + retval = s->jumbo_max_len;
> + break;
> case GEM_PHYMNTNC:
> if (retval & GEM_PHYMNTNC_OP_R) {
> uint32_t phy_addr, reg_num;
> @@ -1516,6 +1522,9 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
> s->regs[GEM_IMR] &= ~val;
> gem_update_int_status(s);
> break;
> + case GEM_JUMBO_MAX_LEN:
> + s->jumbo_max_len = val & MAX_JUMBO_FRAME_SIZE_MASK;
I don't think writing to this register may increase the max len
beyond the max-len selected at design time (the property).
TBH I'm surprised this register is RW in the spec.
We may need two variables here, one for the design-time configured
max and another for the runtime configurable max.
> + break;
> case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
> s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &= ~val;
> gem_update_int_status(s);
> @@ -1611,6 +1620,12 @@ static void gem_realize(DeviceState *dev, Error **errp)
> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
> object_get_typename(OBJECT(dev)), dev->id, s);
>
> + if (s->jumbo_max_len > MAX_FRAME_SIZE) {
> + g_warning("jumbo-max-len is grater than %d",
You've got a typo here "grater".
I also think we could error out here if wrong values are chosen.
Best regards,
Edgar
Hi Edgar,
> -----Original Message-----
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Sent: Friday, May 8, 2020 5:14 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Alistair Francis <Alistair.Francis@wdc.com>; Peter Maydell
> <peter.maydell@linaro.org>; Jason Wang <jasowang@redhat.com>; Markus
> Armbruster <armbru@redhat.com>; Philippe Mathieu-Daudé
> <philmd@redhat.com>; Tong Ho <tongh@xilinx.com>; Ramon Fried
> <rfried.dev@gmail.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org
> Subject: Re: [PATCH v3 07/11] net: cadence_gem: Add support for jumbo
> frames
>
> On Fri, May 08, 2020 at 04:30:41PM +0530, Sai Pavan Boddu wrote:
> > Add a property "jumbo-max-len", which can be configured for jumbo
> > frame size up to 16,383 bytes, and also introduce new register
> GEM_JUMBO_MAX_LEN.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > ---
> > hw/net/cadence_gem.c | 21 +++++++++++++++++++--
> > include/hw/net/cadence_gem.h | 1 +
> > 2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > 5ccec1a..45c50ab 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -61,6 +61,7 @@
> > #define GEM_TXPAUSE (0x0000003C/4) /* TX Pause Time reg */
> > #define GEM_TXPARTIALSF (0x00000040/4) /* TX Partial Store and
> Forward */
> > #define GEM_RXPARTIALSF (0x00000044/4) /* RX Partial Store and
> Forward */
> > +#define GEM_JUMBO_MAX_LEN (0x00000048 / 4) /* Max Jumbo Frame
> Size */
>
> Would be nice to align this in the same way as all the others...
>
>
>
> > #define GEM_HASHLO (0x00000080/4) /* Hash Low address reg */
> > #define GEM_HASHHI (0x00000084/4) /* Hash High address reg */
> > #define GEM_SPADDR1LO (0x00000088/4) /* Specific addr 1 low reg */
> > @@ -314,7 +315,8 @@
> >
> > #define GEM_MODID_VALUE 0x00020118
> >
> > -#define MAX_FRAME_SIZE 2048
> > +#define MAX_JUMBO_FRAME_SIZE_MASK 0x3FFF #define
> MAX_FRAME_SIZE
> > +MAX_JUMBO_FRAME_SIZE_MASK
> >
> > static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s,
> > uint32_t *desc) { @@ -1343,9 +1345,10 @@ static void
> > gem_reset(DeviceState *d)
> > s->regs[GEM_RXPARTIALSF] = 0x000003ff;
> > s->regs[GEM_MODID] = s->revision;
> > s->regs[GEM_DESCONF] = 0x02500111;
> > - s->regs[GEM_DESCONF2] = 0x2ab13fff;
> > + s->regs[GEM_DESCONF2] = 0x2ab10000 | s->jumbo_max_len;
> > s->regs[GEM_DESCONF5] = 0x002f2045;
> > s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
> > + s->regs[GEM_JUMBO_MAX_LEN] = s->jumbo_max_len;
> >
> > if (s->num_priority_queues > 1) {
> > queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
> > @@ -1420,6 +1423,9 @@ static uint64_t gem_read(void *opaque, hwaddr
> offset, unsigned size)
> > DB_PRINT("lowering irqs on ISR read\n");
> > /* The interrupts get updated at the end of the function. */
> > break;
> > + case GEM_JUMBO_MAX_LEN:
> > + retval = s->jumbo_max_len;
> > + break;
> > case GEM_PHYMNTNC:
> > if (retval & GEM_PHYMNTNC_OP_R) {
> > uint32_t phy_addr, reg_num; @@ -1516,6 +1522,9 @@ static
> > void gem_write(void *opaque, hwaddr offset, uint64_t val,
> > s->regs[GEM_IMR] &= ~val;
> > gem_update_int_status(s);
> > break;
> > + case GEM_JUMBO_MAX_LEN:
> > + s->jumbo_max_len = val & MAX_JUMBO_FRAME_SIZE_MASK;
>
> I don't think writing to this register may increase the max len beyond the
> max-len selected at design time (the property).
> TBH I'm surprised this register is RW in the spec.
>
> We may need two variables here, one for the design-time configured max
> and another for the runtime configurable max.
[Sai Pavan Boddu] Better way is to, use new created property to set the reset value of this register. Which can be overwritten by guest on runtime by writing to regs[GEM_JUMBO_MAX_LEN].
I would add few checks, so that frames does not cross JUMBO max length configured.
Thanks,
Sai Pavan
>
>
> > + break;
> > case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
> > s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &=
> ~val;
> > gem_update_int_status(s);
> > @@ -1611,6 +1620,12 @@ static void gem_realize(DeviceState *dev, Error
> **errp)
> > s->nic = qemu_new_nic(&net_gem_info, &s->conf,
> > object_get_typename(OBJECT(dev)), dev->id,
> > s);
> >
> > + if (s->jumbo_max_len > MAX_FRAME_SIZE) {
> > + g_warning("jumbo-max-len is grater than %d",
>
>
> You've got a typo here "grater".
>
> I also think we could error out here if wrong values are chosen.
>
> Best regards,
> Edgar
© 2016 - 2026 Red Hat, Inc.