Bug 197572

Created attachment 95362 [details]
p_pci-domain.diff

Created attachment 95365 [details]
p_pci-ia64.diff

Created attachment 95366 [details]
p_pci-ce-x.diff

Created attachment 95367 [details]
p_mappciBIOS_complete.diff

Created attachment 95368 [details]
p_pci-legacy-mmap.diff

All these patches need to applied to package xorg-x11-server.

Looks like that "p_mappciBIOS_complete.diff" is already upstream. Only the last hunk in hw/xfree86/os-support/linux/lnx_pci.c is handled slighly different.

I've added the patches now to our xorg-x11-server package, but *disabled* them.

Created attachment 96794 [details]
Fixed p_pci-domain.diff

I have (sort-of) guessed the additions to /sys related device file (link) changes (is26), but it seems to correlate.
I also assume that linuxPciHandleBIOS() (new) needs PCI_BUS_NO_DOMAIN() handling.
Fixed several potential buffer overflows due to domain handling.

xf86GetPciHostConfigFromTag() has no PCI_BUS_NO_DOMAIN() handling, but never had so far. Don't know the potential impact here.

Fixed some potential index overflow handling (academic).
Completely rewrote lnx_pci.c device scanning. This needs testing badly!

Created attachment 96866 [details]
The hunks in bus/Pci.h were reverted.

But the new p_pci-domain.diff still needs some improvement. :-)

lnx_pci.c: In function 'xf86OSLinuxGetPciDevs':
lnx_pci.c:46: error: expected '{' before '*' token
lnx_pci.c:54: error: 'dirent1' undeclared (first use in this function)
lnx_pci.c:54: error: (Each undeclared identifier is reported only once
lnx_pci.c:54: error: for each function it appears in.)
lnx_pci.c:59: error: 'dirent2' undeclared (first use in this function)
lnx_pci.c:75: warning: format '%08x' expects type 'unsigned int', but argument 7 has type 'long unsigned int'
lnx_pci.c:75: warning: format '%08x' expects type 'unsigned int', but argument 8 has type 'long unsigned int'
lnx_pci.c: In function 'xf86GetPciSizeFromOS':
lnx_pci.c:160: warning: unused variable 'fn'
lnx_pci.c:160: warning: unused variable 'dev'
lnx_pci.c: In function 'xf86GetPciOffsetFromOS':
lnx_pci.c:196: warning: unused variable 'fn'
lnx_pci.c:196: warning: unused variable 'dev'
lnx_pci.c: In function 'xf86GetOSOffsetFromPCI':
lnx_pci.c:224: warning: unused variable 'fn'
lnx_pci.c:224: warning: unused variable 'dev'
make[5]: *** [lnx_pci.lo] Error 1

Created attachment 96867 [details]
fixed compilation

Still remaining:

lnx_pci.c: In function 'xf86OSLinuxGetPciDevs':
lnx_pci.c:48: warning: 'i' may be used uninitialized in this function

+    unsigned int i, num, devfn;
[...]
+                           if ( (file = fopen (c, "r")) ) {
+                               i = 0;
+                               while (i < 7 && fgets (c, 0x200, file)) {
+                                   if (sscanf (c, PCIADDR_FMT " " PCIADDR_FMT " " PCIADDR_IGNORE_FMT, &begin, &end) == 2) {
+                                       tmp->offset[i] = begin;
+                                       tmp->size[i]   = end - begin;
+                                       i++;
+                                   }
+                               }
+                               fclose (file);
+                           }
+                           if (i > 0) {
+                               tmp->next = ret;
+                               ret       = tmp;
+                           } else
+                               xfree (tmp);

Should we better initialize it with "i=0" in the decleration?

Created attachment 96898 [details]
Hopefully fixed patch

p_pci-ia64.diff:
- applies smoothly and looks ok to me.

p_mappciBIOS_complete.diff:
> Looks like that "p_mappciBIOS_complete.diff" is already upstream. Only the
> last hunk in hw/xfree86/os-support/linux/lnx_pci.c is handled slighly
> different.
Indeed. This patch is already upstream including the last hunk for lnx_pci.c!

Remaining patches for investigation:
- p_pci-ce-x.diff (adjust/review)
- p_pci-legacy-mmap.diff (review, apply also for non-IA64 platforms?)

Created attachment 97000 [details]
Xorg.0.log with PCI domain patch applied.

Looks like there's still some room for improvement. :-)

Created attachment 97003 [details]
Xorg.0.log without PCI domain patch

For comparison the logfile of an Xserver with working PCI scan code. :-)

Created attachment 97027 [details]
Updated patch, using different kernel interface

This patch uses a slightly different kernel interface (/sys/bus/pci/devices instead of /sys/devices), as the later one seems to have a *big* kernel bug involved.

I'm not marking the old patch as obsolete, because it is the way it should be done, but we cannot use it ATM. I already asked for the support level of /sys/bus/pci (whether it's already depricated or what), but ATM this seems to work correctly.

BTW - on ia64 /sys/devices seems to work correctly.

Created attachment 97046 [details]
Updated p_pci-ce-x.diff patch

Nuked unnecessary changes.
Updated to match 7.1.

p_pci-legacy-mmap.diff should currently only be applied for ia64. Implications for other platforms are still unclear, and the patch is trivial to maintain.

The patch from comment #18 still has some debugging output enabled (one ErrorF).
This should of course be removed in the final version.

Created attachment 97112 [details]
Final p_pci-domain patch

This patch finally fixes all issues remaining, verfied with the nv driver. The number of available cards wasn't counted, and the pci resource size was off by one.

The new system still differs in the data in its internal data structure, as in /sys the pci addresses are the real addresses, while the /proc addresses contain flags (io/mem, 32/64bit, prfetchable) in the lowest 2 or 4 bits for io and memory, repsectively. 

These flags are saved internally and removed by checking the flags of some external magic address in xf86GetOSOffsetFromPCI. The new system doesn't save these bits, but due to the masking the final results are the same to the caller of xf86GetOSOffsetFromPCI.

Comment on attachment 95367 [details]
p_mappciBIOS_complete.diff

already included upstream

Thanks a lot, Matthias!

I've tested the new PCI domain patch (+ PCI-PCI bridge patch) on shannon (works fine!) and submitted a new xorg-x11-server package for STABLE. These patches also fixes the PCI scanning on IA64. On this platform we now see different problems (see Bug #197190), but we no longer handle these in this bugreport, since they seem to be unrelated. Finally closing as FIXED. :-)

Reopening - some patches are broken.

Attaching new ones.

Created attachment 100972 [details]
New final p_pci-domain patch

Fixes:

- internal domain number of by one (was supposed to be a
  cleanup, but other code dependet on this semantics)
- fixed another long-standing of-by-1 error

Created attachment 100983 [details]
Additional patch p_enable-altrix.diff

This additional patch enables the build of the altrix detection routines, which have apparently not been included in Xorg 7.1 yet.

This patch needs a autoreconf -fi after application.

Please commit :)

Thanks! Will do so.

Created attachment 101011 [details]
Again - p_enable-altrix.diff

Sorry, missing include directory...

New patches applied now.