| Message ID | 0402d187c9f749701f6fe728faf3e28c6b86a74b.1469491920.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote: > The Cadence GEM hardware allows incoming data to be 'screened' based on some > register values. Add support for these screens. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > V2: > - Initial commit > > hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++ > include/hw/net/cadence_gem.h | 2 + > 2 files changed, 153 insertions(+) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index deae122..d38bc1e 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -26,6 +26,7 @@ > #include <zlib.h> /* For crc32 */ > > #include "hw/net/cadence_gem.h" > +#include "qemu/log.h" > #include "net/checksum.h" > > #ifdef CADENCE_GEM_ERR_DEBUG > @@ -141,6 +142,37 @@ > #define GEM_DESCONF6 (0x00000294/4) > #define GEM_DESCONF7 (0x00000298/4) > > +#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4) > + > +#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29) > +#define GEM_ST1R_DSTC_ENABLE (1 << 28) > +#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12) > +#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1) > +#define GEM_ST1R_DSTC_MATCH_SHIFT (4) > +#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1) > +#define GEM_ST1R_QUEUE_SHIFT (0) > +#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1) > + > +#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4) > + > +#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18) > +#define GEM_ST2R_COMPARE_A_SHIFT (13) > +#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1) > +#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12) > +#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9) > +#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \ > + + 1) > +#define GEM_ST2R_QUEUE_SHIFT (0) > +#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1) > + > +#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4) > +#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4) > + > +#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7) > +#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1) > +#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0) > +#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1) > + > /*****************************************/ > #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */ > #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */ > @@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet) > return GEM_RX_REJECT; > } > > +/* Figure out which queue the recieved data should be sent to */ "received" > +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr) Nothing seems to call this -- this probably results in a complaint about an unused function if you build at this point in the series (possibly only with optimisation on). Do we need to also pass in the length of the rxbuf to avoid reading beyond the end of short packets? > +{ > + uint32_t reg; > + bool matched, mismatched; > + int i, j; > + > + for (i = 0; i < s->num_type1_screeners; i++) { > + reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i]; > + matched = false; > + mismatched = false; > + > + /* Screening is based on UDP Port */ > + if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) { > + uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23]; > + if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT, > + GEM_ST1R_UDP_PORT_MATCH_WIDTH)) { > + matched = true; > + } else { > + mismatched = true; > + } > + } > + > + /* Screening is based on DS/TC */ > + if (reg & GEM_ST1R_DSTC_ENABLE) { > + uint16_t dscp = rxbuf_ptr[14 + 1]; Why uint16_t if we're only reading one byte? > + if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT, > + GEM_ST1R_DSTC_MATCH_WIDTH)) { > + matched = true; > + } else { > + mismatched = true; > + } > + } > + > + if (matched && !mismatched) { > + return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH); > + } > + } > + > + for (i = 0; i < s->num_type2_screeners; i++) { > + reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i]; > + matched = false; > + mismatched = false; > + > + if (reg & GEM_ST2R_ETHERTYPE_ENABLE) { > + uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13]; > + int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT, > + GEM_ST2R_ETHERTYPE_INDEX_WIDTH); > + > + if (et_idx > s->num_type2_screeners) { > + qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype " > + "register index: %d\n", et_idx); > + } > + if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 + > + et_idx]) { > + matched = true; > + } else { > + mismatched = true; > + } > + } > + > + /* Compare A, B, C */ > + for (j = 0; j < 3; j++) { > + uint32_t cr0, cr1, mask; > + uint16_t rx_cmp; > + int offset; > + int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6, > + GEM_ST2R_COMPARE_WIDTH); > + > + if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) { > + continue; > + } > + if (cr_idx > s->num_type2_screeners) { > + qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare " > + "register index: %d\n", cr_idx); > + } > + > + cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2]; > + cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1]; > + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT, > + GEM_T2CW1_OFFSET_VALUE_WIDTH); > + > + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT, > + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) { You pulled this out into 'offset' so you can just switch (offset). > + case (3): /* Skip UDP header */ You don't need brackets around the constants in case labels. > + qemu_log_mask(LOG_UNIMP, "TCP compare offsets" > + "unimplemented - assuming UDP\n"); > + offset += 8; > + /* Fallthrough */ > + case (2): /* skip the IP header */ > + offset += 20; > + /* Fallthrough */ > + case (1): /* Count from after the ethertype */ > + offset += 14; 'break' here would be a good idea. What should happen for case 0? Guest error? > + } > + > + rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset]; > + mask = extract32(cr0, 0, 16); > + > + if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) { > + matched = true; > + } else { > + mismatched = true; > + } > + } > + > + if (matched && !mismatched) { > + return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH); > + } > + } > + > + /* We made it here, assume it's queue 0 */ > + return 0; > +} > + > static void gem_get_rx_desc(CadenceGEMState *s) > { > DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]); > @@ -1263,6 +1410,10 @@ static Property gem_properties[] = { > DEFINE_NIC_PROPERTIES(CadenceGEMState, conf), > DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState, > num_priority_queues, 1), > + DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState, > + num_type1_screeners, 4), > + DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState, > + num_type2_screeners, 4), realize() should sanity check the properties aren't set too large. > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h > index 77e0582..f2f78c0 100644 > --- a/include/hw/net/cadence_gem.h > +++ b/include/hw/net/cadence_gem.h > @@ -46,6 +46,8 @@ typedef struct CadenceGEMState { > > /* Static properties */ > uint8_t num_priority_queues; > + uint8_t num_type1_screeners; > + uint8_t num_type2_screeners; > > /* GEM registers backing store */ > uint32_t regs[CADENCE_GEM_MAXREG]; > -- > 2.7.4 thanks -- PMM
On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote: >> The Cadence GEM hardware allows incoming data to be 'screened' based on some >> register values. Add support for these screens. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> V2: >> - Initial commit >> >> hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++ >> include/hw/net/cadence_gem.h | 2 + >> 2 files changed, 153 insertions(+) >> >> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c >> index deae122..d38bc1e 100644 >> --- a/hw/net/cadence_gem.c >> +++ b/hw/net/cadence_gem.c >> @@ -26,6 +26,7 @@ >> #include <zlib.h> /* For crc32 */ >> >> #include "hw/net/cadence_gem.h" >> +#include "qemu/log.h" >> #include "net/checksum.h" >> >> #ifdef CADENCE_GEM_ERR_DEBUG >> @@ -141,6 +142,37 @@ >> #define GEM_DESCONF6 (0x00000294/4) >> #define GEM_DESCONF7 (0x00000298/4) >> >> +#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4) >> + >> +#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29) >> +#define GEM_ST1R_DSTC_ENABLE (1 << 28) >> +#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12) >> +#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1) >> +#define GEM_ST1R_DSTC_MATCH_SHIFT (4) >> +#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1) >> +#define GEM_ST1R_QUEUE_SHIFT (0) >> +#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1) >> + >> +#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4) >> + >> +#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18) >> +#define GEM_ST2R_COMPARE_A_SHIFT (13) >> +#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1) >> +#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12) >> +#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9) >> +#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \ >> + + 1) >> +#define GEM_ST2R_QUEUE_SHIFT (0) >> +#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1) >> + >> +#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4) >> +#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4) >> + >> +#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7) >> +#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1) >> +#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0) >> +#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1) >> + >> /*****************************************/ >> #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */ >> #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */ >> @@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet) >> return GEM_RX_REJECT; >> } >> >> +/* Figure out which queue the recieved data should be sent to */ > > "received" Fixed. > >> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr) > > Nothing seems to call this -- this probably results in a complaint > about an unused function if you build at this point in the series > (possibly only with optimisation on). There is a warning about this. I wasn't sure what the position on warnings in between patch a series was. I can squash this into the next patch, I was just worried that the patch was getting a little big. What do you think? > > Do we need to also pass in the length of the rxbuf to avoid > reading beyond the end of short packets? That is probably a good idea. > >> +{ >> + uint32_t reg; >> + bool matched, mismatched; >> + int i, j; >> + >> + for (i = 0; i < s->num_type1_screeners; i++) { >> + reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i]; >> + matched = false; >> + mismatched = false; >> + >> + /* Screening is based on UDP Port */ >> + if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) { >> + uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23]; >> + if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT, >> + GEM_ST1R_UDP_PORT_MATCH_WIDTH)) { >> + matched = true; >> + } else { >> + mismatched = true; >> + } >> + } >> + >> + /* Screening is based on DS/TC */ >> + if (reg & GEM_ST1R_DSTC_ENABLE) { >> + uint16_t dscp = rxbuf_ptr[14 + 1]; > > Why uint16_t if we're only reading one byte? Good point, fixed. > >> + if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT, >> + GEM_ST1R_DSTC_MATCH_WIDTH)) { >> + matched = true; >> + } else { >> + mismatched = true; >> + } >> + } >> + >> + if (matched && !mismatched) { >> + return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH); >> + } >> + } >> + >> + for (i = 0; i < s->num_type2_screeners; i++) { >> + reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i]; >> + matched = false; >> + mismatched = false; >> + >> + if (reg & GEM_ST2R_ETHERTYPE_ENABLE) { >> + uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13]; >> + int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT, >> + GEM_ST2R_ETHERTYPE_INDEX_WIDTH); >> + >> + if (et_idx > s->num_type2_screeners) { >> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype " >> + "register index: %d\n", et_idx); >> + } >> + if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 + >> + et_idx]) { >> + matched = true; >> + } else { >> + mismatched = true; >> + } >> + } >> + >> + /* Compare A, B, C */ >> + for (j = 0; j < 3; j++) { >> + uint32_t cr0, cr1, mask; >> + uint16_t rx_cmp; >> + int offset; >> + int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6, >> + GEM_ST2R_COMPARE_WIDTH); >> + >> + if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) { >> + continue; >> + } >> + if (cr_idx > s->num_type2_screeners) { >> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare " >> + "register index: %d\n", cr_idx); >> + } >> + >> + cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2]; >> + cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1]; >> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT, >> + GEM_T2CW1_OFFSET_VALUE_WIDTH); >> + >> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT, >> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) { > > You pulled this out into 'offset' so you can just switch (offset). They are actually different. > >> + case (3): /* Skip UDP header */ > > You don't need brackets around the constants in case labels. Fixed > >> + qemu_log_mask(LOG_UNIMP, "TCP compare offsets" >> + "unimplemented - assuming UDP\n"); >> + offset += 8; >> + /* Fallthrough */ >> + case (2): /* skip the IP header */ >> + offset += 20; >> + /* Fallthrough */ >> + case (1): /* Count from after the ethertype */ >> + offset += 14; > > 'break' here would be a good idea. > > What should happen for case 0? Guest error? No change to offset. I added a case for it. > >> + } >> + >> + rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset]; >> + mask = extract32(cr0, 0, 16); >> + >> + if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) { >> + matched = true; >> + } else { >> + mismatched = true; >> + } >> + } >> + >> + if (matched && !mismatched) { >> + return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH); >> + } >> + } >> + >> + /* We made it here, assume it's queue 0 */ >> + return 0; >> +} >> + >> static void gem_get_rx_desc(CadenceGEMState *s) >> { >> DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]); >> @@ -1263,6 +1410,10 @@ static Property gem_properties[] = { >> DEFINE_NIC_PROPERTIES(CadenceGEMState, conf), >> DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState, >> num_priority_queues, 1), >> + DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState, >> + num_type1_screeners, 4), >> + DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState, >> + num_type2_screeners, 4), > > realize() should sanity check the properties aren't set too large. Ok, adding it. Thanks, Alistair > >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h >> index 77e0582..f2f78c0 100644 >> --- a/include/hw/net/cadence_gem.h >> +++ b/include/hw/net/cadence_gem.h >> @@ -46,6 +46,8 @@ typedef struct CadenceGEMState { >> >> /* Static properties */ >> uint8_t num_priority_queues; >> + uint8_t num_type1_screeners; >> + uint8_t num_type2_screeners; >> >> /* GEM registers backing store */ >> uint32_t regs[CADENCE_GEM_MAXREG]; >> -- >> 2.7.4 > > thanks > -- PMM >
On 26 July 2016 at 17:42, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote: >>> The Cadence GEM hardware allows incoming data to be 'screened' based on some >>> register values. Add support for these screens. >>> >>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr) >> >> Nothing seems to call this -- this probably results in a complaint >> about an unused function if you build at this point in the series >> (possibly only with optimisation on). > > There is a warning about this. I wasn't sure what the position on > warnings in between patch a series was. I can squash this into the > next patch, I was just worried that the patch was getting a little > big. > > What do you think? Warnings are compile failures by default, so they break bisection. That makes them worth avoiding. Here's a rearrangement that I think should work, though it's a little tedious: (1) change patch 2/6 so that instead of using hardcoded [0] for the array dereferences it uses [q], with an "int q = 0;" at the top of the relevant functions (gem_transmit and gem_receive). (This will also reduce churn in patch 4 since we just go from foo to foo[q] rather than foo to foo[0] to foo[q].) (2) Then this patch can switch the 'q = 0' to the + /* Find which queue we are targetting */ + q = get_queue_from_screen(s, rxbuf_ptr); that's currently in patch 4. (It'll always return 0 at this point, since the registers can't be written by the guest yet.) >>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT, >>> + GEM_T2CW1_OFFSET_VALUE_WIDTH); >>> + >>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT, >>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) { >> >> You pulled this out into 'offset' so you can just switch (offset). > > They are actually different. Oops, so they are... thanks -- PMM
On Tue, Jul 26, 2016 at 9:55 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 26 July 2016 at 17:42, Alistair Francis <alistair.francis@xilinx.com> wrote: >> On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote: >>>> The Cadence GEM hardware allows incoming data to be 'screened' based on some >>>> register values. Add support for these screens. >>>> >>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > >>>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr) >>> >>> Nothing seems to call this -- this probably results in a complaint >>> about an unused function if you build at this point in the series >>> (possibly only with optimisation on). >> >> There is a warning about this. I wasn't sure what the position on >> warnings in between patch a series was. I can squash this into the >> next patch, I was just worried that the patch was getting a little >> big. >> >> What do you think? > > Warnings are compile failures by default, so they break bisection. > That makes them worth avoiding. > > Here's a rearrangement that I think should work, though it's > a little tedious: > > (1) change patch 2/6 so that instead of using hardcoded [0] for > the array dereferences it uses [q], with an "int q = 0;" at the > top of the relevant functions (gem_transmit and gem_receive). > (This will also reduce churn in patch 4 since we just go from > foo to foo[q] rather than foo to foo[0] to foo[q].) > > (2) Then this patch can switch the 'q = 0' to the > + /* Find which queue we are targetting */ > + q = get_queue_from_screen(s, rxbuf_ptr); > > that's currently in patch 4. (It'll always return 0 at this point, > since the registers can't be written by the guest yet.) I was hoping to avoid that, but it actually wasn't too bad. Sending the next version now. Thanks, Alistair > >>>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT, >>>> + GEM_T2CW1_OFFSET_VALUE_WIDTH); >>>> + >>>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT, >>>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) { >>> >>> You pulled this out into 'offset' so you can just switch (offset). >> >> They are actually different. > > Oops, so they are... > > thanks > -- PMM >
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index deae122..d38bc1e 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -26,6 +26,7 @@ #include <zlib.h> /* For crc32 */ #include "hw/net/cadence_gem.h" +#include "qemu/log.h" #include "net/checksum.h" #ifdef CADENCE_GEM_ERR_DEBUG @@ -141,6 +142,37 @@ #define GEM_DESCONF6 (0x00000294/4) #define GEM_DESCONF7 (0x00000298/4) +#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4) + +#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29) +#define GEM_ST1R_DSTC_ENABLE (1 << 28) +#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12) +#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1) +#define GEM_ST1R_DSTC_MATCH_SHIFT (4) +#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1) +#define GEM_ST1R_QUEUE_SHIFT (0) +#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1) + +#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4) + +#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18) +#define GEM_ST2R_COMPARE_A_SHIFT (13) +#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1) +#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12) +#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9) +#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \ + + 1) +#define GEM_ST2R_QUEUE_SHIFT (0) +#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1) + +#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4) +#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4) + +#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7) +#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1) +#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0) +#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1) + /*****************************************/ #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */ #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */ @@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet) return GEM_RX_REJECT; } +/* Figure out which queue the recieved data should be sent to */ +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr) +{ + uint32_t reg; + bool matched, mismatched; + int i, j; + + for (i = 0; i < s->num_type1_screeners; i++) { + reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i]; + matched = false; + mismatched = false; + + /* Screening is based on UDP Port */ + if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) { + uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23]; + if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT, + GEM_ST1R_UDP_PORT_MATCH_WIDTH)) { + matched = true; + } else { + mismatched = true; + } + } + + /* Screening is based on DS/TC */ + if (reg & GEM_ST1R_DSTC_ENABLE) { + uint16_t dscp = rxbuf_ptr[14 + 1]; + if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT, + GEM_ST1R_DSTC_MATCH_WIDTH)) { + matched = true; + } else { + mismatched = true; + } + } + + if (matched && !mismatched) { + return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH); + } + } + + for (i = 0; i < s->num_type2_screeners; i++) { + reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i]; + matched = false; + mismatched = false; + + if (reg & GEM_ST2R_ETHERTYPE_ENABLE) { + uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13]; + int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT, + GEM_ST2R_ETHERTYPE_INDEX_WIDTH); + + if (et_idx > s->num_type2_screeners) { + qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype " + "register index: %d\n", et_idx); + } + if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 + + et_idx]) { + matched = true; + } else { + mismatched = true; + } + } + + /* Compare A, B, C */ + for (j = 0; j < 3; j++) { + uint32_t cr0, cr1, mask; + uint16_t rx_cmp; + int offset; + int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6, + GEM_ST2R_COMPARE_WIDTH); + + if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) { + continue; + } + if (cr_idx > s->num_type2_screeners) { + qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare " + "register index: %d\n", cr_idx); + } + + cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2]; + cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1]; + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT, + GEM_T2CW1_OFFSET_VALUE_WIDTH); + + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT, + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) { + case (3): /* Skip UDP header */ + qemu_log_mask(LOG_UNIMP, "TCP compare offsets" + "unimplemented - assuming UDP\n"); + offset += 8; + /* Fallthrough */ + case (2): /* skip the IP header */ + offset += 20; + /* Fallthrough */ + case (1): /* Count from after the ethertype */ + offset += 14; + } + + rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset]; + mask = extract32(cr0, 0, 16); + + if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) { + matched = true; + } else { + mismatched = true; + } + } + + if (matched && !mismatched) { + return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH); + } + } + + /* We made it here, assume it's queue 0 */ + return 0; +} + static void gem_get_rx_desc(CadenceGEMState *s) { DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]); @@ -1263,6 +1410,10 @@ static Property gem_properties[] = { DEFINE_NIC_PROPERTIES(CadenceGEMState, conf), DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState, num_priority_queues, 1), + DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState, + num_type1_screeners, 4), + DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState, + num_type2_screeners, 4), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h index 77e0582..f2f78c0 100644 --- a/include/hw/net/cadence_gem.h +++ b/include/hw/net/cadence_gem.h @@ -46,6 +46,8 @@ typedef struct CadenceGEMState { /* Static properties */ uint8_t num_priority_queues; + uint8_t num_type1_screeners; + uint8_t num_type2_screeners; /* GEM registers backing store */ uint32_t regs[CADENCE_GEM_MAXREG];
The Cadence GEM hardware allows incoming data to be 'screened' based on some register values. Add support for these screens. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- V2: - Initial commit hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++ include/hw/net/cadence_gem.h | 2 + 2 files changed, 153 insertions(+)