On Sun, Nov 05, 2023 at 05:27:03PM +0000, Paz Zcharya wrote:
> Fix the value of variable `phys_base` to be the relative offset in
> stolen memory, and not the absolute offset of the GSM.
to me it looks like the other way around. phys_base is the physical
base address for the frame_buffer. Setting it to zero doesn't seem
to make that relative. And also doesn't look right.
>
> Currently, the value of `phys_base` is set to "Surface Base Address,"
> which in the case of Meter Lake is 0xfc00_0000.
I don't believe this is a fixed value. IIRC this comes from the register
set by video bios, where the idea is to reuse the fb that was used so
far.
With this in mind I don't understand how that could overflow. Maybe
the size of the stolen is not right? maybe the size? maybe different
memory region?
> This causes the
> function `i915_gem_object_create_region_at` to fail in line 128, when
> it attempts to verify that the range does not overflow:
>
> if (range_overflows(offset, size, resource_size(&mem->region)))
> return ERR_PTR(-EINVAL);
>
> where:
> offset = 0xfc000000
> size = 0x8ca000
> mem->region.end + 1 = 0x4400000
> mem->region.start = 0x800000
> resource_size(&mem->region) = 0x3c00000
>
> call stack:
> i915_gem_object_create_region_at
> initial_plane_vma
> intel_alloc_initial_plane_obj
> intel_find_initial_plane_obj
> intel_crtc_initial_plane_config
>
> Looking at the flow coming next, we see that `phys_base` is only used
> once, in function `_i915_gem_object_stolen_init`, in the context of
> the offset *in* the stolen memory. Combining that with an
> examinination of the history of the file seems to indicate the
> current value set is invalid.
>
> call stack (functions using `phys_base`)
> _i915_gem_object_stolen_init
> __i915_gem_object_create_region
> i915_gem_object_create_region_at
> initial_plane_vma
> intel_alloc_initial_plane_obj
> intel_find_initial_plane_obj
> intel_crtc_initial_plane_config
>
> [drm:_i915_gem_object_stolen_init] creating preallocated stolen
> object: stolen_offset=0x0000000000000000, size=0x00000000008ca000
>
> Signed-off-by: Paz Zcharya <pazz@xxxxxxxxxxxx>
> ---
>
> drivers/gpu/drm/i915/display/intel_plane_initial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index a55c09cbd0e4..e696cb13756a 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -90,7 +90,7 @@ initial_plane_vma(struct drm_i915_private *i915,
> "Using phys_base=%pa, based on initial plane programming\n",
> &phys_base);
> } else {
> - phys_base = base;
> + phys_base = 0;
> mem = i915->mm.stolen_region;
> }
>
> --
> 2.42.0.869.gea05f2083d-goog
>
