mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michael Chan <mchan@broadcom.com>
To: David Miller <davem@davemloft.net>
Cc: <sergei.shtylyov@cogentembedded.com>, <natg@google.com>,
	<nsujir@broadcom.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0
Date: Wed, 11 Dec 2013 15:21:18 -0800	[thread overview]
Message-ID: <1386804078.1752.58.camel@LTIRV-MCHAN1.corp.ad.broadcom.com> (raw)
In-Reply-To: <20131210.222313.694303616856894486.davem@davemloft.net>

On Tue, 2013-12-10 at 22:23 -0500, David Miller wrote:
> Ok, that may be true, but I'd like to consider the much larger issue
> at hand.
> 
> If the indirect mechanism is enabled, some of the offsets that may be
> in there might be value, but would be entirely undesirable to be read
> because the read has side effects.
> 
> What if the interrupt status register is what gets read if the user
> scans config space at just the wrong moment, and therefore an
> interrupt gets lost?
> 
I did a quick audit of the driver to see if we are seriously exposed to
this problem.  Only very old chips (5700 to 5703) use indirect register
access as workarounds for various issues.  I looked at the IRQ path
which uses status block to determine if there is work to do.  If status
block contains no new data, we check the PCI_STATE register and the bits
don't get cleared on read.  MAC status register which is used to
determine link status does not have bits that are cleared on read.  So
we should be ok.

We use the statistics block for counters on these older chips so there
is no conflict.  On newer chips, we read counters from registers which
get cleared on read, but we never use indirect methods to read these
counters.

On all new chips, indirect method is not used except to download
firmware (on 57766 only).  Userspace read will not interfere with
firmware download.

So it looks to me that we are in good shape.  Thanks.


  reply	other threads:[~2013-12-11 23:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 18:43 Nat Gurumoorthy
2013-12-09 19:00 ` Michael Chan
2013-12-09 19:22   ` Natarajan Gurumoorthy
2013-12-09 21:18     ` Sergei Shtylyov
2013-12-09 21:07       ` Michael Chan
2013-12-10  6:58         ` Michael Chan
2013-12-10 15:01           ` Natarajan Gurumoorthy
2013-12-10 18:40             ` Michael Chan
2013-12-10 18:43           ` David Miller
2013-12-10 18:49             ` Michael Chan
2013-12-11  3:23               ` David Miller
2013-12-11 23:21                 ` Michael Chan [this message]
2013-12-12  0:35                   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1386804078.1752.58.camel@LTIRV-MCHAN1.corp.ad.broadcom.com \
    --to=mchan@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=natg@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=nsujir@broadcom.com \
    --cc=sergei.shtylyov@cogentembedded.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®