memory overwrites on Arrandale (AGP)

[mattmacy]

All the frame buffer attributes are the same in both 3.8 and 4.6 which is why it's very odd that bzero'ing it after mapping appears to overwrite many thread stacks causing the system to fail rather spectacularly. @dumbbell can you take a look?

static int intelfb_create(struct drm_fb_helper *helper,
              struct drm_fb_helper_surface_size *sizes)
{
    struct intel_fbdev *ifbdev =
        container_of(helper, struct intel_fbdev, helper);
    struct intel_framebuffer *intel_fb = ifbdev->fb;
    struct drm_device *dev = helper->dev;
    struct drm_i915_private *dev_priv = dev->dev_private;
    struct fb_info *info;
    struct drm_framebuffer *fb;
    struct drm_i915_gem_object *obj;
    int size, ret;
    bool prealloc = false;

    if (intel_fb &&
        (sizes->fb_width > intel_fb->base.width ||
         sizes->fb_height > intel_fb->base.height)) {
        DRM_DEBUG_KMS("BIOS fb too small (%dx%d), we require (%dx%d),"
                  " releasing it\n",
                  intel_fb->base.width, intel_fb->base.height,
                  sizes->fb_width, sizes->fb_height);
        drm_framebuffer_unreference(&intel_fb->base);
        intel_fb = ifbdev->fb = NULL;
    }
    if (!intel_fb || WARN_ON(!intel_fb->obj)) {
        DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
        ret = intelfb_alloc(helper, sizes);
        if (ret)
            return ret;
        intel_fb = ifbdev->fb;
    } else {
        DRM_DEBUG_KMS("re-using BIOS fb\n");
        prealloc = true;
        sizes->fb_width = intel_fb->base.width;
        sizes->fb_height = intel_fb->base.height;
    }

    obj = intel_fb->obj;
    size = obj->base.size;

    mutex_lock(&dev->struct_mutex);

    /* Pin the GGTT vma for our access via info->screen_base.
     * This also validates that any existing fb inherited from the
     * BIOS is suitable for own access.
     */
    ret = intel_pin_and_fence_fb_obj(NULL, &ifbdev->fb->base, NULL);
    if (ret)
        goto out_unlock;

    info = drm_fb_helper_alloc_fbi(helper);
    if (IS_ERR(info)) {
        DRM_ERROR("Failed to allocate fb_info\n");
        ret = PTR_ERR(info);
        goto out_unpin;
    }

    info->par = helper;

    fb = &ifbdev->fb->base;

    ifbdev->helper.fb = fb;

    strcpy(info->fix.id, "inteldrmfb");

    info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
    info->fbops = &intelfb_ops;

    /* setup aperture base/size for vesafb takeover */
    info->apertures->ranges[0].base = dev->mode_config.fb_base;
    info->apertures->ranges[0].size = dev_priv->gtt.mappable_end;

    info->fix.smem_start = dev->mode_config.fb_base + i915_gem_obj_ggtt_offset(obj);
    info->fix.smem_len = size;

    vm_phys_fictitious_reg_range(info->apertures->ranges[0].base,
                     info->apertures->ranges[0].base +
                     info->apertures->ranges[0].size,
                     VM_MEMATTR_WRITE_COMBINING);
    info->screen_base =
        ioremap_wc(dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj),
               size);

    if (!info->screen_base) {
        DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
        ret = -ENOSPC;
        goto out_destroy_fbi;
    }
    info->screen_size = size;
    DRM_DEBUG("mappable_base %lx ggtt_offset %lx screen_size %lx\n", dev_priv->gtt.mappable_base,
          i915_gem_obj_ggtt_offset(obj), info->screen_size);

    /* This driver doesn't need a VT switch to restore the mode on resume */
    info->skip_vt_switch = true;

    drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
    drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);

    /* If the object is shmemfs backed, it will have given us zeroed pages.
     * If the object is stolen however, it will be full of whatever
     * garbage was left in there.
     */
    if (ifbdev->fb->obj->stolen && !prealloc)
        memset_io(info->screen_base, 0, info->screen_size); <----- MEMORY OVERWRITE HERE

    /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */

    DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08zx, bo %p\n",
              fb->width, fb->height,
              i915_gem_obj_ggtt_offset(obj), obj);

    mutex_unlock(&dev->struct_mutex);
    vga_switcheroo_client_fb_set(dev->pdev, info);
    return 0;

out_destroy_fbi:
    drm_fb_helper_release_fbi(helper);
out_unpin:
    i915_gem_object_ggtt_unpin(obj);
out_unlock:
    mutex_unlock(&dev->struct_mutex);
    return ret;
}

[dumbbell]

Hi!

The problem is an incompatibility between the struct sg_table and struct sg_list types:

• i915_ggtt_insert_entries passes a struct sg_table to (agp_)intel_gtt_insert_sg_entries: https://github.com/FreeBSDDesktop/freebsd-base-graphics/blob/drm-next-4.6/sys/dev/drm2/i915/i915_gem_gtt.c#L2559
• agp_intel_gtt_insert_sg_entries expects a struct sg_list: https://github.com/FreeBSDDesktop/freebsd-base-graphics/blob/drm-next-4.6/sys/dev/agp/agp_i810.c#L2236

Obviously walking the list of segments doesn't make any sense. On my laptop, it tries to map around 3 GiB of data instead of the expected 128 kiB. I have no idea why the compiler doesn't complain loudly.

The implementation of agp_intel_gtt_insert_sg_entries is buggy too (this code is not used in HEAD) here: https://github.com/FreeBSDDesktop/freebsd-base-graphics/blob/drm-next-4.6/sys/dev/agp/agp_i810.c#L2245-L2247

Ie. it increments j and checks its value against sg_nsegs, however, it uses i to actually access the sg_segs array.

I will fix those two issues tomorrow or Wednesday and come back to you.

[mattmacy]

There are still stray writes but I can actually run X now.