RE: [PATCH 2/3] PCI: Handle device quirks when accessing sysfsresource<N> entries

[Elliott, Robert (Server Storage)]

> On Fri, Mar 22, 2013 at 9:55 AM, Robert Hancock <hancockrwd@xxxxxxxxx>
> wrote:
> > On Fri, Mar 22, 2013 at 9:39 AM, Myron Stowe <myron.stowe@xxxxxxxxx>
> wrote:
> >> On Thu, Mar 21, 2013 at 6:51 PM, Robert Hancock
> <hancockrwd@xxxxxxxxx> wrote:
> >>> On 03/20/2013 10:35 PM, Myron Stowe wrote:
> >>>>
> >>>> Sysfs includes entries to memory regions that back a PCI device's BARs.
> >>>> The pci-sysfs entries backing I/O Port BARs can be accessed by userspace,
> >>>> providing direct access to the device's registers.  File permissions
> >>>> prevent random users from accessing the device's registers through these
> >>>> files, but don't stop a privileged app that chooses to ignore the purpose
> >>>> of these files from doing so.
> >>>>
> >>>> There are devices with abnormally strict restrictions with respect to
> >>>> accessing their registers; aspects that are typically handled by the
> >>>> device's driver.  When these access restrictions are not followed - as
> >>>> when a userspace app such as "udevadm info --attribute-walk
> >>>> --path=/sys/..." parses though reading all the device's sysfs entries - it
> >>>> can cause such devices to fail.
> >>>>
> >>>> This patch introduces a quirking mechanism that can be used to detect
> >>>> accesses that do no meet the device's restrictions, letting a device
> >>>> specific method intervene and decide how to progress.
> >>>>
> >>>> Reported-by: Xiangliang Yu <yuxiangl@xxxxxxxxxxx>
> >>>> Signed-off-by: Myron Stowe <myron.stowe@xxxxxxxxxx>
> >>>
> >>>
> >>> I honestly don't think there's much point in even attempting this strategy.
> >>> This list of devices in the quirk can't possibly be complete. It would
> >>> likely be easier to enumerate a white-list of devices that can deal with
> >>> their IO ports being read willy-nilly than a blacklist of those that don't,
> >>> as there's likely countless devices that fall into this category. Even if
> >>> they don't choke as badly as these ones do, it's quite likely that bad
> >>> behavior will result.
> >>
> >> For the device in question, it seems abnormally restrictive in its
> >> access limitations to BAR1 and BAR3.  The device reserves 4 Bytes of
> >> I/O Port space for these BARs, which is likely based on PCI's DWord
> >> based protocol, but "chokes (we still have not received any specifics
> >> on this yet)" on any access other than a single Byte access at offset
> >> 0x2 (x86 supports 1, 2, and 4 Byte I/O Port accesses).  This seems to
> >> imply that the device did not back the other three reserved Bytes it
> >> claims in any way, which again, seems peculiar to this particular
> >> device as other similar devices tend to back the reserved bytes they
> >> claim and return 0's when accessed.
> >
> > I don't think it's really that unusual. IO ports are weird, they're
> > not necessarily emulating any kind of normal memory - doing a 32-bit
> > access on IO port 0 isn't necessarily equivalent at all to doing
> > separate 8-bit accesses on IO ports 0, 1, 2 and 3 for example. They
> > can do completely different things. For legacy SFF ATA controllers,
> > IIRC most of the IO ports only expect 8-bit accesses other than the
> > PIO data register which can do 16 or 32-bit accesses (which just gives
> > you a different amount of data when doing different size reads from
> > the same location - it doesn't give you the data for the IO ports 2 or
> > 3 bytes ahead of that when doing a 32-bit read, like you might expect
> > if you were thinking of it like memory).
> 
> Very good point; I did have a more memory based mindset.
> 
> > So it's not too shocking that
> > the designer of these Marvell devices didn't pay too much attention to
> > what happens if you access the ports in an unexpected way.
> >
> > ...
> >>
> >> ...
> 
> So this suggests that we will never be able to deal with a userspace
> app accessing device registers in a unknowledgable, random, fashion -
> the exact situation we are currently in today.  We could get to the
> point of supporting UIO drivers, which would be more like device
> drivers and have intimate detailed knowledge of what they are driving
> and which would also introduce some of the mutual-exclusive ownership
> issues we have talked about, but not unknowing userspace apps.
> 

The PCI Express specification allows devices to choose what types of accesses they support to the 
IO or memory address spaces provided by their BARs:

Excerpt from PCI Express Base Specification 3.0 page 109:
" When a device's programming model restricts (vs. what is otherwise permitted in PCI Express) the
characteristics of a Request, that device is permitted to "Completer Abort" any Requests which
violate the programming model. Examples include unaligned or wrong-size access to a register
block and unsupported size of request to a Memory Space.

Generally, devices are able to assume a restricted programming model when all communication will
be between the device's driver software and the device itself. Devices which may be accessed
directly by operating system software or by applications which may not comprehend the restricted
programming model of the device (typically devices which implement legacy capabilities) should be
designed to support all types of Requests which are possible in the existing usage model for the
device. If this is not done, the device may fail to operate with existing software."

How the system handles errors like Completer Aborts will vary - NMIs, hangs, or even processing 
bogus data are all possible.

There are some baseline rules on configuration space accesses (so generic PCI topology software
can work with all PCI cards), but not IO and memory space.  It's prudent to not generate IO
or memory accesses you don't know are supposed to work.

---
Rob Elliott    HP Server Storage



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/