port-i386/48676: DRM2 kernel code can't be used in an i386 environment

To: port-i386-maintainer%netbsd.org@localhost,gnats-admin%netbsd.org@localhost,netbsd-bugs%netbsd.org@localhost
Subject: port-i386/48676: DRM2 kernel code can't be used in an i386 environment
From: dhgutteridge%sympatico.ca@localhost
Date: Sun, 23 Mar 2014 21:55:00 +0000 (UTC)
>Number:         48676
>Category:       port-i386
>Synopsis:       DRM2 kernel code can't be used in an i386 environment
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-i386-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 23 21:55:00 +0000 2014
>Originator:     David H. Gutteridge
>Release:        6.99.37
>Organization:
>Environment:
n/a
>Description:
I naively started trying to get the DRM2 kernel code to compile for
i386, using the ham-fisted approach of finding and fixing problems
through repeated compiles. I'm not sure whether this is of any value
to anyone, or if it's intended that this code should ultimately work
for i386 as well at some point, but it certainly won't at present.
I figured I should make note of it in a PR to save someone else the
same efforts.

I'm out of my depth here, and unsure my choices are all the best ones.
I got as far as the kernel linking phase, when I then found there were
functions that wouldn't link because they're not implemented for i386.
The Linux code from Intel assumes in places that 64-bit reads/writes
from/to registers can be made, and there are no bus_space_*_8(9)
functions for i386. Earlier generation Intel graphics chipsets (like
the 945GM, which is what I'd intended to test in an i386 environment)
are handled with only 32-bit reads/writes, but not the more recent
chipsets, and in places the code is of a piece.
>How-To-Repeat:
Create an equivalent DRMKMS kernel config for i386 and try to compile
it.
>Fix:
Below I've listed compilation errors and what I did to fix them, up to
the point linking failed.

--- drm_bufs.o ---
#   compile  DRMKMS/drm_bufs.o
/usr/builds/netbsd-current/src/obj/tooldir.NetBSD-5.2_STABLE-i386/bin/i486--netbsdelf-gcc
 -pipe -O2 -msoft-float -mno-mmx -mno-sse -mno-avx -ffreestanding 
-fno-zero-initialized-in-bss -g -pipe -O2 -fstack-protector -Wstack-protector 
--param ssp-buffer-size=1 -fno-strict-aliasing -fno-common -std=gnu99 -Werror 
-Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes 
-Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual 
-Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wextra 
-Wno-unused-parameter -Wold-style-definition -Wno-sign-compare 
--sysroot=/usr/builds/netbsd-current/src/obj/destdir.i386 -Di386 -I. 
-I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/arch -I/usr/builds/netbsd-current/src/sys 
-nostdinc -DDIAGNOSTIC -DDEBUG -DMAXUSERS=64 -D_KERNEL -D_KERNEL_OPT -std=gnu99 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/quad 
-I/usr/builds/netbsd-cur
 rent/src/sys/lib/libkern/../../../common/lib/libc/string 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/arch/i386/string
 -D_FORTIFY_SOURCE=2 -I/usr/builds/netbsd-current/src/sys/external/bsd/ipf 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/dist 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/ic 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/uapi 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include 
-D__KERNEL__ -I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915 
-DCONFIG_MTRR -DCONFIG_X86 -DMTRR 
-I/usr/builds/netbsd-current/src/sys/external/bsd/acpica/dist/include -c 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_bufs.c
cc1: warnings being treated as errors
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_bufs.c: In 
function 'drm_addmap_core':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:182:7: 
error: implicit declaration of function 'virt_to_phys'
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:182:34:
 error: 'high_memory' undeclared (first use in this function)
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:182:34:
 note: each undeclared identifier is reported only once for each function it 
appears in
*** [drm_bufs.o] Error code 1

This is due to the fact that Linux-only code is present in the file.
virt_to_phys(9) is a Linux memory management function. I don't know
if the inclusion of that check is a sign there should be something
equivalent for i386 on NetBSD, but if so, it would obviously have to
be implemented differently.

I added a check to the beginning of the macro to make it specifically
for Linux. (I assume that's the best choice given you likely want to
minimize diffs with the upstream source.)

--- drm_bufs.c.orig     2014-03-19 21:01:58.000000000 -0400
+++ drm_bufs.c  2014-03-22 13:50:36.000000000 -0400
@@ -177,7 +177,7 @@
        switch (map->type) {
        case _DRM_REGISTERS:
        case _DRM_FRAME_BUFFER:
-#if !defined(__sparc__) && !defined(__alpha__) && !defined(__ia64__) && 
!defined(__powerpc64__) && !defined(__x86_64__) && !defined(__arm__)
+#if defined(__linux__) && !defined(__sparc__) && !defined(__alpha__) && 
!defined(__ia64__) && !defined(__powerpc64__) && !defined(__x86_64__) && 
!defined(__arm__)
                if (map->offset + (map->size-1) < map->offset ||
                    map->offset < virt_to_phys(high_memory)) {
                        kfree(map);

Next...

--- dvo_ch7xxx.o ---
cc1: warnings being treated as errors
In file included from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include/drm/intel-gtt.h:37:0,
                 from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915/i915_drv.h:39,
                 from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915/intel_drv.h:31,
                 from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915/dvo.h:29,
                 from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915/dvo_ch7xxx.c:29:
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include/drm/bus_dma_hacks.h:
 In function 'bus_dmamem_wire_uvm_object':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include/drm/bus_dma_hacks.h:55:2:
 error: comparison is always true due to limited range of data type

This is because the code assumes sizeof(off_t) == sizeof(bus_size_t),
which isn't the case for i386. Getting it to just compile is quite
simple, since size is really defined as bus_size_t. (It's unclear to
me whether there are potential portability issues lurking here.)

--- bus_dma_hacks.h.orig        2014-03-18 14:20:43.000000000 -0400
+++ bus_dma_hacks.h     2014-03-22 14:08:18.000000000 -0400
@@ -52,7 +52,7 @@
        unsigned int i;
        int error;
 
-       KASSERT(size <= __type_max(off_t));
+       KASSERT(size <= __type_max(bus_size_t));
        KASSERT(start <= (__type_max(off_t) - size));
        KASSERT(alignment == PAGE_SIZE); /* XXX */
        KASSERT(0 < nsegs);

Next...

In file included from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include/drm/drmP.h:52:0,
                 from 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/drm_agpsupport.c:34:
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include/linux/pci.h: In 
function 'pci_bus_alloc_resource':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include/linux/pci.h:255:6: 
error: large integer implicitly truncated to unsigned type
*** [drm_agpsupport.o] Error code 1

This is because the code assumes we're dealing with 64-bit bus_size_t
values, which obviously isn't the case for i386. I don't know if this
is the preferred solution, but I did this:
 
--- pci.h.orig  2014-03-21 23:27:30.000000000 -0400
+++ pci.h       2014-03-22 15:20:31.000000000 -0400
@@ -37,6 +37,8 @@
 #include <sys/kmem.h>
 #include <sys/systm.h>
 
+#include <machine/limits.h>
+
 #include <dev/pci/pcidevs.h>
 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
@@ -251,7 +253,7 @@
        }
 
        resource->r_bst = bst;
-       error = bus_space_alloc(bst, start, 0xffffffffffffffffULL /* XXX */,
+       error = bus_space_alloc(bst, start, ULONG_MAX /* XXX */,
            size, align, 0, 0, &resource->start, &resource->r_bsh);
        if (error)
                return error;

Next...

#   compile  DRMKMS/i915_gem_gtt.o
/usr/builds/netbsd-current/src/obj/tooldir.NetBSD-5.2_STABLE-i386/bin/i486--netbsdelf-gcc
 -pipe -O2 -msoft-float -mno-mmx -mno-sse -mno-avx -ffreestanding 
-fno-zero-initialized-in-bss -g -pipe -O2 -fstack-protector -Wstack-protector 
--param ssp-buffer-size=1 -fno-strict-aliasing -fno-common -std=gnu99 -Werror 
-Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes 
-Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual 
-Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wextra 
-Wno-unused-parameter -Wold-style-definition -Wno-sign-compare 
--sysroot=/usr/builds/netbsd-current/src/obj/destdir.i386 -Di386 -I. 
-I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/arch -I/usr/builds/netbsd-current/src/sys 
-nostdinc -DDIAGNOSTIC -DDEBUG -DMAXUSERS=64 -D_KERNEL -D_KERNEL_OPT -std=gnu99 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/quad 
-I/usr/builds/netbsd-cur
 rent/src/sys/lib/libkern/../../../common/lib/libc/string 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/arch/i386/string
 -D_FORTIFY_SOURCE=2 -I/usr/builds/netbsd-current/src/sys/external/bsd/ipf 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/dist 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/ic 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/uapi 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include 
-D__KERNEL__ -I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915 
-DCONFIG_MTRR -DCONFIG_X86 -DMTRR 
-I/usr/builds/netbsd-current/src/sys/external/bsd/acpica/dist/include -c 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c
cc1: warnings being treated as errors
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c: In 
function 'i915_gem_gtt_init':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c:96:3:
 error: large integer implicitly truncated to unsigned type
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c: In 
function 'gen6_pte_addr_encode':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c:510:2:
 error: comparison is always true due to limited range of data type
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c: In 
function 'gen6_gtt_init':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c:586:3:
 error: format '%llx' expects type 'long long unsigned int', but argument 3 has 
type 'bus_size_t'
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_gem_gtt.c:586:3:
 error: format '%llx' expects type 'long long unsigned int', but argument 4 has 
type 'unsigned int'

Here we have a few different issues, all assumptions we're compiling
for a 64-bit environment. Again, I'm not sure my changes are the
correct NetBSD way, but it compiles. (I haven't looked through the
code to see if there are other 64-bit only surprises that the compiler
doesn't catch, I was just trying to get it to compile so I could test
it. What I did may of course be problematic in that there could be
assumptions elsewhere that 64-bit values are being used, which I'm just
glossing over...)

--- i915_gem_gtt.c.orig 2014-03-18 14:20:42.000000000 -0400
+++ i915_gem_gtt.c      2014-03-22 15:21:32.000000000 -0400
@@ -38,6 +38,8 @@
 #include <sys/kmem.h>
 #include <sys/systm.h>
 
+#include <machine/limits.h>
+
 #include <dev/pci/pcivar.h>
 
 #include <dev/pci/agpvar.h>
@@ -90,10 +92,15 @@
         *
         * XXX pci_set_dma_mask?  pci_set_consistent_dma_mask?
         */
+#if LONG_BIT == 64
        if (INTEL_INFO(dev)->gen < 4)
-               drm_limit_dma_space(dev, 0, 0x00000000ffffffffULL);
+               drm_limit_dma_space(dev, 0, 0x00000000ffffffffUL);
        else
-               drm_limit_dma_space(dev, 0, 0x0000000fffffffffULL);
+               drm_limit_dma_space(dev, 0, 0x0000000fffffffffUL);
+#else
+               /* XXX This should implicitly be redundant... */
+               drm_limit_dma_space(dev, 0, 0xffffffffUL);
+#endif
 
        /* Success!  */
        DRM_INFO("Memory usable by graphics device = %dM\n",
@@ -507,7 +514,9 @@
 static uint32_t
 gen6_pte_addr_encode(bus_addr_t addr)
 {
+#if LONG_BIT == 64
        KASSERT(addr <= __BITS(39, 0));
+#endif
        return (addr | ((addr >> 28) & 0xff0));
 }
 
@@ -585,8 +594,8 @@
                sizeof(gtt_pte_t))) {
                DRM_ERROR("BAR0 too small for GTT: 0x%"PRIxMAX" < 0x%"PRIxMAX
                    "\n",
-                   dev->bus_maps[0].bm_size,
-                   (gtt->gtt_total_entries * sizeof(gtt_pte_t)));
+                   (uintmax_t)dev->bus_maps[0].bm_size,
+                   (uintmax_t)(gtt->gtt_total_entries * sizeof(gtt_pte_t)));
                ret = -ENODEV;
                goto fail0;
        }

Next...

--- i915_pci.o ---
#   compile  DRMKMS/i915_pci.o
/usr/builds/netbsd-current/src/obj/tooldir.NetBSD-5.2_STABLE-i386/bin/i486--netbsdelf-gcc
 -pipe -O2 -msoft-float -mno-mmx -mno-sse -mno-avx -ffreestanding 
-fno-zero-initialized-in-bss -g -pipe -O2 -fstack-protector -Wstack-protector 
--param ssp-buffer-size=1 -fno-strict-aliasing -fno-common -std=gnu99 -Werror 
-Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes 
-Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual 
-Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wextra 
-Wno-unused-parameter -Wold-style-definition -Wno-sign-compare 
--sysroot=/usr/builds/netbsd-current/src/obj/destdir.i386 -Di386 -I. 
-I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/arch -I/usr/builds/netbsd-current/src/sys 
-nostdinc -DDIAGNOSTIC -DDEBUG -DMAXUSERS=64 -D_KERNEL -D_KERNEL_OPT -std=gnu99 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/quad 
-I/usr/builds/netbsd-cur
 rent/src/sys/lib/libkern/../../../common/lib/libc/string 
-I/usr/builds/netbsd-current/src/sys/lib/libkern/../../../common/lib/libc/arch/i386/string
 -D_FORTIFY_SOURCE=2 -I/usr/builds/netbsd-current/src/sys/external/bsd/ipf 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/dist 
-I/usr/builds/netbsd-current/src/sys/external/isc/atheros_hal/ic 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/uapi 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include 
-D__KERNEL__ -I/usr/builds/netbsd-current/src/sys/../common/include 
-I/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/drm/i915 
-DCONFIG_MTRR -DCONFIG_X86 -DMTRR 
-I/usr/builds/netbsd-current/src/sys/external/bsd/acpica/dist/include -c 
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_pci.c
cc1: warnings being treated as errors
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_pci.c: In 
function 'i915drm_fb_probe':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/i915drm/i915_pci.c:426:6: 
error: cast from pointer to integer of different size

Here a pointer is being stored as an int, and it's assumed it's a
64-bit pointer. My workaround is very ugly, but I couldn't find any
code that actually accessed these proplib(3) values, so I assumed I
wasn't breaking any "getter" function call expecting a 64-bit value.

--- i915_pci.c.orig     2014-03-18 14:20:42.000000000 -0400
+++ i915_pci.c  2014-03-22 21:38:30.000000000 -0400
@@ -39,6 +39,8 @@
 #endif
 #include <sys/systm.h>
 
+#include <machine/limits.h>
+
 #include <dev/pci/pciio.h>
 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
@@ -422,8 +424,13 @@
        prop_dictionary_set_uint8(dict, "depth", sizes->surface_bpp);
        prop_dictionary_set_uint16(dict, "linebytes", mode_cmd.pitches[0]);
        prop_dictionary_set_uint32(dict, "address", 0); /* XXX >32-bit */
+#if LONG_BIT == 32
+       prop_dictionary_set_uint32(dict, "virtual_address",
+           (uint32_t)bus_space_vaddr(dev->bst, sc->sc_fb_bsh));
+#else
        prop_dictionary_set_uint64(dict, "virtual_address",
            (uint64_t)bus_space_vaddr(dev->bst, sc->sc_fb_bsh));
+#endif
        sc->sc_genfb.sc_dev = sc->sc_dev;
        genfb_init(&sc->sc_genfb);

Next...

#      link  DRMKMS/netbsd
/usr/builds/netbsd-current/src/obj/tooldir.NetBSD-5.2_STABLE-i386/bin/i486--netbsdelf-ld
 -Map netbsd.map --cref -T 
/usr/builds/netbsd-current/src/sys/arch/i386/conf/kern.ldscript -Ttext c0100000 
-e start -X -o netbsd ${SYSTEM_OBJ} ${EXTRA_OBJ} vers.o
i915_drv.o: In function `DRM_READ64':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include/drm/drmP.h:2114:
 undefined reference to `bus_space_read_8'
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include/drm/drmP.h:2114:
 undefined reference to `bus_space_read_8'
i915_drv.o: In function `DRM_WRITE64':
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include/drm/drmP.h:2154:
 undefined reference to `bus_space_write_8'
/usr/builds/netbsd-current/src/sys/external/bsd/drm2/dist/include/drm/drmP.h:2154:
 undefined reference to `bus_space_write_8'
*** [netbsd] Error code 1

Here we have some 64-bit functions unguarded by checks to see if
they're implemented on the architecture in question. It seems there's
nothing defined to indiciate whether these functions are implemented
or not on a given NetBSD port; rather, if unimplemented, they're set
to deliberately fail at link time, which is kind of ugly.

(I can see other port-specific Linuxisms that haven't been guarded
against in case they're incorrect for NetBSD, but I haven't bothered
looking at them, as I'm just trying to get i386 to work right now.)

At this point, following function names back through some preprocessor
name mangling, I found code like this in the Intel-origin source:

static void i915_restore_modeset_reg(struct drm_device *dev)
{
        struct drm_i915_private *dev_priv = dev->dev_private;
        int dpll_a_reg, fpa0_reg, fpa1_reg;
        int dpll_b_reg, fpb0_reg, fpb1_reg;
        int i;

        if (drm_core_check_feature(dev, DRIVER_MODESET))
                return;

        /* Fences */
        switch (INTEL_INFO(dev)->gen) {
        case 7:
        case 6:
                for (i = 0; i < 16; i++)
                        I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + (i * 8), 
dev_priv->regfile.saveFENCE[i]);
                break;
        case 5:
        case 4:
                for (i = 0; i < 16; i++)
                        I915_WRITE64(FENCE_REG_965_0 + (i * 8), 
dev_priv->regfile.saveFENCE[i]);
                break;
        case 3:
        case 2:
                if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
                        for (i = 0; i < 8; i++)
                                I915_WRITE(FENCE_REG_945_8 + (i * 4), 
dev_priv->regfile.saveFENCE[i+8]);
                for (i = 0; i < 8; i++)
                        I915_WRITE(FENCE_REG_830_0 + (i * 4), 
dev_priv->regfile.saveFENCE[i]);
                break;
        }
Prev by Date: NetBSD Nightly Trouble Ticket Report
Next by Date: port-luna68k/48677: typo in luna68k
Previous by Thread: Re: kern/48545 (CD access with empty CD/R kills the kernel)
Next by Thread: Re: port-i386/48676: DRM2 kernel code can't be used in an i386 environment
Indexes:
Home | Main Index | Thread Index | Old Index