diff mbox series

[6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors

Message ID 20220830031206.13449-7-jarkko@kernel.org (mailing list archive)
State New, archived
Headers show
Series x86/sgx: Test and fixes | expand

Commit Message

Jarkko Sakkinen Aug. 30, 2022, 3:12 a.m. UTC
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/alloc-error.bt | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 tools/testing/selftests/sgx/alloc-error.bt

Comments

Reinette Chatre Aug. 30, 2022, 10:57 p.m. UTC | #1
Hi Jarkko,

On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  tools/testing/selftests/sgx/alloc-error.bt | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
> 
> diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> new file mode 100644
> index 000000000000..9268d50dea29
> --- /dev/null
> +++ b/tools/testing/selftests/sgx/alloc-error.bt
> @@ -0,0 +1,7 @@
> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> +}
> +
> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> +}


Could there be a snippet of comments in this new file to guide users
on how to use this script?

Reinette
Jarkko Sakkinen Aug. 31, 2022, 2:33 a.m. UTC | #2
On Tue, Aug 30, 2022 at 03:57:24PM -0700, Reinette Chatre wrote:
> Hi Jarkko,
> 
> On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  tools/testing/selftests/sgx/alloc-error.bt | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >  create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
> > 
> > diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> > new file mode 100644
> > index 000000000000..9268d50dea29
> > --- /dev/null
> > +++ b/tools/testing/selftests/sgx/alloc-error.bt
> > @@ -0,0 +1,7 @@
> > +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> > +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> > +}
> > +
> > +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> > +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> > +}
> 
> 
> Could there be a snippet of comments in this new file to guide users
> on how to use this script?

Do not mean to be rude but I'm not sure what there is to guide but
I'm open for ideas.

BR, Jarkko
Reinette Chatre Aug. 31, 2022, 6:10 p.m. UTC | #3
Hi Jarkko,

On 8/30/2022 7:33 PM, Jarkko Sakkinen wrote:
> On Tue, Aug 30, 2022 at 03:57:24PM -0700, Reinette Chatre wrote:
>> Hi Jarkko,
>>
>> On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>> ---
>>>  tools/testing/selftests/sgx/alloc-error.bt | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>  create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
>>>
>>> diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
>>> new file mode 100644
>>> index 000000000000..9268d50dea29
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/sgx/alloc-error.bt
>>> @@ -0,0 +1,7 @@
>>> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
>>> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
>>> +}
>>> +
>>> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
>>> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
>>> +}
>>
>>
>> Could there be a snippet of comments in this new file to guide users
>> on how to use this script?
> 
> Do not mean to be rude but I'm not sure what there is to guide but
> I'm open for ideas.

How about something like below in comments as part of the script:

"bpftrace script using kretprobe to trace returns of some key functions
 in support of tracking allocation errors."

This is essentially in the subject line of the patch but that information
will be lost when somebody looks at the script and tries to figure
out what it is.

Reinette
Jarkko Sakkinen Aug. 31, 2022, 6:23 p.m. UTC | #4
On Wed, Aug 31, 2022 at 11:10:20AM -0700, Reinette Chatre wrote:
> Hi Jarkko,
> 
> On 8/30/2022 7:33 PM, Jarkko Sakkinen wrote:
> > On Tue, Aug 30, 2022 at 03:57:24PM -0700, Reinette Chatre wrote:
> >> Hi Jarkko,
> >>
> >> On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
> >>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>> ---
> >>>  tools/testing/selftests/sgx/alloc-error.bt | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>  create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
> >>>
> >>> diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> >>> new file mode 100644
> >>> index 000000000000..9268d50dea29
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/sgx/alloc-error.bt
> >>> @@ -0,0 +1,7 @@
> >>> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> >>> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> >>> +}
> >>> +
> >>> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> >>> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> >>> +}
> >>
> >>
> >> Could there be a snippet of comments in this new file to guide users
> >> on how to use this script?
> > 
> > Do not mean to be rude but I'm not sure what there is to guide but
> > I'm open for ideas.
> 
> How about something like below in comments as part of the script:
> 
> "bpftrace script using kretprobe to trace returns of some key functions
>  in support of tracking allocation errors."

I think comments that I put (before seeing) also
make it clear enough (not to say that what you
had not been a valid alternative).

BR, Jarkko
Dave Hansen Aug. 31, 2022, 6:23 p.m. UTC | #5
On 8/29/22 20:12, Jarkko Sakkinen wrote:
> diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> new file mode 100644
> index 000000000000..9268d50dea29
> --- /dev/null
> +++ b/tools/testing/selftests/sgx/alloc-error.bt
> @@ -0,0 +1,7 @@
> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> +}
> +
> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> +}

I guess this doesn't _hurt_, but it's also not exactly the easiest way
to get this done.  You need a whole bpf toolchain.  You could also just do:

	perf probe 'sgx_encl_page_alloc%return $retval'

Even *that* can be replicated in a few scant lines of shell code echoing
into /sys/kernel/debug/tracing.
Jarkko Sakkinen Sept. 1, 2022, 10:20 p.m. UTC | #6
On Wed, Aug 31, 2022 at 11:23:55AM -0700, Dave Hansen wrote:
> On 8/29/22 20:12, Jarkko Sakkinen wrote:
> > diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> > new file mode 100644
> > index 000000000000..9268d50dea29
> > --- /dev/null
> > +++ b/tools/testing/selftests/sgx/alloc-error.bt
> > @@ -0,0 +1,7 @@
> > +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> > +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> > +}
> > +
> > +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> > +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> > +}
> 
> I guess this doesn't _hurt_, but it's also not exactly the easiest way
> to get this done.  You need a whole bpf toolchain.  You could also just do:
> 
> 	perf probe 'sgx_encl_page_alloc%return $retval'
> 
> Even *that* can be replicated in a few scant lines of shell code echoing
> into /sys/kernel/debug/tracing.

Thanks, I have not used perf that much. What if I replace
this with a shell script using perf? How do you use that
for two kretprobes?

BR, Jarkko
Dave Hansen Sept. 1, 2022, 10:34 p.m. UTC | #7
On 9/1/22 15:20, Jarkko Sakkinen wrote:
>>> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
>>> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
>>> +}
>>> +
>>> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
>>> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
>>> +}
>> I guess this doesn't _hurt_, but it's also not exactly the easiest way
>> to get this done.  You need a whole bpf toolchain.  You could also just do:
>>
>> 	perf probe 'sgx_encl_page_alloc%return $retval'
>>
>> Even *that* can be replicated in a few scant lines of shell code echoing
>> into /sys/kernel/debug/tracing.
> Thanks, I have not used perf that much. What if I replace
> this with a shell script using perf? How do you use that
> for two kretprobes?

The manpage is pretty good.

But, I'd proably be doing something along these lines:

	perf probe 'sgx_encl_page_alloc%return ret=$retval'
	perf record -e probe:sgx_encl_page_alloc -aR	\
		--filter='ret >= 0xwhatever' sleep 1
	perf script

There are probably shorter ways to do it, but I'm pretty sure that works.
Jarkko Sakkinen Sept. 1, 2022, 11:55 p.m. UTC | #8
On Thu, Sep 01, 2022 at 03:34:49PM -0700, Dave Hansen wrote:
> On 9/1/22 15:20, Jarkko Sakkinen wrote:
> >>> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> >>> +		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> >>> +}
> >>> +
> >>> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> >>> +		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> >>> +}
> >> I guess this doesn't _hurt_, but it's also not exactly the easiest way
> >> to get this done.  You need a whole bpf toolchain.  You could also just do:
> >>
> >> 	perf probe 'sgx_encl_page_alloc%return $retval'
> >>
> >> Even *that* can be replicated in a few scant lines of shell code echoing
> >> into /sys/kernel/debug/tracing.
> > Thanks, I have not used perf that much. What if I replace
> > this with a shell script using perf? How do you use that
> > for two kretprobes?
> 
> The manpage is pretty good.
> 
> But, I'd proably be doing something along these lines:
> 
> 	perf probe 'sgx_encl_page_alloc%return ret=$retval'
> 	perf record -e probe:sgx_encl_page_alloc -aR	\
> 		--filter='ret >= 0xwhatever' sleep 1
> 	perf script
> 
> There are probably shorter ways to do it, but I'm pretty sure that works.

Thanks, will give it a shot.

BR, Jarkko
diff mbox series

Patch

diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
new file mode 100644
index 000000000000..9268d50dea29
--- /dev/null
+++ b/tools/testing/selftests/sgx/alloc-error.bt
@@ -0,0 +1,7 @@ 
+kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
+		 printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
+}
+
+kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
+		 printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
+}