[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v3 06/47] igb: Clear IMS bits when committing ICR access
From: |
Sriram Yagnaraman |
Subject: |
RE: [PATCH v3 06/47] igb: Clear IMS bits when committing ICR access |
Date: |
Mon, 24 Apr 2023 11:23:15 +0000 |
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Monday, 24 April 2023 12:52
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: Jason Wang <jasowang@redhat.com>; Dmitry Fleytman
> <dmitry.fleytman@gmail.com>; Michael S . Tsirkin <mst@redhat.com>; Alex
> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>; Paolo
> Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org; Tomasz Dzieciol
> <t.dzieciol@partner.samsung.com>
> Subject: Re: [PATCH v3 06/47] igb: Clear IMS bits when committing ICR access
>
> On 2023/04/24 18:29, Sriram Yagnaraman wrote:
> >> -----Original Message-----
> >> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> Sent: Sunday, 23 April 2023 06:18
> >> Cc: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
> >> <jasowang@redhat.com>; Dmitry Fleytman <dmitry.fleytman@gmail.com>;
> >> Michael S . Tsirkin <mst@redhat.com>; Alex Bennée
> >> <alex.bennee@linaro.org>; Philippe Mathieu-Daudé <philmd@linaro.org>;
> >> Thomas Huth <thuth@redhat.com>; Wainer dos Santos Moschetta
> >> <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>; Cleber Rosa
> >> <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>; Paolo
> >> Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org; Tomasz
> Dzieciol
> >> <t.dzieciol@partner.samsung.com>; Akihiko Odaki
> >> <akihiko.odaki@daynix.com>
> >> Subject: [PATCH v3 06/47] igb: Clear IMS bits when committing ICR
> >> access
> >>
> >> The datasheet says contradicting statements regarding ICR accesses so
> >> it is not reliable to determine the behavior of ICR accesses.
> >> However, e1000e does clear IMS bits when reading ICR accesses and
> >> Linux also expects ICR accesses will clear IMS bits according to:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tr
> >> ee/drivers/
> >> net/ethernet/intel/igb/igb_main.c?h=v6.2#n8048
> >>
> >> Fixes: 3a977deebe ("Intrdocue igb device emulation")
> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> ---
> >> hw/net/igb_core.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> >> 96a118b6c1..eaca5bd2b6 100644
> >> --- a/hw/net/igb_core.c
> >> +++ b/hw/net/igb_core.c
> >> @@ -2452,16 +2452,16 @@ igb_set_ims(IGBCore *core, int index,
> >> uint32_t
> >> val) static void igb_commit_icr(IGBCore *core) {
> >> /*
> >> - * If GPIE.NSICR = 0, then the copy of IAM to IMS will occur only if
> >> at
> >> + * If GPIE.NSICR = 0, then the clear of IMS will occur only if
> >> + at
> >> * least one bit is set in the IMS and there is a true interrupt as
> >> * reflected in ICR.INTA.
> >> */
> >> if ((core->mac[GPIE] & E1000_GPIE_NSICR) ||
> >> (core->mac[IMS] && (core->mac[ICR] & E1000_ICR_INT_ASSERTED))) {
> >> - igb_set_ims(core, IMS, core->mac[IAM]);
> >> - } else {
> >> - igb_update_interrupt_state(core);
> >> + igb_clear_ims_bits(core, core->mac[IAM]);
> >> }
> >> +
> >> + igb_update_interrupt_state(core);
> >> }
> >>
> >> static void igb_set_icr(IGBCore *core, int index, uint32_t val)
> >> --
> >> 2.40.0
> >
> > Reviewed-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> >
> > An additional question, should ICR be cleared if an actual interrupt was
> asserted?
> > (According to 7.3.2.11 GPIE: Non Selective Interrupt clear on read:
> > When set, every read of ICR clears it. When this bit is cleared, an ICR read
> causes it to be cleared only if an actual interrupt was asserted or IMS = 0b.)
> Something like this?
>
> That is handled in igb_commit_icr(), which is renamed to igb_nsicr() in patch
> "igb: Notify only new interrupts".
>
Mm, I must be missing something, but I still don't see the ICR bits being
cleared igb_commit_icr/igb_nsicr().
For e.g. e1000e_mac_icr_read does this explicitly:
if ((core->mac[ICR] & E1000_ICR_ASSERTED) &&
(core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME)) {
trace_e1000e_irq_icr_clear_iame();
core->mac[ICR] = 0;
trace_e1000e_irq_icr_process_iame();
e1000e_clear_ims_bits(core, core->mac[IAM]);
}
> >
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > eaca5bd2b6..aaaf80e4a3 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -2529,6 +2529,9 @@ igb_mac_icr_read(IGBCore *core, int index)
> > } else if (core->mac[IMS] == 0) {
> > trace_e1000e_irq_icr_clear_zero_ims();
> > core->mac[ICR] = 0;
> > + } else if (core->mac[ICR] & E1000_ICR_INT_ASSERTED) {
> > + e1000e_irq_icr_clear_iame();
> > + core->mac[ICR] = 0;
> > } else if (!msix_enabled(core->owner)) {
> > trace_e1000e_irq_icr_clear_nonmsix_icr_read();
> > core->mac[ICR] = 0;
- [PATCH v3 00/47] igb: Fix for DPDK, Akihiko Odaki, 2023/04/23
- [PATCH v3 01/47] hw/net/net_tx_pkt: Decouple implementation from PCI, Akihiko Odaki, 2023/04/23
- [PATCH v3 02/47] hw/net/net_tx_pkt: Decouple interface from PCI, Akihiko Odaki, 2023/04/23
- [PATCH v3 03/47] e1000x: Fix BPRC and MPRC, Akihiko Odaki, 2023/04/23
- [PATCH v3 04/47] igb: Fix Rx packet type encoding, Akihiko Odaki, 2023/04/23
- [PATCH v3 05/47] igb: Do not require CTRL.VME for tx VLAN tagging, Akihiko Odaki, 2023/04/23
- [PATCH v3 06/47] igb: Clear IMS bits when committing ICR access, Akihiko Odaki, 2023/04/23
[PATCH v3 07/47] net/net_rx_pkt: Use iovec for net_rx_pkt_set_protocols(), Akihiko Odaki, 2023/04/23
[PATCH v3 08/47] e1000e: Always copy ethernet header, Akihiko Odaki, 2023/04/23
[PATCH v3 09/47] igb: Always copy ethernet header, Akihiko Odaki, 2023/04/23
[PATCH v3 10/47] Fix references to igb Avocado test, Akihiko Odaki, 2023/04/23
[PATCH v3 11/47] tests/avocado: Remove unused imports, Akihiko Odaki, 2023/04/23
[PATCH v3 12/47] tests/avocado: Remove test_igb_nomsi_kvm, Akihiko Odaki, 2023/04/23
[PATCH v3 13/47] hw/net/net_tx_pkt: Remove net_rx_pkt_get_l4_info, Akihiko Odaki, 2023/04/23
[PATCH v3 14/47] net/eth: Rename eth_setup_vlan_headers_ex, Akihiko Odaki, 2023/04/23