* [PATCH 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 2/9] igb: handle PF/VF reset properly Sriram Yagnaraman
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
I would like to review and be informed on changes to igb device
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ece23b2b15..7d0e84ce37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2224,6 +2224,7 @@ F: tests/qtest/libqos/e1000e.*
igb
M: Akihiko Odaki <akihiko.odaki@daynix.com>
+R: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
S: Maintained
F: docs/system/devices/igb.rst
F: hw/net/igb*
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/9] igb: handle PF/VF reset properly
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-29 5:58 ` Akihiko Odaki
2023-01-28 13:46 ` [PATCH 3/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Use PFRSTD to reset RSTI bit for VFs, and raise VFLRE interrupt when VF
is reset.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/e1000x_regs.h | 1 +
hw/net/igb_core.c | 33 +++++++++++++++++++++------------
hw/net/trace-events | 2 ++
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
index fb5b861135..bb3fb36b8d 100644
--- a/hw/net/e1000x_regs.h
+++ b/hw/net/e1000x_regs.h
@@ -548,6 +548,7 @@
#define E1000_CTRL_EXT_ASDCHK 0x00001000 /* auto speed detection check */
#define E1000_CTRL_EXT_EE_RST 0x00002000 /* EEPROM reset */
+#define E1000_CTRL_EXT_PFRSTD 0x00004000 /* PF reset done indication */
#define E1000_CTRL_EXT_LINK_EN 0x00010000 /* enable link status from external LINK_0 and LINK_1 pins */
#define E1000_CTRL_EXT_DRV_LOAD 0x10000000 /* Driver loaded bit for FW */
#define E1000_CTRL_EXT_EIAME 0x01000000
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index abeb9c7889..9bd53cc25f 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1902,14 +1902,6 @@ static void igb_set_eims(IGBCore *core, int index, uint32_t val)
igb_update_interrupt_state(core);
}
-static void igb_vf_reset(IGBCore *core, uint16_t vfn)
-{
- /* TODO: Reset of the queue enable and the interrupt registers of the VF. */
-
- core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
- core->mac[V2PMAILBOX0 + vfn] = E1000_V2PMAILBOX_RSTD;
-}
-
static void mailbox_interrupt_to_vf(IGBCore *core, uint16_t vfn)
{
uint32_t ent = core->mac[VTIVAR_MISC + vfn];
@@ -1987,6 +1979,17 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
}
}
+static void igb_vf_reset(IGBCore *core, uint16_t vfn)
+{
+ /* disable Rx and Tx for the VF*/
+ core->mac[VFTE] &= ~BIT(vfn);
+ core->mac[VFRE] &= ~BIT(vfn);
+ /* indicate VF reset to PF */
+ core->mac[VFLRE] |= BIT(vfn);
+ /* VFLRE and mailbox use the same interrupt cause */
+ mailbox_interrupt_to_pf(core);
+}
+
static void igb_w1c(IGBCore *core, int index, uint32_t val)
{
core->mac[index] &= ~val;
@@ -2241,14 +2244,20 @@ igb_set_status(IGBCore *core, int index, uint32_t val)
static void
igb_set_ctrlext(IGBCore *core, int index, uint32_t val)
{
- trace_e1000e_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
- !!(val & E1000_CTRL_EXT_SPD_BYPS));
-
- /* TODO: PFRSTD */
+ trace_igb_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
+ !!(val & E1000_CTRL_EXT_SPD_BYPS),
+ !!(val & E1000_CTRL_EXT_PFRSTD));
/* Zero self-clearing bits */
val &= ~(E1000_CTRL_EXT_ASDCHK | E1000_CTRL_EXT_EE_RST);
core->mac[CTRL_EXT] = val;
+
+ if (core->mac[CTRL_EXT] & E1000_CTRL_EXT_PFRSTD) {
+ for (int vfn = 0; vfn < IGB_MAX_VF_FUNCTIONS; vfn++) {
+ core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
+ core->mac[V2PMAILBOX0 + vfn] |= E1000_V2PMAILBOX_RSTD;
+ }
+ }
}
static void
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 2f791b9b57..e94172e748 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -281,6 +281,8 @@ igb_core_mdic_read_unhandled(uint32_t addr) "MDIC READ: PHY[%u] UNHANDLED"
igb_core_mdic_write(uint32_t addr, uint32_t data) "MDIC WRITE: PHY[%u] = 0x%x"
igb_core_mdic_write_unhandled(uint32_t addr) "MDIC WRITE: PHY[%u] UNHANDLED"
+igb_link_set_ext_params(bool asd_check, bool speed_select_bypass, bool pfrstd) "Set extended link params: ASD check: %d, Speed select bypass: %d, PF reset done: %d"
+
igb_rx_desc_buff_size(uint32_t b) "buffer size: %u"
igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint32_t len) "addr: 0x%"PRIx64", offset: %u, from: %p, length: %u"
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/9] igb: handle PF/VF reset properly
2023-01-28 13:46 ` [PATCH 2/9] igb: handle PF/VF reset properly Sriram Yagnaraman
@ 2023-01-29 5:58 ` Akihiko Odaki
2023-01-29 6:15 ` Akihiko Odaki
0 siblings, 1 reply; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 5:58 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> Use PFRSTD to reset RSTI bit for VFs, and raise VFLRE interrupt when VF
> is reset.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/e1000x_regs.h | 1 +
> hw/net/igb_core.c | 33 +++++++++++++++++++++------------
> hw/net/trace-events | 2 ++
> 3 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
> index fb5b861135..bb3fb36b8d 100644
> --- a/hw/net/e1000x_regs.h
> +++ b/hw/net/e1000x_regs.h
> @@ -548,6 +548,7 @@
>
> #define E1000_CTRL_EXT_ASDCHK 0x00001000 /* auto speed detection check */
> #define E1000_CTRL_EXT_EE_RST 0x00002000 /* EEPROM reset */
> +#define E1000_CTRL_EXT_PFRSTD 0x00004000 /* PF reset done indication */
> #define E1000_CTRL_EXT_LINK_EN 0x00010000 /* enable link status from external LINK_0 and LINK_1 pins */
> #define E1000_CTRL_EXT_DRV_LOAD 0x10000000 /* Driver loaded bit for FW */
> #define E1000_CTRL_EXT_EIAME 0x01000000
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index abeb9c7889..9bd53cc25f 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -1902,14 +1902,6 @@ static void igb_set_eims(IGBCore *core, int index, uint32_t val)
> igb_update_interrupt_state(core);
> }
>
> -static void igb_vf_reset(IGBCore *core, uint16_t vfn)
> -{
> - /* TODO: Reset of the queue enable and the interrupt registers of the VF. */
> -
> - core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
> - core->mac[V2PMAILBOX0 + vfn] = E1000_V2PMAILBOX_RSTD;
> -}
> -
> static void mailbox_interrupt_to_vf(IGBCore *core, uint16_t vfn)
> {
> uint32_t ent = core->mac[VTIVAR_MISC + vfn];
> @@ -1987,6 +1979,17 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
> }
> }
>
> +static void igb_vf_reset(IGBCore *core, uint16_t vfn)
> +{
> + /* disable Rx and Tx for the VF*/
> + core->mac[VFTE] &= ~BIT(vfn);
> + core->mac[VFRE] &= ~BIT(vfn);
> + /* indicate VF reset to PF */
> + core->mac[VFLRE] |= BIT(vfn);
> + /* VFLRE and mailbox use the same interrupt cause */
> + mailbox_interrupt_to_pf(core);
> +}
> +
Please do not move the function unless you have a legitimate reason for
that.
> static void igb_w1c(IGBCore *core, int index, uint32_t val)
> {
> core->mac[index] &= ~val;
> @@ -2241,14 +2244,20 @@ igb_set_status(IGBCore *core, int index, uint32_t val)
> static void
> igb_set_ctrlext(IGBCore *core, int index, uint32_t val)
> {
> - trace_e1000e_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
> - !!(val & E1000_CTRL_EXT_SPD_BYPS));
> -
> - /* TODO: PFRSTD */
> + trace_igb_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
> + !!(val & E1000_CTRL_EXT_SPD_BYPS),
> + !!(val & E1000_CTRL_EXT_PFRSTD));
>
> /* Zero self-clearing bits */
> val &= ~(E1000_CTRL_EXT_ASDCHK | E1000_CTRL_EXT_EE_RST);
> core->mac[CTRL_EXT] = val;
> +
> + if (core->mac[CTRL_EXT] & E1000_CTRL_EXT_PFRSTD) {
> + for (int vfn = 0; vfn < IGB_MAX_VF_FUNCTIONS; vfn++) {
> + core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
> + core->mac[V2PMAILBOX0 + vfn] |= E1000_V2PMAILBOX_RSTD;
> + }
> + }
> }
>
> static void
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index 2f791b9b57..e94172e748 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -281,6 +281,8 @@ igb_core_mdic_read_unhandled(uint32_t addr) "MDIC READ: PHY[%u] UNHANDLED"
> igb_core_mdic_write(uint32_t addr, uint32_t data) "MDIC WRITE: PHY[%u] = 0x%x"
> igb_core_mdic_write_unhandled(uint32_t addr) "MDIC WRITE: PHY[%u] UNHANDLED"
>
> +igb_link_set_ext_params(bool asd_check, bool speed_select_bypass, bool pfrstd) "Set extended link params: ASD check: %d, Speed select bypass: %d, PF reset done: %d"
> +
> igb_rx_desc_buff_size(uint32_t b) "buffer size: %u"
> igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint32_t len) "addr: 0x%"PRIx64", offset: %u, from: %p, length: %u"
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/9] igb: handle PF/VF reset properly
2023-01-29 5:58 ` Akihiko Odaki
@ 2023-01-29 6:15 ` Akihiko Odaki
0 siblings, 0 replies; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 6:15 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/29 14:58, Akihiko Odaki wrote:
> On 2023/01/28 22:46, Sriram Yagnaraman wrote:
>> Use PFRSTD to reset RSTI bit for VFs, and raise VFLRE interrupt when VF
>> is reset.
>>
>> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>> ---
>> hw/net/e1000x_regs.h | 1 +
>> hw/net/igb_core.c | 33 +++++++++++++++++++++------------
>> hw/net/trace-events | 2 ++
>> 3 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
>> index fb5b861135..bb3fb36b8d 100644
>> --- a/hw/net/e1000x_regs.h
>> +++ b/hw/net/e1000x_regs.h
>> @@ -548,6 +548,7 @@
>> #define E1000_CTRL_EXT_ASDCHK 0x00001000 /* auto speed detection
>> check */
>> #define E1000_CTRL_EXT_EE_RST 0x00002000 /* EEPROM reset */
>> +#define E1000_CTRL_EXT_PFRSTD 0x00004000 /* PF reset done indication */
>> #define E1000_CTRL_EXT_LINK_EN 0x00010000 /* enable link status from
>> external LINK_0 and LINK_1 pins */
>> #define E1000_CTRL_EXT_DRV_LOAD 0x10000000 /* Driver loaded bit for
>> FW */
>> #define E1000_CTRL_EXT_EIAME 0x01000000
>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
>> index abeb9c7889..9bd53cc25f 100644
>> --- a/hw/net/igb_core.c
>> +++ b/hw/net/igb_core.c
>> @@ -1902,14 +1902,6 @@ static void igb_set_eims(IGBCore *core, int
>> index, uint32_t val)
>> igb_update_interrupt_state(core);
>> }
>> -static void igb_vf_reset(IGBCore *core, uint16_t vfn)
>> -{
>> - /* TODO: Reset of the queue enable and the interrupt registers of
>> the VF. */
>> -
>> - core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
>> - core->mac[V2PMAILBOX0 + vfn] = E1000_V2PMAILBOX_RSTD;
>> -}
>> -
>> static void mailbox_interrupt_to_vf(IGBCore *core, uint16_t vfn)
>> {
>> uint32_t ent = core->mac[VTIVAR_MISC + vfn];
>> @@ -1987,6 +1979,17 @@ static void igb_set_vfmailbox(IGBCore *core,
>> int index, uint32_t val)
>> }
>> }
>> +static void igb_vf_reset(IGBCore *core, uint16_t vfn)
>> +{
>> + /* disable Rx and Tx for the VF*/
>> + core->mac[VFTE] &= ~BIT(vfn);
>> + core->mac[VFRE] &= ~BIT(vfn);
>> + /* indicate VF reset to PF */
>> + core->mac[VFLRE] |= BIT(vfn);
>> + /* VFLRE and mailbox use the same interrupt cause */
>> + mailbox_interrupt_to_pf(core);
>> +}
>> +
>
> Please do not move the function unless you have a legitimate reason for
> that.
I got it. It is necessary to refer mailbox_interrupt_to_pf().
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
>> static void igb_w1c(IGBCore *core, int index, uint32_t val)
>> {
>> core->mac[index] &= ~val;
>> @@ -2241,14 +2244,20 @@ igb_set_status(IGBCore *core, int index,
>> uint32_t val)
>> static void
>> igb_set_ctrlext(IGBCore *core, int index, uint32_t val)
>> {
>> - trace_e1000e_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
>> - !!(val & E1000_CTRL_EXT_SPD_BYPS));
>> -
>> - /* TODO: PFRSTD */
>> + trace_igb_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
>> + !!(val & E1000_CTRL_EXT_SPD_BYPS),
>> + !!(val & E1000_CTRL_EXT_PFRSTD));
>> /* Zero self-clearing bits */
>> val &= ~(E1000_CTRL_EXT_ASDCHK | E1000_CTRL_EXT_EE_RST);
>> core->mac[CTRL_EXT] = val;
>> +
>> + if (core->mac[CTRL_EXT] & E1000_CTRL_EXT_PFRSTD) {
>> + for (int vfn = 0; vfn < IGB_MAX_VF_FUNCTIONS; vfn++) {
>> + core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
>> + core->mac[V2PMAILBOX0 + vfn] |= E1000_V2PMAILBOX_RSTD;
>> + }
>> + }
>> }
>> static void
>> diff --git a/hw/net/trace-events b/hw/net/trace-events
>> index 2f791b9b57..e94172e748 100644
>> --- a/hw/net/trace-events
>> +++ b/hw/net/trace-events
>> @@ -281,6 +281,8 @@ igb_core_mdic_read_unhandled(uint32_t addr) "MDIC
>> READ: PHY[%u] UNHANDLED"
>> igb_core_mdic_write(uint32_t addr, uint32_t data) "MDIC WRITE:
>> PHY[%u] = 0x%x"
>> igb_core_mdic_write_unhandled(uint32_t addr) "MDIC WRITE: PHY[%u]
>> UNHANDLED"
>> +igb_link_set_ext_params(bool asd_check, bool speed_select_bypass,
>> bool pfrstd) "Set extended link params: ASD check: %d, Speed select
>> bypass: %d, PF reset done: %d"
>> +
>> igb_rx_desc_buff_size(uint32_t b) "buffer size: %u"
>> igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void*
>> source, uint32_t len) "addr: 0x%"PRIx64", offset: %u, from: %p,
>> length: %u"
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/9] igb: implement VFRE and VFTE registers
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 2/9] igb: handle PF/VF reset properly Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-29 9:15 ` Akihiko Odaki
2023-01-28 13:46 ` [PATCH 4/9] igb: check oversized packets for VMDq Sriram Yagnaraman
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Also add checks for RXDCTL/TXDCTL queue enable bits
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 42 +++++++++++++++++++++++++++++++-----------
hw/net/igb_regs.h | 3 ++-
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 9bd53cc25f..6bca5459b9 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -778,6 +778,19 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t base,
return igb_tx_wb_eic(core, txi->idx);
}
+static inline bool
+igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi)
+{
+ bool vmdq = core->mac[MRQC] & 1;
+ uint16_t qn = txi->idx;
+ uint16_t vfn = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+
+ return (core->mac[TCTL] & E1000_TCTL_EN) &&
+ (vmdq ? (core->mac[VFTE] & BIT(vfn)) : true) &&
+ (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
+}
+
static void
igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
{
@@ -787,8 +800,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
const E1000E_RingInfo *txi = txr->i;
uint32_t eic = 0;
- /* TODO: check if the queue itself is enabled too. */
- if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
+ if (!igb_tx_enabled(core, txi)) {
trace_e1000e_tx_disabled();
return;
}
@@ -1003,6 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
}
+ queues &= core->mac[VFRE];
igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
if (rss_info->queue & 1) {
queues <<= 8;
@@ -1486,7 +1499,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
uint16_t queues = 0;
- uint32_t n;
+ uint32_t n = 0;
uint8_t min_buf[ETH_ZLEN];
struct iovec min_iov;
struct eth_header *ehdr;
@@ -1566,26 +1579,22 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
}
igb_rx_ring_init(core, &rxr, i);
-
- trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
-
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
retval = 0;
}
}
if (retval) {
- n = E1000_ICR_RXT0;
-
igb_rx_fix_l4_csum(core, core->rx_pkt);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
- if (!(queues & BIT(i))) {
+ if (!(queues & BIT(i)) ||
+ !(core->mac[E1000_RXDCTL(i) >> 2] & E1000_RXDCTL_QUEUE_ENABLE)) {
continue;
}
igb_rx_ring_init(core, &rxr, i);
-
+ trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
/* Check if receive descriptor minimum threshold hit */
@@ -1594,6 +1603,9 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
}
core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
+
+ /* same as RXDW (rx descriptor written back)*/
+ n = E1000_ICR_RXT0;
}
trace_e1000e_rx_written_to_guest(n);
@@ -1981,9 +1993,16 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
static void igb_vf_reset(IGBCore *core, uint16_t vfn)
{
+ uint16_t qn0 = vfn;
+ uint16_t qn1 = vfn + IGB_MAX_VF_FUNCTIONS;
+
/* disable Rx and Tx for the VF*/
- core->mac[VFTE] &= ~BIT(vfn);
+ core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
+ core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
+ core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
+ core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
core->mac[VFRE] &= ~BIT(vfn);
+ core->mac[VFTE] &= ~BIT(vfn);
/* indicate VF reset to PF */
core->mac[VFLRE] |= BIT(vfn);
/* VFLRE and mailbox use the same interrupt cause */
@@ -3889,6 +3908,7 @@ igb_phy_reg_init[] = {
static const uint32_t igb_mac_reg_init[] = {
[LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
[EEMNGCTL] = BIT(31),
+ [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
[RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
[RXDCTL1] = 1 << 16,
[RXDCTL2] = 1 << 16,
diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h
index ebf3e95023..084e751378 100644
--- a/hw/net/igb_regs.h
+++ b/hw/net/igb_regs.h
@@ -160,7 +160,8 @@ union e1000_adv_rx_desc {
#define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
#define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
-/* Additional Receive Descriptor Control definitions */
+/* Additional RX/TX Descriptor Control definitions */
+#define E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue */
#define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx Queue */
/* Direct Cache Access (DCA) definitions */
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/9] igb: implement VFRE and VFTE registers
2023-01-28 13:46 ` [PATCH 3/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
@ 2023-01-29 9:15 ` Akihiko Odaki
2023-01-30 10:16 ` Sriram Yagnaraman
0 siblings, 1 reply; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 9:15 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> Also add checks for RXDCTL/TXDCTL queue enable bits
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 42 +++++++++++++++++++++++++++++++-----------
> hw/net/igb_regs.h | 3 ++-
> 2 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 9bd53cc25f..6bca5459b9 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -778,6 +778,19 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t base,
> return igb_tx_wb_eic(core, txi->idx);
> }
>
> +static inline bool
> +igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi)
> +{
> + bool vmdq = core->mac[MRQC] & 1;
> + uint16_t qn = txi->idx;
> + uint16_t vfn = (qn > IGB_MAX_VF_FUNCTIONS) ?
> + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
> +
> + return (core->mac[TCTL] & E1000_TCTL_EN) &&
> + (vmdq ? (core->mac[VFTE] & BIT(vfn)) : true) &&
Instead, do: (!vmdq || core->mac[VFTE] & BIT(vfn))
> + (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
> +}
> +
> static void
> igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
> {
> @@ -787,8 +800,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
> const E1000E_RingInfo *txi = txr->i;
> uint32_t eic = 0;
>
> - /* TODO: check if the queue itself is enabled too. */
> - if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
> + if (!igb_tx_enabled(core, txi)) {
> trace_e1000e_tx_disabled();
> return;
> }
> @@ -1003,6 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
> }
>
> + queues &= core->mac[VFRE];
> igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
> if (rss_info->queue & 1) {
> queues <<= 8;
> @@ -1486,7 +1499,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
>
> uint16_t queues = 0;
> - uint32_t n;
> + uint32_t n = 0;
> uint8_t min_buf[ETH_ZLEN];
> struct iovec min_iov;
> struct eth_header *ehdr;
> @@ -1566,26 +1579,22 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> }
>
> igb_rx_ring_init(core, &rxr, i);
> -
> - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> -
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> retval = 0;
> }
This stops sending packet when a disabled queue has no space.
> }
>
> if (retval) {
> - n = E1000_ICR_RXT0;
> -
> igb_rx_fix_l4_csum(core, core->rx_pkt);
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> - if (!(queues & BIT(i))) {
> + if (!(queues & BIT(i)) ||
> + !(core->mac[E1000_RXDCTL(i) >> 2] & E1000_RXDCTL_QUEUE_ENABLE)) {
> continue;
> }
>
> igb_rx_ring_init(core, &rxr, i);
> -
> + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
>
> /* Check if receive descriptor minimum threshold hit */
> @@ -1594,6 +1603,9 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> }
>
> core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> +
> + /* same as RXDW (rx descriptor written back)*/
> + n = E1000_ICR_RXT0;
> }
>
> trace_e1000e_rx_written_to_guest(n);
> @@ -1981,9 +1993,16 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
>
> static void igb_vf_reset(IGBCore *core, uint16_t vfn)
> {
> + uint16_t qn0 = vfn;
> + uint16_t qn1 = vfn + IGB_MAX_VF_FUNCTIONS;
> +
> /* disable Rx and Tx for the VF*/
> - core->mac[VFTE] &= ~BIT(vfn);
> + core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> + core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> + core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> + core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> core->mac[VFRE] &= ~BIT(vfn);
> + core->mac[VFTE] &= ~BIT(vfn);
> /* indicate VF reset to PF */
> core->mac[VFLRE] |= BIT(vfn);
> /* VFLRE and mailbox use the same interrupt cause */
> @@ -3889,6 +3908,7 @@ igb_phy_reg_init[] = {
> static const uint32_t igb_mac_reg_init[] = {
> [LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
> [EEMNGCTL] = BIT(31),
> + [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
> [RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
> [RXDCTL1] = 1 << 16,
> [RXDCTL2] = 1 << 16,
> diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h
> index ebf3e95023..084e751378 100644
> --- a/hw/net/igb_regs.h
> +++ b/hw/net/igb_regs.h
> @@ -160,7 +160,8 @@ union e1000_adv_rx_desc {
> #define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
> #define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
>
> -/* Additional Receive Descriptor Control definitions */
> +/* Additional RX/TX Descriptor Control definitions */
> +#define E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue */
> #define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx Queue */
>
> /* Direct Cache Access (DCA) definitions */
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH 3/9] igb: implement VFRE and VFTE registers
2023-01-29 9:15 ` Akihiko Odaki
@ 2023-01-30 10:16 ` Sriram Yagnaraman
2023-01-30 13:20 ` Akihiko Odaki
0 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-30 10:16 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Sunday, 29 January 2023 10:16
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH 3/9] igb: implement VFRE and VFTE registers
>
> On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> > Also add checks for RXDCTL/TXDCTL queue enable bits
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > ---
> > hw/net/igb_core.c | 42 +++++++++++++++++++++++++++++++-----------
> > hw/net/igb_regs.h | 3 ++-
> > 2 files changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > 9bd53cc25f..6bca5459b9 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -778,6 +778,19 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t
> base,
> > return igb_tx_wb_eic(core, txi->idx);
> > }
> >
> > +static inline bool
> > +igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi) {
> > + bool vmdq = core->mac[MRQC] & 1;
> > + uint16_t qn = txi->idx;
> > + uint16_t vfn = (qn > IGB_MAX_VF_FUNCTIONS) ?
> > + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
> > +
> > + return (core->mac[TCTL] & E1000_TCTL_EN) &&
> > + (vmdq ? (core->mac[VFTE] & BIT(vfn)) : true) &&
>
> Instead, do: (!vmdq || core->mac[VFTE] & BIT(vfn))
>
> > + (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
> > +}
> > +
> > static void
> > igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
> > {
> > @@ -787,8 +800,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing
> *txr)
> > const E1000E_RingInfo *txi = txr->i;
> > uint32_t eic = 0;
> >
> > - /* TODO: check if the queue itself is enabled too. */
> > - if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
> > + if (!igb_tx_enabled(core, txi)) {
> > trace_e1000e_tx_disabled();
> > return;
> > }
> > @@ -1003,6 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> > queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
> > }
> >
> > + queues &= core->mac[VFRE];
> > igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL,
> rss_info);
> > if (rss_info->queue & 1) {
> > queues <<= 8;
> > @@ -1486,7 +1499,7 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
> > static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
> >
> > uint16_t queues = 0;
> > - uint32_t n;
> > + uint32_t n = 0;
> > uint8_t min_buf[ETH_ZLEN];
> > struct iovec min_iov;
> > struct eth_header *ehdr;
> > @@ -1566,26 +1579,22 @@ igb_receive_internal(IGBCore *core, const
> struct iovec *iov, int iovcnt,
> > }
> >
> > igb_rx_ring_init(core, &rxr, i);
> > -
> > - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> > -
> > if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> > retval = 0;
> > }
>
> This stops sending packet when a disabled queue has no space.
Yes, that is true, but I have refactored this part a bit in patchset 9 that fixes this.
>
> > }
> >
> > if (retval) {
> > - n = E1000_ICR_RXT0;
> > -
> > igb_rx_fix_l4_csum(core, core->rx_pkt);
> >
> > for (i = 0; i < IGB_NUM_QUEUES; i++) {
> > - if (!(queues & BIT(i))) {
> > + if (!(queues & BIT(i)) ||
> > + !(core->mac[E1000_RXDCTL(i) >> 2] &
> > + E1000_RXDCTL_QUEUE_ENABLE)) {
> > continue;
> > }
> >
> > igb_rx_ring_init(core, &rxr, i);
> > -
> > + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> > igb_write_packet_to_guest(core, core->rx_pkt, &rxr,
> > &rss_info);
> >
> > /* Check if receive descriptor minimum threshold hit */
> > @@ -1594,6 +1603,9 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
> > }
> >
> > core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> > +
> > + /* same as RXDW (rx descriptor written back)*/
> > + n = E1000_ICR_RXT0;
> > }
> >
> > trace_e1000e_rx_written_to_guest(n);
> > @@ -1981,9 +1993,16 @@ static void igb_set_vfmailbox(IGBCore *core,
> > int index, uint32_t val)
> >
> > static void igb_vf_reset(IGBCore *core, uint16_t vfn)
> > {
> > + uint16_t qn0 = vfn;
> > + uint16_t qn1 = vfn + IGB_MAX_VF_FUNCTIONS;
> > +
> > /* disable Rx and Tx for the VF*/
> > - core->mac[VFTE] &= ~BIT(vfn);
> > + core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> > + core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> > + core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> > + core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> > core->mac[VFRE] &= ~BIT(vfn);
> > + core->mac[VFTE] &= ~BIT(vfn);
> > /* indicate VF reset to PF */
> > core->mac[VFLRE] |= BIT(vfn);
> > /* VFLRE and mailbox use the same interrupt cause */ @@ -3889,6
> > +3908,7 @@ igb_phy_reg_init[] = {
> > static const uint32_t igb_mac_reg_init[] = {
> > [LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
> > [EEMNGCTL] = BIT(31),
> > + [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
> > [RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
> > [RXDCTL1] = 1 << 16,
> > [RXDCTL2] = 1 << 16,
> > diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h index
> > ebf3e95023..084e751378 100644
> > --- a/hw/net/igb_regs.h
> > +++ b/hw/net/igb_regs.h
> > @@ -160,7 +160,8 @@ union e1000_adv_rx_desc {
> > #define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
> > #define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
> >
> > -/* Additional Receive Descriptor Control definitions */
> > +/* Additional RX/TX Descriptor Control definitions */ #define
> > +E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue
> */
> > #define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx
> > Queue */
> >
> > /* Direct Cache Access (DCA) definitions */
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 3/9] igb: implement VFRE and VFTE registers
2023-01-30 10:16 ` Sriram Yagnaraman
@ 2023-01-30 13:20 ` Akihiko Odaki
0 siblings, 0 replies; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-30 13:20 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/30 19:16, Sriram Yagnaraman wrote:
>
>
>> -----Original Message-----
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Sent: Sunday, 29 January 2023 10:16
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
>> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Subject: Re: [PATCH 3/9] igb: implement VFRE and VFTE registers
>>
>> On 2023/01/28 22:46, Sriram Yagnaraman wrote:
>>> Also add checks for RXDCTL/TXDCTL queue enable bits
>>>
>>> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>>> ---
>>> hw/net/igb_core.c | 42 +++++++++++++++++++++++++++++++-----------
>>> hw/net/igb_regs.h | 3 ++-
>>> 2 files changed, 33 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
>>> 9bd53cc25f..6bca5459b9 100644
>>> --- a/hw/net/igb_core.c
>>> +++ b/hw/net/igb_core.c
>>> @@ -778,6 +778,19 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t
>> base,
>>> return igb_tx_wb_eic(core, txi->idx);
>>> }
>>>
>>> +static inline bool
>>> +igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi) {
>>> + bool vmdq = core->mac[MRQC] & 1;
>>> + uint16_t qn = txi->idx;
>>> + uint16_t vfn = (qn > IGB_MAX_VF_FUNCTIONS) ?
>>> + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
>>> +
>>> + return (core->mac[TCTL] & E1000_TCTL_EN) &&
>>> + (vmdq ? (core->mac[VFTE] & BIT(vfn)) : true) &&
>>
>> Instead, do: (!vmdq || core->mac[VFTE] & BIT(vfn))
>>
>>> + (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
>>> +}
>>> +
>>> static void
>>> igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
>>> {
>>> @@ -787,8 +800,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing
>> *txr)
>>> const E1000E_RingInfo *txi = txr->i;
>>> uint32_t eic = 0;
>>>
>>> - /* TODO: check if the queue itself is enabled too. */
>>> - if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
>>> + if (!igb_tx_enabled(core, txi)) {
>>> trace_e1000e_tx_disabled();
>>> return;
>>> }
>>> @@ -1003,6 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
>> const struct eth_header *ehdr,
>>> queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
>>> }
>>>
>>> + queues &= core->mac[VFRE];
>>> igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL,
>> rss_info);
>>> if (rss_info->queue & 1) {
>>> queues <<= 8;
>>> @@ -1486,7 +1499,7 @@ igb_receive_internal(IGBCore *core, const struct
>> iovec *iov, int iovcnt,
>>> static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
>>>
>>> uint16_t queues = 0;
>>> - uint32_t n;
>>> + uint32_t n = 0;
>>> uint8_t min_buf[ETH_ZLEN];
>>> struct iovec min_iov;
>>> struct eth_header *ehdr;
>>> @@ -1566,26 +1579,22 @@ igb_receive_internal(IGBCore *core, const
>> struct iovec *iov, int iovcnt,
>>> }
>>>
>>> igb_rx_ring_init(core, &rxr, i);
>>> -
>>> - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
>>> -
>>> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
>>> retval = 0;
>>> }
>>
>> This stops sending packet when a disabled queue has no space.
>
> Yes, that is true, but I have refactored this part a bit in patchset 9 that fixes this.
I see. Please include the fix in this patch so it will be easier to
review and it won't prevent from bisecting.
>
>>
>>> }
>>>
>>> if (retval) {
>>> - n = E1000_ICR_RXT0;
>>> -
>>> igb_rx_fix_l4_csum(core, core->rx_pkt);
>>>
>>> for (i = 0; i < IGB_NUM_QUEUES; i++) {
>>> - if (!(queues & BIT(i))) {
>>> + if (!(queues & BIT(i)) ||
>>> + !(core->mac[E1000_RXDCTL(i) >> 2] &
>>> + E1000_RXDCTL_QUEUE_ENABLE)) {
>>> continue;
>>> }
>>>
>>> igb_rx_ring_init(core, &rxr, i);
>>> -
>>> + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
>>> igb_write_packet_to_guest(core, core->rx_pkt, &rxr,
>>> &rss_info);
>>>
>>> /* Check if receive descriptor minimum threshold hit */
>>> @@ -1594,6 +1603,9 @@ igb_receive_internal(IGBCore *core, const struct
>> iovec *iov, int iovcnt,
>>> }
>>>
>>> core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
>>> +
>>> + /* same as RXDW (rx descriptor written back)*/
>>> + n = E1000_ICR_RXT0;
>>> }
>>>
>>> trace_e1000e_rx_written_to_guest(n);
>>> @@ -1981,9 +1993,16 @@ static void igb_set_vfmailbox(IGBCore *core,
>>> int index, uint32_t val)
>>>
>>> static void igb_vf_reset(IGBCore *core, uint16_t vfn)
>>> {
>>> + uint16_t qn0 = vfn;
>>> + uint16_t qn1 = vfn + IGB_MAX_VF_FUNCTIONS;
>>> +
>>> /* disable Rx and Tx for the VF*/
>>> - core->mac[VFTE] &= ~BIT(vfn);
>>> + core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
>>> + core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
>>> + core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
>>> + core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
>>> core->mac[VFRE] &= ~BIT(vfn);
>>> + core->mac[VFTE] &= ~BIT(vfn);
>>> /* indicate VF reset to PF */
>>> core->mac[VFLRE] |= BIT(vfn);
>>> /* VFLRE and mailbox use the same interrupt cause */ @@ -3889,6
>>> +3908,7 @@ igb_phy_reg_init[] = {
>>> static const uint32_t igb_mac_reg_init[] = {
>>> [LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
>>> [EEMNGCTL] = BIT(31),
>>> + [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
>>> [RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
>>> [RXDCTL1] = 1 << 16,
>>> [RXDCTL2] = 1 << 16,
>>> diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h index
>>> ebf3e95023..084e751378 100644
>>> --- a/hw/net/igb_regs.h
>>> +++ b/hw/net/igb_regs.h
>>> @@ -160,7 +160,8 @@ union e1000_adv_rx_desc {
>>> #define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
>>> #define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
>>>
>>> -/* Additional Receive Descriptor Control definitions */
>>> +/* Additional RX/TX Descriptor Control definitions */ #define
>>> +E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue
>> */
>>> #define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx
>>> Queue */
>>>
>>> /* Direct Cache Access (DCA) definitions */
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/9] igb: check oversized packets for VMDq
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (2 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 3/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-29 7:06 ` Akihiko Odaki
2023-01-28 13:46 ` [PATCH 5/9] igb: respect E1000_VMOLR_RSSE Sriram Yagnaraman
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 74 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 54 insertions(+), 20 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 6bca5459b9..1eb7ba168f 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1476,6 +1476,30 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
igb_update_rx_stats(core, size, total_size);
}
+static inline bool
+igb_is_oversized(IGBCore *core, const E1000E_RingInfo *rxi, size_t size)
+{
+ bool vmdq = core->mac[MRQC] & 1;
+ uint16_t qn = rxi->idx;
+ uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+
+ bool lpe = (vmdq ? core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE :
+ core->mac[RCTL] & E1000_RCTL_LPE);
+ bool sbp = core->mac[RCTL] & E1000_RCTL_SBP;
+ int maximum_ethernet_vlan_size = 1522;
+ int maximum_ethernet_lpe_size =
+ (vmdq ? core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK :
+ core->mac[RLPML] & E1000_VMOLR_RLPML_MASK);
+
+ if (size > maximum_ethernet_lpe_size ||
+ (size > maximum_ethernet_vlan_size && !lpe && !sbp)) {
+ return true;
+ }
+
+ return false;
+}
+
static inline void
igb_rx_fix_l4_csum(IGBCore *core, struct NetRxPkt *pkt)
{
@@ -1499,7 +1523,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
uint16_t queues = 0;
- uint32_t n = 0;
+ uint16_t oversized = 0;
+ uint32_t icr_bits = 0;
uint8_t min_buf[ETH_ZLEN];
struct iovec min_iov;
struct eth_header *ehdr;
@@ -1509,7 +1534,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
E1000E_RxRing rxr;
E1000E_RSSInfo rss_info;
size_t total_size;
- ssize_t retval;
+ ssize_t retval = 0;
int i;
trace_e1000e_rx_receive_iov(iovcnt);
@@ -1550,11 +1575,6 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
filter_buf = min_buf;
}
- /* Discard oversized packets if !LPE and !SBP. */
- if (e1000x_is_oversized(core->mac, size)) {
- return orig_size;
- }
-
ehdr = PKT_GET_ETH_HDR(filter_buf);
net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
@@ -1571,8 +1591,6 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
e1000x_fcs_len(core->mac);
- retval = orig_size;
-
for (i = 0; i < IGB_NUM_QUEUES; i++) {
if (!(queues & BIT(i))) {
continue;
@@ -1580,42 +1598,58 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
igb_rx_ring_init(core, &rxr, i);
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
- retval = 0;
+ icr_bits |= E1000_ICS_RXO;
}
}
- if (retval) {
+ if (!icr_bits) {
+ retval = orig_size;
igb_rx_fix_l4_csum(core, core->rx_pkt);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
- if (!(queues & BIT(i)) ||
- !(core->mac[E1000_RXDCTL(i) >> 2] & E1000_RXDCTL_QUEUE_ENABLE)) {
+ if (!(queues & BIT(i))) {
continue;
}
igb_rx_ring_init(core, &rxr, i);
+ if (igb_is_oversized(core, rxr.i, size)) {
+ oversized |= BIT(i);
+ continue;
+ }
+
+ if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
+ continue;
+ }
+
trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
/* Check if receive descriptor minimum threshold hit */
if (igb_rx_descr_threshold_hit(core, rxr.i)) {
- n |= E1000_ICS_RXDMT0;
+ icr_bits |= E1000_ICS_RXDMT0;
}
core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
/* same as RXDW (rx descriptor written back)*/
- n = E1000_ICR_RXT0;
+ icr_bits |= E1000_ICR_RXT0;
}
+ }
+
+ /* 8.19.37 increment ROC only if packet is oversized for all queues */
+ if (oversized == queues) {
+ trace_e1000x_rx_oversized(size);
+ e1000x_inc_reg_if_not_full(core->mac, ROC);
+ }
- trace_e1000e_rx_written_to_guest(n);
+ if (icr_bits & E1000_ICR_RXT0) {
+ trace_e1000e_rx_written_to_guest(icr_bits);
} else {
- n = E1000_ICS_RXO;
- trace_e1000e_rx_not_written_to_guest(n);
+ trace_e1000e_rx_not_written_to_guest(icr_bits);
}
- trace_e1000e_rx_interrupt_set(n);
- igb_set_interrupt_cause(core, n);
+ trace_e1000e_rx_interrupt_set(icr_bits);
+ igb_set_interrupt_cause(core, icr_bits);
return retval;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] igb: check oversized packets for VMDq
2023-01-28 13:46 ` [PATCH 4/9] igb: check oversized packets for VMDq Sriram Yagnaraman
@ 2023-01-29 7:06 ` Akihiko Odaki
0 siblings, 0 replies; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 7:06 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 74 ++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 54 insertions(+), 20 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 6bca5459b9..1eb7ba168f 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -1476,6 +1476,30 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
> igb_update_rx_stats(core, size, total_size);
> }
>
> +static inline bool
Please remove inline qualifier. inline qualifier has some adverse effects:
- It suppresses GCC warnings for unused functions. This behavior is
useful when you write a function in a header file, but it is not in this
case.
- It confuses the compiler if the function later grows and becomes
unsuitable for inlining.
- It is noise in source code.
In this case, the compiler should be able to decide to inline the
function or not by its own.
> +igb_is_oversized(IGBCore *core, const E1000E_RingInfo *rxi, size_t size)
> +{
> + bool vmdq = core->mac[MRQC] & 1;
> + uint16_t qn = rxi->idx;
> + uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
> + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
Write as qn % 8; this pattern is already prevalent.
> +
> + bool lpe = (vmdq ? core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE :
> + core->mac[RCTL] & E1000_RCTL_LPE);
RCTL.LPE should be checked even if VMDq is enabled; In section 7.10.3.4,
Size Filtering is defined to check RCTL.LPE, and it is part of packet
switching procedure for virtualized environment. Linux also ensures it
sets the maximum value to RLPML.RLPML if VMDq is enabled:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cfbc871c2174f352542053d25659920d6841ed41
> + bool sbp = core->mac[RCTL] & E1000_RCTL_SBP;
> + int maximum_ethernet_vlan_size = 1522;
> + int maximum_ethernet_lpe_size =
> + (vmdq ? core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK :
> + core->mac[RLPML] & E1000_VMOLR_RLPML_MASK);
> +
> + if (size > maximum_ethernet_lpe_size ||
> + (size > maximum_ethernet_vlan_size && !lpe && !sbp)) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> static inline void
> igb_rx_fix_l4_csum(IGBCore *core, struct NetRxPkt *pkt)
> {
> @@ -1499,7 +1523,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
>
> uint16_t queues = 0;
> - uint32_t n = 0;
> + uint16_t oversized = 0;
> + uint32_t icr_bits = 0;
> uint8_t min_buf[ETH_ZLEN];
> struct iovec min_iov;
> struct eth_header *ehdr;
> @@ -1509,7 +1534,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> E1000E_RxRing rxr;
> E1000E_RSSInfo rss_info;
> size_t total_size;
> - ssize_t retval;
> + ssize_t retval = 0;
> int i;
>
> trace_e1000e_rx_receive_iov(iovcnt);
> @@ -1550,11 +1575,6 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> filter_buf = min_buf;
> }
>
> - /* Discard oversized packets if !LPE and !SBP. */
> - if (e1000x_is_oversized(core->mac, size)) {
> - return orig_size;
> - }
> -
> ehdr = PKT_GET_ETH_HDR(filter_buf);
> net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
>
> @@ -1571,8 +1591,6 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
> e1000x_fcs_len(core->mac);
>
> - retval = orig_size;
> -
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> if (!(queues & BIT(i))) {
> continue;
> @@ -1580,42 +1598,58 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>
> igb_rx_ring_init(core, &rxr, i);
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> - retval = 0;
> + icr_bits |= E1000_ICS_RXO;
> }
> }
>
> - if (retval) {
> + if (!icr_bits) {
> + retval = orig_size;
> igb_rx_fix_l4_csum(core, core->rx_pkt);
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> - if (!(queues & BIT(i)) ||
> - !(core->mac[E1000_RXDCTL(i) >> 2] & E1000_RXDCTL_QUEUE_ENABLE)) {
> + if (!(queues & BIT(i))) {
> continue;
> }
>
> igb_rx_ring_init(core, &rxr, i);
> + if (igb_is_oversized(core, rxr.i, size)) {
> + oversized |= BIT(i);
> + continue;
> + }
VMOLR.RLPML is checked during Rx queue assignment, which is implemented
in igb_receive_assign(). The oversize check should be moved to the function.
> +
> + if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
> + continue;
> + }
> +
> trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
>
> /* Check if receive descriptor minimum threshold hit */
> if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> - n |= E1000_ICS_RXDMT0;
> + icr_bits |= E1000_ICS_RXDMT0;
> }
>
> core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
>
> /* same as RXDW (rx descriptor written back)*/
> - n = E1000_ICR_RXT0;
> + icr_bits |= E1000_ICR_RXT0;
> }
> + }
> +
> + /* 8.19.37 increment ROC only if packet is oversized for all queues */
> + if (oversized == queues) {
> + trace_e1000x_rx_oversized(size);
> + e1000x_inc_reg_if_not_full(core->mac, ROC);
> + }
>
> - trace_e1000e_rx_written_to_guest(n);
> + if (icr_bits & E1000_ICR_RXT0) {
> + trace_e1000e_rx_written_to_guest(icr_bits);
> } else {
> - n = E1000_ICS_RXO;
> - trace_e1000e_rx_not_written_to_guest(n);
> + trace_e1000e_rx_not_written_to_guest(icr_bits);
> }
>
> - trace_e1000e_rx_interrupt_set(n);
> - igb_set_interrupt_cause(core, n);
> + trace_e1000e_rx_interrupt_set(icr_bits);
> + igb_set_interrupt_cause(core, icr_bits);
>
> return retval;
> }
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (3 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 4/9] igb: check oversized packets for VMDq Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-29 7:24 ` Akihiko Odaki
2023-01-28 13:46 ` [PATCH 6/9] igb: add ICR_RXDW Sriram Yagnaraman
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
RSS for VFs is only enabled if VMOLR[n].RSSE is set.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 1eb7ba168f..e4fd4a1a5f 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -69,7 +69,7 @@ typedef struct IGBTxPktVmdqCallbackContext {
static ssize_t
igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
- bool has_vnet, bool *assigned);
+ bool has_vnet, bool *external_tx);
static inline void
igb_set_interrupt_cause(IGBCore *core, uint32_t val);
@@ -942,7 +942,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
if (core->mac[MRQC] & 1) {
if (is_broadcast_ether_addr(ehdr->h_dest)) {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
queues |= BIT(i);
}
@@ -976,7 +976,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
if (macp[f >> 5] & (1 << (f & 0x1f))) {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
queues |= BIT(i);
}
@@ -999,7 +999,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
}
} else {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
mask |= BIT(i);
}
@@ -1018,7 +1018,15 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
queues &= core->mac[VFRE];
igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
if (rss_info->queue & 1) {
- queues <<= 8;
+ for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
+ if (!(queues & BIT(i))) {
+ continue;
+ }
+ if (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE) {
+ queues |= BIT(i + IGB_MAX_VF_FUNCTIONS);
+ queues &= ~BIT(i);
+ }
+ }
}
} else {
switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
2023-01-28 13:46 ` [PATCH 5/9] igb: respect E1000_VMOLR_RSSE Sriram Yagnaraman
@ 2023-01-29 7:24 ` Akihiko Odaki
2023-01-30 12:07 ` Sriram Yagnaraman
0 siblings, 1 reply; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 7:24 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> RSS for VFs is only enabled if VMOLR[n].RSSE is set.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 1eb7ba168f..e4fd4a1a5f 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -69,7 +69,7 @@ typedef struct IGBTxPktVmdqCallbackContext {
>
> static ssize_t
> igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> - bool has_vnet, bool *assigned);
> + bool has_vnet, bool *external_tx);
I admit external_tx is somewhat confusing, but it is more than just
telling if it is assigned to Rx queue or not. If external_tx is not
NULL, it indicates it is part of Tx packet switching. In that case, a
bool value which describes whether the packet should be routed to
external LAN must be assigned. The value can be false even if the packet
is assigned to Rx queues; it will be always false if it is multicast,
for example.
>
> static inline void
> igb_set_interrupt_cause(IGBCore *core, uint32_t val);
> @@ -942,7 +942,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>
> if (core->mac[MRQC] & 1) {
> if (is_broadcast_ether_addr(ehdr->h_dest)) {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
I just left it as 8 because VMDq is not specific to VF. Perhaps it is
better to have another macro to denote the number of VMDq pools, but it
is not done yet.
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
> queues |= BIT(i);
> }
> @@ -976,7 +976,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
> f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
> if (macp[f >> 5] & (1 << (f & 0x1f))) {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
> queues |= BIT(i);
> }
> @@ -999,7 +999,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> }
> }
> } else {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
> mask |= BIT(i);
> }
> @@ -1018,7 +1018,15 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> queues &= core->mac[VFRE];
> igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
> if (rss_info->queue & 1) {
> - queues <<= 8;
> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> + if (!(queues & BIT(i))) {
> + continue;
> + }
> + if (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE) {
> + queues |= BIT(i + IGB_MAX_VF_FUNCTIONS);
> + queues &= ~BIT(i);
> + }
> + }
> }
> } else {
> switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
2023-01-29 7:24 ` Akihiko Odaki
@ 2023-01-30 12:07 ` Sriram Yagnaraman
2023-01-30 13:20 ` Akihiko Odaki
0 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-30 12:07 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Sunday, 29 January 2023 08:25
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
>
> On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> > RSS for VFs is only enabled if VMOLR[n].RSSE is set.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > ---
> > hw/net/igb_core.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > 1eb7ba168f..e4fd4a1a5f 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -69,7 +69,7 @@ typedef struct IGBTxPktVmdqCallbackContext {
> >
> > static ssize_t
> > igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> > - bool has_vnet, bool *assigned);
> > + bool has_vnet, bool *external_tx);
>
> I admit external_tx is somewhat confusing, but it is more than just telling if it
> is assigned to Rx queue or not. If external_tx is not NULL, it indicates it is part
> of Tx packet switching. In that case, a bool value which describes whether the
> packet should be routed to external LAN must be assigned. The value can be
> false even if the packet is assigned to Rx queues; it will be always false if it is
> multicast, for example.
Yes, I undestand the purpose of external_tx. I merely changed the parameter name in the function declaration to match it's defintion. Anyhow, I will remove this change since it was purely comsetic.
>
> >
> > static inline void
> > igb_set_interrupt_cause(IGBCore *core, uint32_t val); @@ -942,7
> > +942,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const
> > struct eth_header *ehdr,
> >
> > if (core->mac[MRQC] & 1) {
> > if (is_broadcast_ether_addr(ehdr->h_dest)) {
> > - for (i = 0; i < 8; i++) {
> > + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
>
> I just left it as 8 because VMDq is not specific to VF. Perhaps it is better to
> have another macro to denote the number of VMDq pools, but it is not done
> yet.
Ok, I agree. I will introduce a new IGB_MAX_VM_POOLS macro instead.
>
> > if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
> > queues |= BIT(i);
> > }
> > @@ -976,7 +976,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> > f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
> > f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
> > if (macp[f >> 5] & (1 << (f & 0x1f))) {
> > - for (i = 0; i < 8; i++) {
> > + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> > if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
> > queues |= BIT(i);
> > }
> > @@ -999,7 +999,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> > }
> > }
> > } else {
> > - for (i = 0; i < 8; i++) {
> > + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> > if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
> > mask |= BIT(i);
> > }
> > @@ -1018,7 +1018,15 @@ static uint16_t igb_receive_assign(IGBCore
> *core, const struct eth_header *ehdr,
> > queues &= core->mac[VFRE];
> > igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL,
> rss_info);
> > if (rss_info->queue & 1) {
> > - queues <<= 8;
> > + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> > + if (!(queues & BIT(i))) {
> > + continue;
> > + }
> > + if (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE) {
> > + queues |= BIT(i + IGB_MAX_VF_FUNCTIONS);
> > + queues &= ~BIT(i);
> > + }
> > + }
> > }
> > } else {
> > switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
2023-01-30 12:07 ` Sriram Yagnaraman
@ 2023-01-30 13:20 ` Akihiko Odaki
0 siblings, 0 replies; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-30 13:20 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/30 21:07, Sriram Yagnaraman wrote:
>> -----Original Message-----
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Sent: Sunday, 29 January 2023 08:25
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
>> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Subject: Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
>>
>> On 2023/01/28 22:46, Sriram Yagnaraman wrote:
>>> RSS for VFs is only enabled if VMOLR[n].RSSE is set.
>>>
>>> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>>> ---
>>> hw/net/igb_core.c | 18 +++++++++++++-----
>>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
>>> 1eb7ba168f..e4fd4a1a5f 100644
>>> --- a/hw/net/igb_core.c
>>> +++ b/hw/net/igb_core.c
>>> @@ -69,7 +69,7 @@ typedef struct IGBTxPktVmdqCallbackContext {
>>>
>>> static ssize_t
>>> igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>>> - bool has_vnet, bool *assigned);
>>> + bool has_vnet, bool *external_tx);
>>
>> I admit external_tx is somewhat confusing, but it is more than just telling if it
>> is assigned to Rx queue or not. If external_tx is not NULL, it indicates it is part
>> of Tx packet switching. In that case, a bool value which describes whether the
>> packet should be routed to external LAN must be assigned. The value can be
>> false even if the packet is assigned to Rx queues; it will be always false if it is
>> multicast, for example.
>
> Yes, I undestand the purpose of external_tx. I merely changed the parameter name in the function declaration to match it's defintion. Anyhow, I will remove this change since it was purely comsetic.
The definition is wrong then. I'll submit the new version with it fixed.
>
>>
>>>
>>> static inline void
>>> igb_set_interrupt_cause(IGBCore *core, uint32_t val); @@ -942,7
>>> +942,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const
>>> struct eth_header *ehdr,
>>>
>>> if (core->mac[MRQC] & 1) {
>>> if (is_broadcast_ether_addr(ehdr->h_dest)) {
>>> - for (i = 0; i < 8; i++) {
>>> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
>>
>> I just left it as 8 because VMDq is not specific to VF. Perhaps it is better to
>> have another macro to denote the number of VMDq pools, but it is not done
>> yet.
>
> Ok, I agree. I will introduce a new IGB_MAX_VM_POOLS macro instead.
You don't need "MAX" as the number of pools is fixed if I understand it
correctly.
>
>>
>>> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
>>> queues |= BIT(i);
>>> }
>>> @@ -976,7 +976,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
>> const struct eth_header *ehdr,
>>> f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
>>> f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
>>> if (macp[f >> 5] & (1 << (f & 0x1f))) {
>>> - for (i = 0; i < 8; i++) {
>>> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
>>> if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
>>> queues |= BIT(i);
>>> }
>>> @@ -999,7 +999,7 @@ static uint16_t igb_receive_assign(IGBCore *core,
>> const struct eth_header *ehdr,
>>> }
>>> }
>>> } else {
>>> - for (i = 0; i < 8; i++) {
>>> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
>>> if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
>>> mask |= BIT(i);
>>> }
>>> @@ -1018,7 +1018,15 @@ static uint16_t igb_receive_assign(IGBCore
>> *core, const struct eth_header *ehdr,
>>> queues &= core->mac[VFRE];
>>> igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL,
>> rss_info);
>>> if (rss_info->queue & 1) {
>>> - queues <<= 8;
>>> + for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
>>> + if (!(queues & BIT(i))) {
>>> + continue;
>>> + }
>>> + if (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE) {
>>> + queues |= BIT(i + IGB_MAX_VF_FUNCTIONS);
>>> + queues &= ~BIT(i);
>>> + }
>>> + }
>>> }
>>> } else {
>>> switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/9] igb: add ICR_RXDW
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (4 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 5/9] igb: respect E1000_VMOLR_RSSE Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 7/9] igb: implement VF Tx and Rx stats Sriram Yagnaraman
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
IGB uses RXDW ICR bit to indicate that rx descriptor has been written
back. This is the same as RXT0 bit in older HW.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/e1000x_regs.h | 4 ++++
hw/net/igb_core.c | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
index bb3fb36b8d..3a3431d878 100644
--- a/hw/net/e1000x_regs.h
+++ b/hw/net/e1000x_regs.h
@@ -335,6 +335,7 @@
#define E1000_ICR_RXDMT0 0x00000010 /* rx desc min. threshold (0) */
#define E1000_ICR_RXO 0x00000040 /* rx overrun */
#define E1000_ICR_RXT0 0x00000080 /* rx timer intr (ring 0) */
+#define E1000_ICR_RXDW 0x00000080 /* rx desc written back */
#define E1000_ICR_MDAC 0x00000200 /* MDIO access complete */
#define E1000_ICR_RXCFG 0x00000400 /* RX /c/ ordered set */
#define E1000_ICR_GPI_EN0 0x00000800 /* GP Int 0 */
@@ -378,6 +379,7 @@
#define E1000_ICS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_ICS_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_ICS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_ICS_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_ICS_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_ICS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_ICS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
@@ -407,6 +409,7 @@
#define E1000_IMS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_IMS_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_IMS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_IMS_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_IMS_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_IMS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_IMS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
@@ -441,6 +444,7 @@
#define E1000_IMC_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_IMC_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_IMC_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_IMC_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_IMC_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_IMC_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_IMC_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index e4fd4a1a5f..43ff387b16 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1640,7 +1640,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
/* same as RXDW (rx descriptor written back)*/
- icr_bits |= E1000_ICR_RXT0;
+ icr_bits |= E1000_ICR_RXDW;
}
}
@@ -1650,7 +1650,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
e1000x_inc_reg_if_not_full(core->mac, ROC);
}
- if (icr_bits & E1000_ICR_RXT0) {
+ if (icr_bits & E1000_ICR_RXDW) {
trace_e1000e_rx_written_to_guest(icr_bits);
} else {
trace_e1000e_rx_not_written_to_guest(icr_bits);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 7/9] igb: implement VF Tx and Rx stats
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (5 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 6/9] igb: add ICR_RXDW Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 8/9] igb: respect VT_CTL ignore MAC field Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
8 siblings, 0 replies; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Please note that loopback counters for VM to VM traffic is not
implemented yet: VFGOTLBC, VFGPTLBC, VFGORLBC and VFGPRLBC.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 43ff387b16..375d9d5e34 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -488,7 +488,7 @@ igb_tx_pkt_send(IGBCore *core, struct igb_tx *tx, int queue_index)
}
static void
-igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt)
+igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt, int qn)
{
static const int PTCregs[6] = { PTC64, PTC127, PTC255, PTC511,
PTC1023, PTC1522 };
@@ -515,6 +515,14 @@ igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt)
core->mac[GPTC] = core->mac[TPT];
core->mac[GOTCL] = core->mac[TOTL];
core->mac[GOTCH] = core->mac[TOTH];
+
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+
+ core->mac[PVFGOTC0 + (pool * 64)] += tot_len;
+ core->mac[PVFGPTC0 + (pool * 64)]++;
+ }
}
static void
@@ -577,7 +585,7 @@ igb_process_tx_desc(IGBCore *core,
core->mac[VET] & 0xffff);
}
if (igb_tx_pkt_send(core, tx, queue_index)) {
- igb_on_tx_done_update_stats(core, tx->tx_pkt);
+ igb_on_tx_done_update_stats(core, tx->tx_pkt, queue_index);
}
}
@@ -1364,7 +1372,8 @@ igb_write_to_rx_buffers(IGBCore *core,
}
static void
-igb_update_rx_stats(IGBCore *core, size_t data_size, size_t data_fcs_size)
+igb_update_rx_stats(IGBCore *core, const E1000E_RingInfo *rxi,
+ size_t data_size, size_t data_fcs_size)
{
e1000x_update_rx_total_stats(core->mac, data_size, data_fcs_size);
@@ -1380,6 +1389,18 @@ igb_update_rx_stats(IGBCore *core, size_t data_size, size_t data_fcs_size)
default:
break;
}
+
+ if (core->mac[MRQC] & 1) {
+ uint16_t qn = rxi->idx;
+ uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+
+ core->mac[PVFGORC0 + (pool * 64)] += data_size + 4;
+ core->mac[PVFGPRC0 + (pool * 64)]++;
+ if (net_rx_pkt_get_packet_type(core->rx_pkt) == ETH_PKT_MCAST) {
+ core->mac[PVFMPRC0 + (pool * 64)]++;
+ }
+ }
}
static inline bool
@@ -1481,7 +1502,7 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
} while (desc_offset < total_size);
- igb_update_rx_stats(core, size, total_size);
+ igb_update_rx_stats(core, rxi, size, total_size);
}
static inline bool
@@ -1490,7 +1511,7 @@ igb_is_oversized(IGBCore *core, const E1000E_RingInfo *rxi, size_t size)
bool vmdq = core->mac[MRQC] & 1;
uint16_t qn = rxi->idx;
uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
- (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
bool lpe = (vmdq ? core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE :
core->mac[RCTL] & E1000_RCTL_LPE);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 8/9] igb: respect VT_CTL ignore MAC field
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (6 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 7/9] igb: implement VF Tx and Rx stats Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-28 13:46 ` [PATCH 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
8 siblings, 0 replies; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Also trace out a warning if replication mode is disabled, since we only
support replication mode enabled.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 9 +++++++++
hw/net/trace-events | 2 ++
2 files changed, 11 insertions(+)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 375d9d5e34..8e33e15505 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -949,6 +949,10 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
if (core->mac[MRQC] & 1) {
+ if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
+ trace_igb_rx_vmdq_replication_mode_disabled();
+ }
+
if (is_broadcast_ether_addr(ehdr->h_dest)) {
for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
@@ -995,6 +999,11 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
}
+ /* assume a full pool list if IGMAC is set */
+ if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
+ queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
+ }
+
if (e1000x_vlan_rx_filter_enabled(core->mac)) {
uint16_t mask = 0;
diff --git a/hw/net/trace-events b/hw/net/trace-events
index e94172e748..9bc7658692 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint3
igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
+igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication mode enabled is supported"
+
igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR enabled"
igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr) "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 9/9] igb: respect VMVIR and VMOLR for VLAN
2023-01-28 13:46 [PATCH 0/9] igb: add missing feature set from Sriram Yagnaraman
` (7 preceding siblings ...)
2023-01-28 13:46 ` [PATCH 8/9] igb: respect VT_CTL ignore MAC field Sriram Yagnaraman
@ 2023-01-28 13:46 ` Sriram Yagnaraman
2023-01-29 8:13 ` Akihiko Odaki
8 siblings, 1 reply; 20+ messages in thread
From: Sriram Yagnaraman @ 2023-01-28 13:46 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Add support for stripping/inserting VLAN for VFs.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 100 ++++++++++++++++++++++++++++++----------------
1 file changed, 65 insertions(+), 35 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 8e33e15505..96a5c5eca3 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -384,6 +384,26 @@ igb_rss_parse_packet(IGBCore *core, struct NetRxPkt *pkt, bool tx,
info->queue = E1000_RSS_QUEUE(&core->mac[RETA], info->hash);
}
+static inline bool
+igb_tx_insert_vlan(IGBCore *core, uint16_t qn,
+ struct igb_tx *tx, bool desc_vle)
+{
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+
+ if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_DEFAULT) {
+ /* always insert default VLAN */
+ desc_vle = true;
+ tx->vlan = core->mac[VMVIR0 + pool] & 0xfff;
+ } else if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_NEVER) {
+ return false;
+ }
+ }
+
+ return desc_vle && e1000x_vlan_enabled(core->mac);
+}
+
static bool
igb_setup_tx_offloads(IGBCore *core, struct igb_tx *tx)
{
@@ -580,7 +600,8 @@ igb_process_tx_desc(IGBCore *core,
if (cmd_type_len & E1000_TXD_CMD_EOP) {
if (!tx->skip_cp && net_tx_pkt_parse(tx->tx_pkt)) {
- if (cmd_type_len & E1000_TXD_CMD_VLE) {
+ if (igb_tx_insert_vlan(core, queue_index, tx,
+ (cmd_type_len & E1000_TXD_CMD_VLE))) {
net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt, tx->vlan,
core->mac[VET] & 0xffff);
}
@@ -1514,6 +1535,22 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
igb_update_rx_stats(core, rxi, size, total_size);
}
+static inline bool
+igb_rx_strip_vlan(IGBCore *core, const E1000E_RingInfo *rxi,
+ eth_pkt_types_e pkt_type)
+{
+ if (core->mac[MRQC] & 1) {
+ uint16_t qn = rxi->idx;
+ uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
+ (qn - IGB_MAX_VF_FUNCTIONS) : qn;
+ return (pkt_type == ETH_PKT_MCAST) ?
+ core->mac[RPLOLR] & E1000_RPLOLR_STRVLAN :
+ core->mac[VMOLR0 + pool] & E1000_VMOLR_STRVLAN;
+ }
+
+ return e1000x_vlan_enabled(core->mac);
+}
+
static inline bool
igb_is_oversized(IGBCore *core, const E1000E_RingInfo *rxi, size_t size)
{
@@ -1574,6 +1611,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
size_t total_size;
ssize_t retval = 0;
int i;
+ bool strip_vlan = false;
trace_e1000e_rx_receive_iov(iovcnt);
@@ -1615,10 +1653,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
ehdr = PKT_GET_ETH_HDR(filter_buf);
net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
-
- net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
- e1000x_vlan_enabled(core->mac),
- core->mac[VET] & 0xffff);
+ net_rx_pkt_set_protocols(core->rx_pkt, filter_buf, size);
queues = igb_receive_assign(core, ehdr, &rss_info, external_tx);
if (!queues) {
@@ -1626,8 +1661,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
return orig_size;
}
- total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
- e1000x_fcs_len(core->mac);
+ retval = orig_size;
+ total_size = size + e1000x_fcs_len(core->mac);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
if (!(queues & BIT(i))) {
@@ -1635,43 +1670,38 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
}
igb_rx_ring_init(core, &rxr, i);
+ strip_vlan = igb_rx_strip_vlan(core, rxr.i,
+ get_eth_packet_type(ehdr));
+ net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
+ strip_vlan, core->mac[VET] & 0xffff);
+ igb_rx_fix_l4_csum(core, core->rx_pkt);
+
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
icr_bits |= E1000_ICS_RXO;
+ continue;
}
- }
-
- if (!icr_bits) {
- retval = orig_size;
- igb_rx_fix_l4_csum(core, core->rx_pkt);
-
- for (i = 0; i < IGB_NUM_QUEUES; i++) {
- if (!(queues & BIT(i))) {
- continue;
- }
- igb_rx_ring_init(core, &rxr, i);
- if (igb_is_oversized(core, rxr.i, size)) {
- oversized |= BIT(i);
- continue;
- }
+ if (igb_is_oversized(core, rxr.i, total_size)) {
+ oversized |= BIT(i);
+ continue;
+ }
- if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
- continue;
- }
+ if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
+ continue;
+ }
- trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
- igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
+ trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
+ igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
- /* Check if receive descriptor minimum threshold hit */
- if (igb_rx_descr_threshold_hit(core, rxr.i)) {
- icr_bits |= E1000_ICS_RXDMT0;
- }
+ /* Check if receive descriptor minimum threshold hit */
+ if (igb_rx_descr_threshold_hit(core, rxr.i)) {
+ icr_bits |= E1000_ICS_RXDMT0;
+ }
- core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
+ core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
- /* same as RXDW (rx descriptor written back)*/
- icr_bits |= E1000_ICR_RXDW;
- }
+ /* same as RXDW (rx descriptor written back)*/
+ icr_bits |= E1000_ICR_RXDW;
}
/* 8.19.37 increment ROC only if packet is oversized for all queues */
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 9/9] igb: respect VMVIR and VMOLR for VLAN
2023-01-28 13:46 ` [PATCH 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
@ 2023-01-29 8:13 ` Akihiko Odaki
0 siblings, 0 replies; 20+ messages in thread
From: Akihiko Odaki @ 2023-01-29 8:13 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> Add support for stripping/inserting VLAN for VFs.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 100 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 65 insertions(+), 35 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 8e33e15505..96a5c5eca3 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -384,6 +384,26 @@ igb_rss_parse_packet(IGBCore *core, struct NetRxPkt *pkt, bool tx,
> info->queue = E1000_RSS_QUEUE(&core->mac[RETA], info->hash);
> }
>
> +static inline bool
> +igb_tx_insert_vlan(IGBCore *core, uint16_t qn,
> + struct igb_tx *tx, bool desc_vle)
> +{
> + if (core->mac[MRQC] & 1) {
> + uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
> + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
> +
> + if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_DEFAULT) {
> + /* always insert default VLAN */
> + desc_vle = true;
> + tx->vlan = core->mac[VMVIR0 + pool] & 0xfff;
This should be masked with 0xffff; "Port VLAN ID" field is defined as
16-bit.
> + } else if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_NEVER) {
> + return false;
> + }
> + }
> +
> + return desc_vle && e1000x_vlan_enabled(core->mac);
> +}
> +
> static bool
> igb_setup_tx_offloads(IGBCore *core, struct igb_tx *tx)
> {
> @@ -580,7 +600,8 @@ igb_process_tx_desc(IGBCore *core,
>
> if (cmd_type_len & E1000_TXD_CMD_EOP) {
> if (!tx->skip_cp && net_tx_pkt_parse(tx->tx_pkt)) {
> - if (cmd_type_len & E1000_TXD_CMD_VLE) {
> + if (igb_tx_insert_vlan(core, queue_index, tx,
> + (cmd_type_len & E1000_TXD_CMD_VLE))) {
The fourth parameter of igb_tx_insert_vlan() is bool, which is defined
as it only ensures it has storage of 1-bit, but this passes a value
greater than that.
> net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt, tx->vlan,
> core->mac[VET] & 0xffff);
> }
> @@ -1514,6 +1535,22 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
> igb_update_rx_stats(core, rxi, size, total_size);
> }
>
> +static inline bool
> +igb_rx_strip_vlan(IGBCore *core, const E1000E_RingInfo *rxi,
> + eth_pkt_types_e pkt_type)
> +{
> + if (core->mac[MRQC] & 1) {
> + uint16_t qn = rxi->idx;
> + uint16_t pool = (qn > IGB_MAX_VF_FUNCTIONS) ?
> + (qn - IGB_MAX_VF_FUNCTIONS) : qn;
> + return (pkt_type == ETH_PKT_MCAST) ?
> + core->mac[RPLOLR] & E1000_RPLOLR_STRVLAN :
> + core->mac[VMOLR0 + pool] & E1000_VMOLR_STRVLAN;
> + }
> +
> + return e1000x_vlan_enabled(core->mac);
> +}
> +
> static inline bool
> igb_is_oversized(IGBCore *core, const E1000E_RingInfo *rxi, size_t size)
> {
> @@ -1574,6 +1611,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> size_t total_size;
> ssize_t retval = 0;
> int i;
> + bool strip_vlan = false;
strip_vlan does not need a default value. Having a default value will
suppresss compiler warnings when you actually need to compute a valid value.
>
> trace_e1000e_rx_receive_iov(iovcnt);
>
> @@ -1615,10 +1653,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>
> ehdr = PKT_GET_ETH_HDR(filter_buf);
> net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
> -
> - net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
> - e1000x_vlan_enabled(core->mac),
> - core->mac[VET] & 0xffff);
> + net_rx_pkt_set_protocols(core->rx_pkt, filter_buf, size);
>
> queues = igb_receive_assign(core, ehdr, &rss_info, external_tx);
> if (!queues) {
> @@ -1626,8 +1661,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> return orig_size;
> }
>
> - total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
> - e1000x_fcs_len(core->mac);
> + retval = orig_size;
> + total_size = size + e1000x_fcs_len(core->mac);
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> if (!(queues & BIT(i))) {
> @@ -1635,43 +1670,38 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> }
>
> igb_rx_ring_init(core, &rxr, i);
> + strip_vlan = igb_rx_strip_vlan(core, rxr.i,
> + get_eth_packet_type(ehdr));
> + net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
> + strip_vlan, core->mac[VET] & 0xffff);
> + igb_rx_fix_l4_csum(core, core->rx_pkt);
> +
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> icr_bits |= E1000_ICS_RXO;
> + continue;
> }
> - }
> -
> - if (!icr_bits) {
> - retval = orig_size;
> - igb_rx_fix_l4_csum(core, core->rx_pkt);
> -
> - for (i = 0; i < IGB_NUM_QUEUES; i++) {
> - if (!(queues & BIT(i))) {
> - continue;
> - }
>
> - igb_rx_ring_init(core, &rxr, i);
> - if (igb_is_oversized(core, rxr.i, size)) {
> - oversized |= BIT(i);
> - continue;
> - }
> + if (igb_is_oversized(core, rxr.i, total_size)) {
> + oversized |= BIT(i);
> + continue;
> + }
>
> - if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
> - continue;
> - }
> + if (!(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
> + continue;
> + }
>
> - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> - igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
> + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> + igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
>
> - /* Check if receive descriptor minimum threshold hit */
> - if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> - icr_bits |= E1000_ICS_RXDMT0;
> - }
> + /* Check if receive descriptor minimum threshold hit */
> + if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> + icr_bits |= E1000_ICS_RXDMT0;
> + }
>
> - core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> + core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
>
> - /* same as RXDW (rx descriptor written back)*/
> - icr_bits |= E1000_ICR_RXDW;
> - }
> + /* same as RXDW (rx descriptor written back)*/
> + icr_bits |= E1000_ICR_RXDW;
> }
>
> /* 8.19.37 increment ROC only if packet is oversized for all queues */
^ permalink raw reply [flat|nested] 20+ messages in thread