qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 07/11] net: cadence_gem: Add support for jumbo frames


From: Edgar E. Iglesias
Subject: Re: [PATCH v3 07/11] net: cadence_gem: Add support for jumbo frames
Date: Fri, 8 May 2020 13:43:47 +0200
User-agent: Mutt/1.10.1 (2018-07-13)

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 <address@hidden>
> ---
>  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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]