Bug 8221

Summary: Cannot copy video BIOS
Product: xorg Reporter: Eiichiro Oiwa <eiichiro.oiwa.nm>
Component: Server/DDX/XorgAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED WORKSFORME QA Contact:
Severity: blocker    
Priority: high CC: ajax, eich, jbarnes, mat, sndirsch
Version: 7.1 (2006.05)Keywords: patch
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description Eiichiro Oiwa 2006-09-10 23:16:23 UTC
If PCI VGA BIOS image is empty or invalid and video BIOS integrated into system
BIOS, xf86ReadDomainMemory function cannot copy video BIOS from System BIOS. In
this situation, the size of the rom sysfs is set to zero by Linux kernel and
Xorg cannot start. Therefore xf86ReadDomainMemory function should be:
--- xorg-server-X11R7.1-1.1.0.org/hw/xfree86/os-support/bus/linuxPci.c 
2006-04-29 07:28:56.000000000 +0900
+++ xorg-server-X11R7.1-1.1.0/hw/xfree86/os-support/bus/linuxPci.c     
2006-09-11 15:02:02.734375000 +0900
@@ -712,7 +712,7 @@ _X_EXPORT int
 xf86ReadDomainMemory(PCITAG Tag, ADDRESS Base, int Len, unsigned char *Buf)
 {
     unsigned char *ptr, *src;
-    ADDRESS offset;
+    ADDRESS offset, TmpBase;
     unsigned long size;
     int len, pagemask = getpagesize() - 1;
 
@@ -733,6 +733,7 @@ xf86ReadDomainMemory(PCITAG Tag, ADDRESS
      * try to use it instead of reading it from /proc/bus/pci.
      */
     if (((Base & 0xfffff) == 0xC0000) && (stat(file, &st) == 0)) {
+        TmpBase = Base;     
         if ((fd = open(file, O_RDWR)))
             Base = 0x0;
 
@@ -747,7 +748,14 @@ xf86ReadDomainMemory(PCITAG Tag, ADDRESS
        write(fd, "0", 2);
        close(fd);
 
-       return Len;
+       if (i != 0)
+           return Len;
+
+       /*
+        * unable to retrive all from the sysfs rom. we will try to copy
+        * the ROM from system BIOS.
+        */
+       Base = TmpBase;
     }
 
     /* Ensure page boundaries */
Comment 1 Eiichiro Oiwa 2006-09-14 00:50:24 UTC
I describe the details of the reason why we need this patch.

I couldn't start X Window system using Linux-2.6.16 on our IA64 machine. On our
IA64 machine, integrated VGA hasn't PCI ROM. However System BIOS includes Video
BIOS and system BIOS copies the Video BIOS image to 0xC0000 in system RAM.
Therefore, our machine is correct. Fortunately, Xorg is able to read the Video
BIOS from the sysfs rom on x86 platform because x86 Linux kernel has
pci_fixup_video. On the other hand, IA64 Linux kernel doesn't include
pci_fixup_video. Unfortunately, in first step, Xorg program copies Video BIOS
image from the sysfs rom and xf86ReadDomainMemroy function always returns the
value of success regardless of rom-read failed. As a result, Xorg fall into
unexpected infinite loop and VGA screen never appear. The other IA64 machine
such as SGI altix is different from our memory map. And it is true that our
integrated VGA device hasn't PCI ROM. And then, I am thinking that IA64 Linux
kernel shouldn't have pci_fixup_video. Therefore, We need this patch. 
Comment 2 Eiichiro Oiwa 2006-09-19 00:32:03 UTC
Hello Adam,
Your 16c2499b8f5c2405e36c7d5a922bb0b150df1762 has a problem with starting X on
IA64 platform except SGI Altix because the sysfs rom is empty file on many IA64
platform machines which support legacy I/O Map and integrate Video BIOS into
System BIOS. Your patch doesn't confirm whether or not the sysfs rom is empty.
And IA64 Linux kernel doesn't point to Video BIOS (0xC0000) as the sysfs rom
because IA64 Linux kernel hasn't pci_fixup_video(), unlike x86 platform. As a
result, many IA64 machines cannot start X Window system. SGI Altix is not only
IA64 platform. There are also many machines as IA64 platform.
Could you accept my patch or remove your patch for pciLinux.c in order to start
X Window System on many IA64 platform machines?
Comment 3 Stefan Dirsch 2006-09-19 01:26:01 UTC
Well, Adam only committed it. The issue has been discussed in Bug #2373.

# git-log linuxPci.c
[...]
commit 16c2499b8f5c2405e36c7d5a922bb0b150df1762
Author: Adam Jackson <ajax@nwnk.net>
Date:   Fri Apr 22 16:49:22 2005 +0000

    Bug #2373: SGI Altix platform support. (Shrijeet Mukherjee, Jesse Barnes,
        Bjorn Helgaas, Egbert Eich.)
Comment 4 Eiichiro Oiwa 2006-09-19 02:46:41 UTC
I understood Adam only committed it. However I cannot understand Bug #2373 was
closed. There is still a problem on IA64 platform machines support legacy I/O
Map and integrate Video BIOS into System BIOS. In other words, we cannot still
start X Window System on IA64 machines using embedded VGA device until he
commits my patch. Therefore, I opened this defect.
Could you commit this patch?
Comment 5 Stefan Dirsch 2006-09-19 03:56:18 UTC
No, I can't since I don't have git commit rights (CVS commit rights probably no 
longer work) and I'm not an expert in the PCI area anway.
Comment 6 Eiichiro Oiwa 2006-09-19 04:10:15 UTC
I don't have commit right, too.

Hello Adam,
I would like you to commit this patch.
Could you commit this patch?
Comment 7 Jesse Barnes 2006-09-20 11:49:27 UTC
I think you should fix the ia64 kernel to do the appropriate ROM fixups for 
your platform instead...

Jesse
Comment 8 Eiichiro Oiwa 2006-09-20 21:55:25 UTC
Do you mean we should add pci_fix_video() in the IA64 Linux kernel like x86
platform? I don't think so. There are many types on IA64 platform, although I
know two types. Some machines doesn't support legacy memory map like SGI Altix.
And the VGA embedded on main board normally doesn't have PCI ROM. There is
normally the Video BIOS in System BIOS instead of PCI ROM on these machines are
embedded VGA type. I am thinking that there is a problem in Xorg rather than
IA64 Linux kernel and machines embedded VGA. Therefore I think you should fix
xf86ReadDomainMemory() instead of IA64 Linux kernel.
Could you fix xf86ReadDomainMemory()?
Comment 9 Jesse Barnes 2006-09-20 22:34:29 UTC
Even if the embedded VGA device doesn't have its own ROM like an add-in card 
would, you can still fix the kernel to expose the video ROM (which may be part 
of the system ROM) in the appropriate sysfs file.  That's the way the kernel 
API was intended to work, so we should probably fix it at any rate.

The idea is that video cards typically require ROMs for POSTing.  The ROM for 
a given device should be copied (by the kernel) into a buffer at boot time and 
made available in the sysfs directory corresponding to the video device's PCI 
device, embedded or not.  That makes accessing ROMs from userspace easy since 
the API is common regardless of video device type (though presumably devices 
without ROM POST code altogether would be required to have very full featured 
drivers).

Whether or not we add a workaround to the X server is really a separate issue.  
I don't mind much either way, but I think ajax has some ideas about the patch 
in this bug.  Hopefully he'll elaborate his concerns shortly.  I'm just saying 
we should fix the kernel too...

Hope that helps,
Jese
Comment 10 Eiichiro Oiwa 2006-09-21 00:10:00 UTC
Sorry, Linux kernel doesn't copy the Video BIOS from the System BIOS.
pci_fixup_video() in Linux kernel only points to Video BIOS in the System RAM at
kernel boot time. System BIOS normally copy the Video BIOS (by the system
firmware not kernel) into the System RAM (0xC0000) at initializing machine. Xorg
requires the Video ROM image for INT10 functions, although some video drivers
don't require this Video ROM. Linux kernel doesn't require the Video ROM at boot
time, because Linux kernel directly uses the VGA registers instead of INT10
functions. 
I agree the sysfs rom is convenient. However, it is true that IA64 Linux is fine.

The root issue is what xf86ReadDomainMemory() doesn't confirm whether or not the
sysfs rom is empty, and this function always returns required length regardless
of actual read length.

I only hope to start X server on IA64 platform.
Comment 11 Jesse Barnes 2006-09-21 09:52:50 UTC
Yeah, it copies it in drivers/pci/rom.c:pci_map_rom_copy(), though it appears 
arches don't use it yet.

If a video bios is integrated into the system bios (for example at offset 
0xc0000), the underlying arch code set the IORESOURCE_ROM_SHADOW flag in the 
resource corresponding to the ROM, just like x86 does in pci_fixup_video().  
Copying the ROM may be necessary depending on the platform and/or video device, 
but you'd have to wire that code up if you needed it.

So, I think there are a few bugs here:
  o on ia64 the kernel isn't providing a sysfs ROM file for video devices whose
    ROM is part of the system BIOS (at 0xc0000).  this can be fixed as
    described above.
  o xf86ReadDomainMemory doesn't fail as it should if the sysfs ROM file
    doesn't exist or is zero length

I guess we disagree what the root issue is:  you say it's in 
xf86ReadDomainMemory but I think it's in the kernel.  The idea of the sysfs ROM 
file is to avoid the need for applications to poke around in /dev/mem at 
0xc0000, so in your case I think it's best to fix the ia64 kernel to get X 
running on your platform.
Comment 12 Eiichiro Oiwa 2006-09-21 23:35:17 UTC
>   o on ia64 the kernel isn't providing a sysfs ROM file for video devices whose
>     ROM is part of the system BIOS (at 0xc0000).  this can be fixed as
>     described above.
I think this should be fixed to be convenience.

>   o xf86ReadDomainMemory doesn't fail as it should if the sysfs ROM file
>     doesn't exist or is zero length
I think this must be fixed.

> I guess we disagree what the root issue is:  you say it's in 
> xf86ReadDomainMemory but I think it's in the kernel.  The idea of the sysfs ROM 
> file is to avoid the need for applications to poke around in /dev/mem at 
> 0xc0000, so in your case I think it's best to fix the ia64 kernel to get X 
> running on your platform.
I agree that applications avoid to map /dev/mem in user space. In recent code,
however, Xorg application uses legacy_mem instead of /dev/mem. Xorg maps the
sysfs rom or legacy_mem and copies a ROM image into Xorg's buffer despite
/dev/mem. Although there are many policies, these are not main issue.
Therefore, I disagree xf86ReadDomainMemory should not be fixed.
Comment 13 Jesse Barnes 2006-09-22 09:55:19 UTC
> >   o on ia64 the kernel isn't providing a sysfs ROM file for video
> > devices whose ROM is part of the system BIOS (at 0xc0000).  this can
> > be fixed as described above.
>
> I think this should be fixed to be convenience.

Ok, glad we agree on that, it should fix this bug at least.

> >   o xf86ReadDomainMemory doesn't fail as it should if the sysfs ROM
> > file doesn't exist or is zero length
>
> I think this must be fixed.

I agree, failures should be reported correctly.

> > I guess we disagree what the root issue is:  you say it's in
> > xf86ReadDomainMemory but I think it's in the kernel.  The idea of the
> > sysfs ROM file is to avoid the need for applications to poke around in
> > /dev/mem at 0xc0000, so in your case I think it's best to fix the ia64
> > kernel to get X running on your platform.
>
> I agree that applications avoid to map /dev/mem in user space. In recent
> code, however, Xorg application uses legacy_mem instead of /dev/mem.
> Xorg maps the sysfs rom or legacy_mem and copies a ROM image into Xorg's
> buffer despite /dev/mem. Although there are many policies, these are not
> main issue. Therefore, I disagree xf86ReadDomainMemory should not be
> fixed.

True, legacy_mem is better than /dev/mem, but my intent for legacy_mem was 
that it be used for mapping legacy ISA memory spaces, like the VGA 
framebuffer at 0xa0000; I had expected all ROM accesses to go through the 
sysfs rom file...

As for this bug, I'll leave it up to ajax, I know he had some concerns as 
well.  At any rate, I think if the ia64 kernel gets fixed you'll be able 
to run X on your box...

Jesse
Comment 14 Eiichiro Oiwa 2006-09-23 03:46:47 UTC
> As for this bug, I'll leave it up to ajax, I know he had some concerns as 
> well.  At any rate, I think if the ia64 kernel gets fixed you'll be able 
> to run X on your box...
I have known if pci_fixup_video is added into the IA64 kernel we will be able to
start X. I have tested it on our box. However I am thinking that
xf86ReadDomainMemory should be fixed for old IA64 kernel as well. I was thinking
 that the following remained code intends to be compatible with old kernel.

    /* Ensure page boundaries */
    offset = Base & ~pagemask;
    size = ((Base + Len + pagemask) & ~pagemask) - offset;

    ptr = xf86MapDomainMemory(-1, VIDMEM_READONLY, Tag, offset, size);

    if (!ptr)
        return -1;

    /* Using memcpy() here can hang the system */
    src = ptr + (Base - offset);
    for (len = Len;  len-- > 0;)
        *Buf++ = *src++;

    xf86UnMapVidMem(-1, ptr, size);

    return Len;

I'm a bit concerned about other machines, which have a different memory map 
such as Altix, although pci_fixup_video() behaved well on our box...
Comment 15 Jesse Barnes 2006-09-25 18:00:43 UTC
Yeah, you're right that some of the code may be incompatible with old kernels.  
But on the other hand, X really has to stop carrying code with knowledge of all 
the weirdness of PCI on various platforms and system memory layouts on 
different architectures at some point, which is why I say I'd rather see the 
kernel fixed than X changed to support systems w/o decent PCI/system bus 
abstraction layers.  I'm glad to hear you already tested that approach.

But since most of X's platform management code will go away once the rework 
branch is merged, I don't really care much if we check in a temporary hack for 
7.2 (I already did so for a sparc64 bug in fact).

As long as pci-rework gets merged post-7.2 anyway. :)

Thanks,
Jesse
Comment 16 Eiichiro Oiwa 2006-09-26 01:31:51 UTC
Ok, I requested kernel people to add pci_fixup_video into IA64 kernel.

Thanks,
Eiichiro
Comment 17 Eiichiro Oiwa 2006-09-28 19:38:16 UTC
I decided to modify Linux kernel to resolve this problem.
We need the following patch until this patch involves mainline kernel.
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18/2.6.18-mm2/broken-out/pci-turn-pci_fixup_video-into-generic-for-embedded.patch

We can close this defect because of this patch.

Thanks,
Eiichiro

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.