Skip to content

Commit

Permalink
x86/PCI: config space accessor functions should not ignore the segmen…
Browse files Browse the repository at this point in the history
…t argument

Without this change, the majority of the raw PCI config space access
functions silently ignore a non-zero segment argument, which is
certainly wrong.

Apart from pci_direct_conf1, all other non-MMCFG access methods get
used only for non-extended accesses (i.e. assigned to raw_pci_ops
only). Consequently, with the way raw_pci_{read,write}() work, it would
be a coding error to call these functions with a non-zero segment (with
the current call flow this cannot happen afaict).

The access method 1 accessor, as it can be used for extended accesses
(on AMD systems) instead gets checks added for the passed in segment to
be zero. This would be the case when on such a system having multiple
PCI segments (don't know whether any exist in practice) MMCFG for some
reason is not usable, and method 1 gets selected for doing extended
accesses. Rather than accessing the wrong device's config space, the
function will now error out.

v2: Convert BUG_ON() to WARN_ON(), and extend description as per Ingo's
request.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Reviewed-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
  • Loading branch information
Jan Beulich authored and Jesse Barnes committed Jul 22, 2011
1 parent 688398b commit db34a36
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 2 deletions.
2 changes: 2 additions & 0 deletions arch/x86/pci/ce4100.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ static int ce4100_conf_read(unsigned int seg, unsigned int bus,
{
int i;

WARN_ON(seg);
if (bus == 1) {
for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
if (bus1_fixups[i].dev_func == devfn &&
Expand All @@ -282,6 +283,7 @@ static int ce4100_conf_write(unsigned int seg, unsigned int bus,
{
int i;

WARN_ON(seg);
if (bus == 1) {
for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
if (bus1_fixups[i].dev_func == devfn &&
Expand Down
6 changes: 4 additions & 2 deletions arch/x86/pci/direct.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ static int pci_conf1_read(unsigned int seg, unsigned int bus,
{
unsigned long flags;

if ((bus > 255) || (devfn > 255) || (reg > 4095)) {
if (seg || (bus > 255) || (devfn > 255) || (reg > 4095)) {
*value = -1;
return -EINVAL;
}
Expand Down Expand Up @@ -53,7 +53,7 @@ static int pci_conf1_write(unsigned int seg, unsigned int bus,
{
unsigned long flags;

if ((bus > 255) || (devfn > 255) || (reg > 4095))
if (seg || (bus > 255) || (devfn > 255) || (reg > 4095))
return -EINVAL;

raw_spin_lock_irqsave(&pci_config_lock, flags);
Expand Down Expand Up @@ -97,6 +97,7 @@ static int pci_conf2_read(unsigned int seg, unsigned int bus,
unsigned long flags;
int dev, fn;

WARN_ON(seg);
if ((bus > 255) || (devfn > 255) || (reg > 255)) {
*value = -1;
return -EINVAL;
Expand Down Expand Up @@ -138,6 +139,7 @@ static int pci_conf2_write(unsigned int seg, unsigned int bus,
unsigned long flags;
int dev, fn;

WARN_ON(seg);
if ((bus > 255) || (devfn > 255) || (reg > 255))
return -EINVAL;

Expand Down
2 changes: 2 additions & 0 deletions arch/x86/pci/numaq_32.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ static int pci_conf1_mq_read(unsigned int seg, unsigned int bus,
unsigned long flags;
void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));

WARN_ON(seg);
if (!value || (bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
return -EINVAL;

Expand Down Expand Up @@ -73,6 +74,7 @@ static int pci_conf1_mq_write(unsigned int seg, unsigned int bus,
unsigned long flags;
void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));

WARN_ON(seg);
if ((bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
return -EINVAL;

Expand Down
4 changes: 4 additions & 0 deletions arch/x86/pci/olpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ static int pci_olpc_read(unsigned int seg, unsigned int bus,
{
uint32_t *addr;

WARN_ON(seg);

/* Use the hardware mechanism for non-simulated devices */
if (!is_simulated(bus, devfn))
return pci_direct_conf1.read(seg, bus, devfn, reg, len, value);
Expand Down Expand Up @@ -264,6 +266,8 @@ static int pci_olpc_read(unsigned int seg, unsigned int bus,
static int pci_olpc_write(unsigned int seg, unsigned int bus,
unsigned int devfn, int reg, int len, uint32_t value)
{
WARN_ON(seg);

/* Use the hardware mechanism for non-simulated devices */
if (!is_simulated(bus, devfn))
return pci_direct_conf1.write(seg, bus, devfn, reg, len, value);
Expand Down
2 changes: 2 additions & 0 deletions arch/x86/pci/pcbios.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ static int pci_bios_read(unsigned int seg, unsigned int bus,
unsigned long flags;
unsigned long bx = (bus << 8) | devfn;

WARN_ON(seg);
if (!value || (bus > 255) || (devfn > 255) || (reg > 255))
return -EINVAL;

Expand Down Expand Up @@ -247,6 +248,7 @@ static int pci_bios_write(unsigned int seg, unsigned int bus,
unsigned long flags;
unsigned long bx = (bus << 8) | devfn;

WARN_ON(seg);
if ((bus > 255) || (devfn > 255) || (reg > 255))
return -EINVAL;

Expand Down

0 comments on commit db34a36

Please sign in to comment.