From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH 2/2] PCI: Stop clearing bridge Secondary Status when setting up I/O aperture
Date: Mon, 9 Dec 2013 18:31:36 -0700 [thread overview]
Message-ID: <20131210013136.GO5429@obsidianresearch.com> (raw)
In-Reply-To: <CAErSpo63eRPzXeQ94XTR3T7PDoQUjTmqfwJKn2EKSNPi3Jyc0Q@mail.gmail.com>
On Mon, Dec 09, 2013 at 05:18:30PM -0700, Bjorn Helgaas wrote:
> > Basically, your fix to pci_setup_bridge_io is fine - but your
> > observation led me to realize that the HW drivers implementing RMW for
> > 8 and 16 bit ops under their struct pci_ops.read function have exactly
> > the same flaw you are fixing here - they will silently wipe out write
> > 1 to clear bits.
>
> Ah, OK. I thought you were saying that we couldn't change
> pci_setup_bridge_io() to use 16-bit accesses because of this problem.
No, not really - just that this bug you discovered is broader than just
that one place :)
> Sure, if somebody can come up with a reasonable way to share this sort
> of code, that sounds like a good thing. Maybe extending struct
> pci_ops is the way to do this, but I hope not, because it seems like
Well, sharing the code is no problem, it is exactly the same for every
32 bit only host driver.
But supporting RW1C bits in capability lists seems fairly hard once
the context at the call site is lost.
Which makes me wonder if supporting sub-dword writes to dwords with
RW1C bits makes sense at all :(
That was why my initial reaction was to not do sub dword writes if you
know it will conflict with a RW1C bit.
Jason
next prev parent reply other threads:[~2013-12-10 1:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-06 0:19 [PATCH 0/2] Bridge window discovery and setup fixes Bjorn Helgaas
2013-12-06 0:19 ` [PATCH 1/2] PCI: Prevent bus conflicts while checking for bridge apertures Bjorn Helgaas
2013-12-09 19:31 ` Jason Gunthorpe
2013-12-09 20:00 ` Bjorn Helgaas
2013-12-09 20:15 ` Jason Gunthorpe
2013-12-06 0:19 ` [PATCH 2/2] PCI: Stop clearing bridge Secondary Status when setting up I/O aperture Bjorn Helgaas
2013-12-09 19:33 ` Jason Gunthorpe
2013-12-09 20:15 ` Bjorn Helgaas
2013-12-09 21:27 ` Jason Gunthorpe
2013-12-09 21:58 ` Bjorn Helgaas
2013-12-10 0:10 ` Jason Gunthorpe
2013-12-10 0:18 ` Bjorn Helgaas
2013-12-10 1:31 ` Jason Gunthorpe [this message]
2013-12-10 18:22 ` Bjorn Helgaas
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=20131210013136.GO5429@obsidianresearch.com \
--to=jgunthorpe@obsidianresearch.com \
--cc=bhelgaas@google.com \
--cc=linux-pci@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).