[PATCH 2/2] I/O port access routines

[Adam Jackson]

On Thu, 2009-11-19 at 00:37 +0100, Luc Verhaegen wrote:

> And i just keep on wondering... Just like with the VGA arbitration... 
> Didn't the RAC used wrap all this crap for us _outside_ calls to driver 
> code so that we could just access IO and memory directly, without 
> having to care at all?

RAC did not handle this correctly when multiple X servers were running;
VGA arb does.  The I/O port routines in the server - not strictly part
of RAC - leave the onus of domain safety on the driver, and are a
wretched pile of ifdef (though admittedly a working pile).

The deeper assumption with RAC was that the OS could not be trusted to
give you a working PCI memory map, and that it was okay to rewrite it.
No no no no no.

> How hard would it have been to have written up a 
> backend for the RAC for the linux pci code that popped up during the 
> active life of RAC? My guess is, much easier than the stuff that has to 
> be pulled now, all it took was that someone was willing to go into 
> existing code and understand how it work(s/ed).

The Linux PCI code did use RAC.  I don't know why you think it didn't?

As someone who's actually had to fix bugs in the PCI code in 6.8:
while: 

- the transition to the new library was ungraceful
- the motivation for switching was a complete MacGuffin
- the internal conversion to pciaccess is still incomplete, and
- the completion of VGA arbitration took embarassingly long

I can honestly and without reservation say that the current code is
comprehensible and pleasant to work with, and the old PCI layer has
literally woken me from sleep sweating in terror.

> Oh, and that's before i start wondering about gratuitous void pointers 
> and goto.

The handle needs to be opaque to the user, because it's intrinsically
platform-dependent.  Kristian's opaque struct name suggestion is
probably what I'll go with.

In this particular case, yeah, the goto should reasonably be thus:

    /* First check if there's a legacy io method for the device */
    while (dev) {
        snprintf(name, PATH_MAX, "/sys/class/pci_bus/%04x:%02x/legacy_io",
                 dev->domain, dev->bus);

        ret = open(name, O_RDWR);
        if (ret >= 0)
            break;

        dev = pci_device_get_parent_bridge(dev);
    }

    /* If not, /dev/port is the best we can do */
    if (!dev)
        ret = open("/dev/port", O_RDWR);

    if (ret == 0) {
        int new = dup(ret);
        close(ret);
        ret = new;
    }

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
Url : http://lists.x.org/archives/xorg-devel/attachments/20091119/932fcb43/attachment.pgp