| Message ID | 20171214145255.31545-6-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andrey, On 12/14/2017 11:52 AM, Andrey Smirnov wrote: > Frame truncation length, TRUNC_FL, is determined by the contents of > ENET_FTRL register, so convert the code to use it instead of a > hardcoded constant. > > To avoid the case where TRUNC_FL is greater that ENET_MAX_FRAME_SIZE, > increase the value of the latter to its theoretical maximum of 16K. > > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> > Cc: qemu-devel@nongnu.org > Cc: qemu-arm@nongnu.org > Cc: yurovsky@gmail.com > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > --- > hw/net/imx_fec.c | 4 ++-- > include/hw/net/imx_fec.h | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c > index 56cb72273c..50da91bf9e 100644 > --- a/hw/net/imx_fec.c > +++ b/hw/net/imx_fec.c > @@ -1052,8 +1052,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, > crc_ptr = (uint8_t *) &crc; > > /* Huge frames are truncted. */ > - if (size > ENET_MAX_FRAME_SIZE) { > - size = ENET_MAX_FRAME_SIZE; > + if (size > s->regs[ENET_FTRL]) { Shouldn't this be: if (size > s->regs[ENET_FTRL] + 1) { > + size = s->regs[ENET_FTRL]; and: size = s->regs[ENET_FTRL] + 1; > flags |= ENET_BD_TR | ENET_BD_LG; > } > > diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h > index 67993870a2..a390d704a6 100644 > --- a/include/hw/net/imx_fec.h > +++ b/include/hw/net/imx_fec.h > @@ -86,7 +86,6 @@ > #define ENET_TCCR3 393 > #define ENET_MAX 400 > > -#define ENET_MAX_FRAME_SIZE 2032 > > /* EIR and EIMR */ > #define ENET_INT_HB (1 << 31) > @@ -155,6 +154,8 @@ > #define ENET_RCR_NLC (1 << 30) > #define ENET_RCR_GRS (1 << 31) > > +#define ENET_MAX_FRAME_SIZE (1 << ENET_RCR_MAX_FL_LENGTH) > + > /* TCR */ > #define ENET_TCR_GTS (1 << 0) > #define ENET_TCR_FDEN (1 << 2) >
On Fri, Dec 15, 2017 at 2:15 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Andrey, > > On 12/14/2017 11:52 AM, Andrey Smirnov wrote: >> Frame truncation length, TRUNC_FL, is determined by the contents of >> ENET_FTRL register, so convert the code to use it instead of a >> hardcoded constant. >> >> To avoid the case where TRUNC_FL is greater that ENET_MAX_FRAME_SIZE, >> increase the value of the latter to its theoretical maximum of 16K. >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Cc: qemu-devel@nongnu.org >> Cc: qemu-arm@nongnu.org >> Cc: yurovsky@gmail.com >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> >> --- >> hw/net/imx_fec.c | 4 ++-- >> include/hw/net/imx_fec.h | 3 ++- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c >> index 56cb72273c..50da91bf9e 100644 >> --- a/hw/net/imx_fec.c >> +++ b/hw/net/imx_fec.c >> @@ -1052,8 +1052,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, >> crc_ptr = (uint8_t *) &crc; >> >> /* Huge frames are truncted. */ >> - if (size > ENET_MAX_FRAME_SIZE) { >> - size = ENET_MAX_FRAME_SIZE; >> + if (size > s->regs[ENET_FTRL]) { > > Shouldn't this be: > > if (size > s->regs[ENET_FTRL] + 1) { > >> + size = s->regs[ENET_FTRL]; > > and: > > size = s->regs[ENET_FTRL] + 1; > I haven't tried to reproduce this in real HW and verify the behavior, so I can't say for a fact, but the datasheet seems to be pretty clear about "ENETx_FTRL": "Frame Truncation Length Indicates the value a receive frame is truncated, if it is greater than this value. Must be greater than or equal to RCR[MAX_FL]. NOTE: Truncation happens at TRUNC_FL. However, when truncation occurs, the application (FIFO) may receive less data, guaranteeing that it never receives more than the set limit." so, having nothing better to go on than above description, I think the original code is OK. >> flags |= ENET_BD_TR | ENET_BD_LG; >> } >> >> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h >> index 67993870a2..a390d704a6 100644 >> --- a/include/hw/net/imx_fec.h >> +++ b/include/hw/net/imx_fec.h >> @@ -86,7 +86,6 @@ >> #define ENET_TCCR3 393 >> #define ENET_MAX 400 >> >> -#define ENET_MAX_FRAME_SIZE 2032 >> >> /* EIR and EIMR */ >> #define ENET_INT_HB (1 << 31) >> @@ -155,6 +154,8 @@ >> #define ENET_RCR_NLC (1 << 30) >> #define ENET_RCR_GRS (1 << 31) >> >> +#define ENET_MAX_FRAME_SIZE (1 << ENET_RCR_MAX_FL_LENGTH) >> + >> /* TCR */ >> #define ENET_TCR_GTS (1 << 0) >> #define ENET_TCR_FDEN (1 << 2) >>
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index 56cb72273c..50da91bf9e 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -1052,8 +1052,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf, crc_ptr = (uint8_t *) &crc; /* Huge frames are truncted. */ - if (size > ENET_MAX_FRAME_SIZE) { - size = ENET_MAX_FRAME_SIZE; + if (size > s->regs[ENET_FTRL]) { + size = s->regs[ENET_FTRL]; flags |= ENET_BD_TR | ENET_BD_LG; } diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h index 67993870a2..a390d704a6 100644 --- a/include/hw/net/imx_fec.h +++ b/include/hw/net/imx_fec.h @@ -86,7 +86,6 @@ #define ENET_TCCR3 393 #define ENET_MAX 400 -#define ENET_MAX_FRAME_SIZE 2032 /* EIR and EIMR */ #define ENET_INT_HB (1 << 31) @@ -155,6 +154,8 @@ #define ENET_RCR_NLC (1 << 30) #define ENET_RCR_GRS (1 << 31) +#define ENET_MAX_FRAME_SIZE (1 << ENET_RCR_MAX_FL_LENGTH) + /* TCR */ #define ENET_TCR_GTS (1 << 0) #define ENET_TCR_FDEN (1 << 2)
Frame truncation length, TRUNC_FL, is determined by the contents of ENET_FTRL register, so convert the code to use it instead of a hardcoded constant. To avoid the case where TRUNC_FL is greater that ENET_MAX_FRAME_SIZE, increase the value of the latter to its theoretical maximum of 16K. Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Jason Wang <jasowang@redhat.com> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> Cc: qemu-devel@nongnu.org Cc: qemu-arm@nongnu.org Cc: yurovsky@gmail.com Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> --- hw/net/imx_fec.c | 4 ++-- include/hw/net/imx_fec.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-)