All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH igt] igt/gem_exec_capture: MI_STORE_DWORD requires EXEC_SECURE + DRM_MASTER on ctg/ilk
Date: Mon, 12 Feb 2018 19:30:52 +0200	[thread overview]
Message-ID: <20180212173052.GU5453@intel.com> (raw)
In-Reply-To: <20180210214338.7611-1-chris@chris-wilson.co.uk>

On Sat, Feb 10, 2018 at 09:43:38PM +0000, Chris Wilson wrote:
> On ctg/ilk, for whatever reason, MI_STORE_DWORD is a privileged operation
> so we must request a SECURE batch.

IIRC ctg supposedly introduced some form of ppgtt. Isn't that the
reason?

Hmm. Now I wonder how anything works on these platforms. Should the
batch itself be executed via ppgtt if it's non-secure? Maybe the hw
has a fallback mechanism of some sort to execute via ggtt if ppgtt
isn't enabled...

ppgtt enable bit:
"When this bit is clear, all memory accesses will be completed using the
GGTT. Privileged memory protections will not be enforced (it is
acceptable for a non-secure batch buffer to access GGTT space)"

OK. That seems to confirm that part of the theory.

For pre-ctg the spec says:
"Although Buffer Security Indicator is implemented, there is no usage
 model for it and it need not be validated."

So I'm thinking we should never set the non-secure bit on these old
platforms.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/gem_exec_capture.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
> index 1c7d1e7cb..3a1f70567 100644
> --- a/tests/gem_exec_capture.c
> +++ b/tests/gem_exec_capture.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "igt.h"
> +#include "igt_device.h"
>  #include "igt_sysfs.h"
>  
>  #define LOCAL_OBJECT_CAPTURE (1 << 7)
> @@ -141,6 +142,8 @@ static void __capture(int fd, int dir, unsigned ring, uint32_t target)
>  	execbuf.buffers_ptr = (uintptr_t)obj;
>  	execbuf.buffer_count = ARRAY_SIZE(obj);
>  	execbuf.flags = ring;
> +	if (gen > 3 && gen < 6)
> +		execbuf.flags |= I915_EXEC_SECURE;
>  	gem_execbuf(fd, &execbuf);
>  
>  	/* Wait for the request to start */
> @@ -204,7 +207,14 @@ igt_main
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> +		int gen;
> +
>  		fd = drm_open_driver(DRIVER_INTEL);
> +
> +		gen = intel_gen(intel_get_drm_devid(fd));
> +		if (gen > 3 && gen < 6) /* ctg and ilk need secure batches */
> +			igt_device_set_master(fd);
> +
>  		igt_require_gem(fd);
>  		gem_require_mmap_wc(fd);
>  		igt_require(has_capture(fd));
> -- 
> 2.16.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH igt] igt/gem_exec_capture: MI_STORE_DWORD requires EXEC_SECURE + DRM_MASTER on ctg/ilk
Date: Mon, 12 Feb 2018 19:30:52 +0200	[thread overview]
Message-ID: <20180212173052.GU5453@intel.com> (raw)
In-Reply-To: <20180210214338.7611-1-chris@chris-wilson.co.uk>

On Sat, Feb 10, 2018 at 09:43:38PM +0000, Chris Wilson wrote:
> On ctg/ilk, for whatever reason, MI_STORE_DWORD is a privileged operation
> so we must request a SECURE batch.

IIRC ctg supposedly introduced some form of ppgtt. Isn't that the
reason?

Hmm. Now I wonder how anything works on these platforms. Should the
batch itself be executed via ppgtt if it's non-secure? Maybe the hw
has a fallback mechanism of some sort to execute via ggtt if ppgtt
isn't enabled...

ppgtt enable bit:
"When this bit is clear, all memory accesses will be completed using the
GGTT. Privileged memory protections will not be enforced (it is
acceptable for a non-secure batch buffer to access GGTT space)"

OK. That seems to confirm that part of the theory.

For pre-ctg the spec says:
"Although Buffer Security Indicator is implemented, there is no usage
 model for it and it need not be validated."

So I'm thinking we should never set the non-secure bit on these old
platforms.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/gem_exec_capture.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
> index 1c7d1e7cb..3a1f70567 100644
> --- a/tests/gem_exec_capture.c
> +++ b/tests/gem_exec_capture.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "igt.h"
> +#include "igt_device.h"
>  #include "igt_sysfs.h"
>  
>  #define LOCAL_OBJECT_CAPTURE (1 << 7)
> @@ -141,6 +142,8 @@ static void __capture(int fd, int dir, unsigned ring, uint32_t target)
>  	execbuf.buffers_ptr = (uintptr_t)obj;
>  	execbuf.buffer_count = ARRAY_SIZE(obj);
>  	execbuf.flags = ring;
> +	if (gen > 3 && gen < 6)
> +		execbuf.flags |= I915_EXEC_SECURE;
>  	gem_execbuf(fd, &execbuf);
>  
>  	/* Wait for the request to start */
> @@ -204,7 +207,14 @@ igt_main
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> +		int gen;
> +
>  		fd = drm_open_driver(DRIVER_INTEL);
> +
> +		gen = intel_gen(intel_get_drm_devid(fd));
> +		if (gen > 3 && gen < 6) /* ctg and ilk need secure batches */
> +			igt_device_set_master(fd);
> +
>  		igt_require_gem(fd);
>  		gem_require_mmap_wc(fd);
>  		igt_require(has_capture(fd));
> -- 
> 2.16.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2018-02-12 17:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-10 21:43 [igt-dev] [PATCH igt] igt/gem_exec_capture: MI_STORE_DWORD requires EXEC_SECURE + DRM_MASTER on ctg/ilk Chris Wilson
2018-02-10 21:43 ` Chris Wilson
2018-02-10 22:03 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2018-02-12 17:30 ` Ville Syrjälä [this message]
2018-02-12 17:30   ` [igt-dev] [PATCH igt] " Ville Syrjälä
2018-02-12 17:37   ` [Intel-gfx] " Chris Wilson
2018-02-12 17:37     ` Chris Wilson
2018-02-12 18:35     ` Ville Syrjälä
2018-02-12 18:35       ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180212173052.GU5453@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.