| Message ID | 20190806190021.6566-1-tjaalton@ubuntu.com |
|---|---|
| Headers | show |
| Series | drm/i915: Fix softpin on 32bit | expand |
On 2019-08-06 22:00:18 , Timo Aaltonen wrote: > BugLink: https://bugs.launchpad.net/bugs/1815172 > > We've been carrying a commit reverting softpin support from mesa in bionic, > because softpin support was broken on 4.15 and 4.18. The patches to fix it > were sent to stable@ but not all of them got applied to 4.15.x because there > were minor conflicts. The patch for drm/i915/gvt was added to make the other > one cherry-pick cleanly. > > The reason to get these backported is so that we can drop the revert from > mesa, because it actually broke Ice Lake which apparently requires softpin > support in the DRI driver. > > Chris Wilson (2): > drm/i915: Mark up GTT sizes as u64 > drm/i915: Compare user's 64b GTT offset even on 32b > > Zhi Wang (1): > drm/i915/gvt: Use I915_GTT_PAGE_SIZE > > drivers/gpu/drm/i915/gvt/cmd_parser.c | 13 ++--- > drivers/gpu/drm/i915/gvt/execlist.c | 2 +- > drivers/gpu/drm/i915/gvt/gtt.c | 51 ++++++++++--------- > drivers/gpu/drm/i915/gvt/gtt.h | 6 +-- > drivers/gpu/drm/i915/gvt/reg.h | 3 +- > drivers/gpu/drm/i915/gvt/scheduler.c | 12 ++--- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.h | 8 +-- > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 +-- > 11 files changed, 55 insertions(+), 52 deletions(-) > Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
On 8/6/19 12:00 PM, Timo Aaltonen wrote: > BugLink: https://bugs.launchpad.net/bugs/1815172 > > We've been carrying a commit reverting softpin support from mesa in bionic, > because softpin support was broken on 4.15 and 4.18. The patches to fix it > were sent to stable@ but not all of them got applied to 4.15.x because there > were minor conflicts. The patch for drm/i915/gvt was added to make the other > one cherry-pick cleanly. > > The reason to get these backported is so that we can drop the revert from > mesa, because it actually broke Ice Lake which apparently requires softpin > support in the DRI driver. > > Chris Wilson (2): > drm/i915: Mark up GTT sizes as u64 > drm/i915: Compare user's 64b GTT offset even on 32b > > Zhi Wang (1): > drm/i915/gvt: Use I915_GTT_PAGE_SIZE > > drivers/gpu/drm/i915/gvt/cmd_parser.c | 13 ++--- > drivers/gpu/drm/i915/gvt/execlist.c | 2 +- > drivers/gpu/drm/i915/gvt/gtt.c | 51 ++++++++++--------- > drivers/gpu/drm/i915/gvt/gtt.h | 6 +-- > drivers/gpu/drm/i915/gvt/reg.h | 3 +- > drivers/gpu/drm/i915/gvt/scheduler.c | 12 ++--- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.h | 8 +-- > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 +-- > 11 files changed, 55 insertions(+), 52 deletions(-) > Acked-by: Connor Kuehl <connor.kuehl@canonical.com>
Applied to Bionic.
Timo, note that there was a conflict with patch 2/3.
The first chunk of delta in drivers/gpu/drm/i915/gvt/cmd_parser.c expects the line:
if (guest_gma >= GTT_PAGE_SIZE / sizeof(u64)) {
but in bionic/master-next, that line is actually:
if (guest_gma >= GTT_PAGE_SIZE) {
I went ahead and replaced the line anyway with:
if (guest_gma >= I915_GTT_PAGE_SIZE / sizeof(u64)) {
Khaled
On 2019-08-06 22:00:18 , Timo Aaltonen wrote:
> BugLink: https://bugs.launchpad.net/bugs/1815172
>
> We've been carrying a commit reverting softpin support from mesa in bionic,
> because softpin support was broken on 4.15 and 4.18. The patches to fix it
> were sent to stable@ but not all of them got applied to 4.15.x because there
> were minor conflicts. The patch for drm/i915/gvt was added to make the other
> one cherry-pick cleanly.
>
> The reason to get these backported is so that we can drop the revert from
> mesa, because it actually broke Ice Lake which apparently requires softpin
> support in the DRI driver.
>
> Chris Wilson (2):
> drm/i915: Mark up GTT sizes as u64
> drm/i915: Compare user's 64b GTT offset even on 32b
>
> Zhi Wang (1):
> drm/i915/gvt: Use I915_GTT_PAGE_SIZE
>
> drivers/gpu/drm/i915/gvt/cmd_parser.c | 13 ++---
> drivers/gpu/drm/i915/gvt/execlist.c | 2 +-
> drivers/gpu/drm/i915/gvt/gtt.c | 51 ++++++++++---------
> drivers/gpu/drm/i915/gvt/gtt.h | 6 +--
> drivers/gpu/drm/i915/gvt/reg.h | 3 +-
> drivers/gpu/drm/i915/gvt/scheduler.c | 12 ++---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.h | 8 +--
> drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +-
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 +--
> 11 files changed, 55 insertions(+), 52 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 8/12/19 4:31 AM, Khaled Elmously wrote: > Applied to Bionic. > > Timo, note that there was a conflict with patch 2/3. > > The first chunk of delta in drivers/gpu/drm/i915/gvt/cmd_parser.c expects the line: > > > > if (guest_gma >= GTT_PAGE_SIZE / sizeof(u64)) { > > > > but in bionic/master-next, that line is actually: > > > if (guest_gma >= GTT_PAGE_SIZE) { > > > > > I went ahead and replaced the line anyway with: > > > if (guest_gma >= I915_GTT_PAGE_SIZE / sizeof(u64)) { Hi Khaled, If you make any change when applying a patch from the mailing-list, please state the reason above your s-o-b line. If we have an issue with the code in the future we can trace back the changes and understand the rationale behind it. The removal of the division for "sizeof(u64)" has been explicitly done in the meantime with the backport of the following commit applied as stable update: drm/i915/gvt: Fix MI_FLUSH_DW parsing with correct index check which is commit 13bcb80b7ee79431fce361e060611134cb19e209 upstream. So adding back the division functionally reverts this commit. Also, it was applied upstream after "drm/i915/gvt: Use I915_GTT_PAGE_SIZE" and the change it makes is: if (index_mode) { - if (guest_gma >= I915_GTT_PAGE_SIZE / sizeof(u64)) { + if (guest_gma >= I915_GTT_PAGE_SIZE) { ret = -EFAULT; goto err; } so I believe we should stick to what Patch 2/3 does, just replacing GTT_PAGE_SIZE by I915_GTT_PAGE_SIZE, which end result would be exactly what's upstream in rev 9556e1188892. I will amend this commit on master-next to fix it. Thanks, Kleber > > > > Khaled > > > > On 2019-08-06 22:00:18 , Timo Aaltonen wrote: >> BugLink: https://bugs.launchpad.net/bugs/1815172 >> >> We've been carrying a commit reverting softpin support from mesa in bionic, >> because softpin support was broken on 4.15 and 4.18. The patches to fix it >> were sent to stable@ but not all of them got applied to 4.15.x because there >> were minor conflicts. The patch for drm/i915/gvt was added to make the other >> one cherry-pick cleanly. >> >> The reason to get these backported is so that we can drop the revert from >> mesa, because it actually broke Ice Lake which apparently requires softpin >> support in the DRI driver. >> >> Chris Wilson (2): >> drm/i915: Mark up GTT sizes as u64 >> drm/i915: Compare user's 64b GTT offset even on 32b >> >> Zhi Wang (1): >> drm/i915/gvt: Use I915_GTT_PAGE_SIZE >> >> drivers/gpu/drm/i915/gvt/cmd_parser.c | 13 ++--- >> drivers/gpu/drm/i915/gvt/execlist.c | 2 +- >> drivers/gpu/drm/i915/gvt/gtt.c | 51 ++++++++++--------- >> drivers/gpu/drm/i915/gvt/gtt.h | 6 +-- >> drivers/gpu/drm/i915/gvt/reg.h | 3 +- >> drivers/gpu/drm/i915/gvt/scheduler.c | 12 ++--- >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- >> drivers/gpu/drm/i915/i915_gem_gtt.h | 8 +-- >> drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- >> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 +-- >> 11 files changed, 55 insertions(+), 52 deletions(-) >> >> -- >> 2.17.1 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team >
On 12.8.2019 11.04, Kleber Souza wrote: > On 8/12/19 4:31 AM, Khaled Elmously wrote: >> Applied to Bionic. >> >> Timo, note that there was a conflict with patch 2/3. >> >> The first chunk of delta in drivers/gpu/drm/i915/gvt/cmd_parser.c expects the line: >> >> >> >> if (guest_gma >= GTT_PAGE_SIZE / sizeof(u64)) { >> >> >> >> but in bionic/master-next, that line is actually: >> >> >> if (guest_gma >= GTT_PAGE_SIZE) { >> >> >> >> >> I went ahead and replaced the line anyway with: >> >> >> if (guest_gma >= I915_GTT_PAGE_SIZE / sizeof(u64)) { > > Hi Khaled, > > If you make any change when applying a patch from the mailing-list, > please state the reason above your s-o-b line. If we have an issue > with the code in the future we can trace back the changes and > understand the rationale behind it. > > The removal of the division for "sizeof(u64)" has been explicitly done > in the meantime with the backport of the following commit applied as > stable update: > > drm/i915/gvt: Fix MI_FLUSH_DW parsing with correct index check > > which is commit 13bcb80b7ee79431fce361e060611134cb19e209 upstream. > > So adding back the division functionally reverts this commit. Also, > it was applied upstream after "drm/i915/gvt: Use I915_GTT_PAGE_SIZE" > and the change it makes is: > > if (index_mode) { > - if (guest_gma >= I915_GTT_PAGE_SIZE / sizeof(u64)) { > + if (guest_gma >= I915_GTT_PAGE_SIZE) { > ret = -EFAULT; > goto err; > } > > so I believe we should stick to what Patch 2/3 does, just replacing > GTT_PAGE_SIZE by I915_GTT_PAGE_SIZE, which end result would be > exactly what's upstream in rev 9556e1188892. > > I will amend this commit on master-next to fix it. > > > Thanks, > Kleber Right, I based these on -56.62 and not master-next, because they were tested that way. Should work just the same with the new rebase to stable.