Bug 5000

Created attachment 3762 [details] [review]
Patch to fix PCI domain support for Altix

This patch works on Altix, and should fall back correctly on other platforms.

egbert: any comments on this, or is it okay to apply?

Created attachment 4139 [details] [review]
revised PCI domain support patch for Altix

This is the revised patch (from Egbert and Andreas) that SUSE took.

For X.org after modularization the patches had to be revised and several bugs to
be fixed. Currently this is tracked in Novell's bugzilla under

  https://bugzilla.novell.com/show_bug.cgi?id=197190

I will upstream the patches as soon as I'm confident that we fixed all issues
and do not have any regressions WRT other architectures.

Except for a patch for CE bridges that looks too scary to unconditionally check
it in everything is now in git.

Created attachment 7713 [details] [review]
Patch for supporting CE-X for altix

This is the patch that looks a bit suspicious.

The changes to altixPCI.c can, at worst, only break altix, so, okay.

The change to linuxOpenLegacy I'm not sure about.

The change to xf86MapDomainMemory looks plausible.

I have no idea why the change to ia64LinuxFindPciNext should be necessary.  Do
we really not have any generic code for handling that?  Also, are we sure busses
are going to be numbered consecutively, because if not then the pciNumBusses
assignment looks pretty bogus.

(In reply to comment #7)
> The change to linuxOpenLegacy I'm not sure about.

Me neither. We're currently in beta phase, so I don't want to apply something
like this to the Xserver unconditionally ATM. Maybe for an alpha of openSUSE, so
it can be tested widely at least for x86 before upstreaming, to approve that it
has no side effect.

> I have no idea why the change to ia64LinuxFindPciNext should be necessary.  Do
> we really not have any generic code for handling that?  Also, are we sure busses
> are going to be numbered consecutively, because if not then the pciNumBusses
> assignment looks pretty bogus.

I have no clue. I think I should ask sgi first whether we have a machine
in-house, and test the Xserver w/o this part of the patch first. Though I guess
it is necessary. It should probably be more robust before upstreaming.

Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.

(In reply to comment #7)
> The change to linuxOpenLegacy I'm not sure about.

If you mean this change:

-       pBusInfo = pciBusInfo[bus];
+       pBusInfo = pciBusInfo[PCI_BUS_FROM_TAG(Tag)];

where bus = PCI_BUS_NO_DOMAIN(PCI_BUS_FROM_TAG(Tag))

I'm guessing it's because in getnextbus(), pciBusInfo[] is indexed using
dombus = PCI_MAKE_BUS(*domain, *bus), and in ia64linuxPciFindNext(), it's
indexed using sec_bus = PCI_SECONDARY_BUS_EXTRACT(bridge_reg, pciDeviceTag);
both account for the domain.

> I have no idea why the change to ia64LinuxFindPciNext should be necessary.
> Do we really not have any generic code for handling that?

That change is very similar to the PCI_BRIDGE_SUPPORT section in Pci.c; seems
that it could easily be copied over to linuxPci.c.

Created attachment 9918 [details] [review]
revised patch to linuxPci.c (PCI domain support for Altix)

Note that the change to ia64linuxPciFindNext() is basically what's
in the PCI_BRIDGE_SUPPORT ifdef in pciGenFindNext() in Pci.c.

(In reply to comment #11)
> Created an attachment (id=9918) [details]
> revised patch to linuxPci.c (PCI domain support for Altix)

The diff above is based on the latest revision in the git tree as of today.
I've tested it on Altix 450 and 4700 with the ATI FireMV 2200 graphics card
in the base IO blade.

(In reply to comment #12)
> (In reply to comment #11)
> > Created an attachment (id=9918) [details] [details]
> > revised patch to linuxPci.c (PCI domain support for Altix)
> 
> The diff above is based on the latest revision in the git tree as of today.
> I've tested it on Altix 450 and 4700 with the ATI FireMV 2200 graphics card
> in the base IO blade.

This looks fine to me.  Matthias or Egbert, any comment?

Matthias is on LinuxTag this week.

Looks good. The bridge code argument is reasonable as well.

No more complaints -> finally pushed into git.