[PATCH v2 0/1] Documentation/x86: Update the dynamic XSTATE doc

[PATCH v2 0/1] Documentation/x86: Update the dynamic XSTATE doc

[Chang S. Bae]

Hi all,

A couple of changes from the last posting [1]:
* Trim the sentences in the AMX enabling example (Dave Hansen).
* Drop the description for the guest options as this change alone does not
  cover the practical (KVM) use case. Will follow up with a new series for
  this.

Sending this to the Intel reviewers again as one of their feedback was
just addressed.

=== Cover Letter ===

With the AMX support in the mainline, recently I heard some folks had a
hard time understanding the AMX enabling process. A code example is
expected to clarify the steps.

The arch_prctl(2) manual page [2] is missing these new options. Perhaps,
the man-page update follows up along with this.

These changes can be found in the repo:
  git://github.com/intel/amx-linux.git doc

And the compiled preview is available here:
  https://htmlpreview.github.io/?https://github.com/intel/amx-linux/doc-web/x86/xstate.html

Thanks,
Chang

[1] https://lore.kernel.org/lkml/20220616212210.3182-1-chang.seok.bae@intel.com/
[2] arch_prctl(2): https://man7.org/linux/man-pages/man2/arch_prctl.2.html


Chang S. Bae (1):
  Documentation/x86: Add the AMX enabling example

 Documentation/x86/xstate.rst | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)


base-commit: 03c765b0e3b4cb5063276b086c76f7a612856a9a
-- 
2.17.1

[PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Chang S. Bae]

Explain steps to enable the dynamic feature with a code example.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes from v1:
* Update the description without mentioning CPUID & XGETBV (Dave Hansen).
---
 Documentation/x86/xstate.rst | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/Documentation/x86/xstate.rst b/Documentation/x86/xstate.rst
index 5cec7fb558d6..c439901419fb 100644
--- a/Documentation/x86/xstate.rst
+++ b/Documentation/x86/xstate.rst
@@ -64,6 +64,48 @@ the handler allocates a larger xstate buffer for the task so the large
 state can be context switched. In the unlikely cases that the allocation
 fails, the kernel sends SIGSEGV.
 
+AMX TILE_DATA enabling example
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The following steps dynamically enable TILE_DATA:
+
+  1. An application first needs to determine the feature support::
+
+        #include <asm/prctl.h>
+        #include <sys/syscall.h>
+        #include <stdio.h>
+        #include <unistd.h>
+
+        #define ARCH_GET_XCOMP_SUPP  0x1021
+
+        #define XFEATURE_XTILECFG    17
+        #define XFEATURE_XTILEDATA   18
+        #define XFEATURE_MASK_XTILE ((1 << XFEATURE_XTILECFG) | (1 << XFEATURE_XTILEDATA))
+
+        unsigned long features;
+        long rc;
+
+        ...
+
+        rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_SUPP, &features);
+
+        if (!rc && (features & XFEATURE_MASK_XTILE) == XFEATURE_MASK_XTILE)
+            printf("AMX is available.\n");
+
+  2. After determining support for AMX, an application must explicitly ask
+     permission to use it::
+
+        #define ARCH_REQ_XCOMP_PERM  0x1023
+
+        ...
+
+        rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_PERM, XFEATURE_XTILEDATA);
+
+        if (!rc)
+            printf("AMX is ready for use.\n");
+
+Note this example does not include the sigaltstack preparation.
+
 Dynamic features in signal frames
 ---------------------------------
 
-- 
2.17.1

RE: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Luck, Tony]

+        #define ARCH_GET_XCOMP_SUPP  0x1021
+
+        #define XFEATURE_XTILECFG    17
+        #define XFEATURE_XTILEDATA   18

What's the long-term plan for these #defines?  I see that ARCH_GET_XCOMP_SUPP
is in arch/x86/include/uapi/asm/prctl.h ... so eventually that will show up in distribution
/usr/include/asm/prctl.h courtesy of a glibc update.

But the XFEATURE bits aren't in a "uapi" file. They are an "enum" in:

arch/x86/include/asm/fpu/types.h

How will that get to /usr/include?

-Tony

Re: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Chang S. Bae]

On 6/29/2022 4:30 PM, Luck, Tony wrote:
> +        #define ARCH_GET_XCOMP_SUPP  0x1021
> +
> +        #define XFEATURE_XTILECFG    17
> +        #define XFEATURE_XTILEDATA   18
> 
> What's the long-term plan for these #defines?  I see that ARCH_GET_XCOMP_SUPP
> is in arch/x86/include/uapi/asm/prctl.h ... so eventually that will show up in distribution
> /usr/include/asm/prctl.h courtesy of a glibc update.
> 
> But the XFEATURE bits aren't in a "uapi" file. They are an "enum" in:
> 
> arch/x86/include/asm/fpu/types.h
> 
> How will that get to /usr/include?

Perhaps I can think of something like this for AMX state:

diff --git a/arch/x86/include/uapi/asm/prctl.h 
b/arch/x86/include/uapi/asm/prctl.h
index 500b96e71f18..08b6c9155b8a 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -16,6 +16,9 @@
  #define ARCH_GET_XCOMP_GUEST_PERM      0x1024
  #define ARCH_REQ_XCOMP_GUEST_PERM      0x1025

+#define ARCH_XCOMP_XTILECFG            17
+#define ARCH_XCOMP_XTILEDATA           18
+
  #define ARCH_MAP_VDSO_X32              0x2001
  #define ARCH_MAP_VDSO_32               0x2002
  #define ARCH_MAP_VDSO_64               0x2003

But these state components are architectural. While this can help 
userspace anyway, saying "XSTATE component" here and on the man-page is 
probably it as they are already defined in the x86 spec.

Thanks,
Chang

RE: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Luck, Tony]

> But these state components are architectural. While this can help 
> userspace anyway, saying "XSTATE component" here and on the man-page is 
> probably it as they are already defined in the x86 spec.

An application writer can't use:

# include {x86 spec}"

to get these values ... if applications need them to find out if AMX is present,
and to enable it, then they need an API.

Maybe your example code should just be a library routine? So application writers
can just do:

	if (!intel_amx_enable()) {
		error message, or fall back to non-AMX implementation
	}

without having to worry about those #defines.

-Tony

Re: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Chang S. Bae]

On 6/30/2022 9:07 AM, Luck, Tony wrote:
>> But these state components are architectural. While this can help
>> userspace anyway, saying "XSTATE component" here and on the man-page is
>> probably it as they are already defined in the x86 spec.
> 
> An application writer can't use:
> 
> # include {x86 spec}"
> 
> to get these values ... if applications need them to find out if AMX is present,
> and to enable it, then they need an API.

Yeah, you're right.

> 
> Maybe your example code should just be a library routine? So application writers
> can just do:
> 
> if (!intel_amx_enable()) {
> error message, or fall back to non-AMX implementation
> }
> 
> without having to worry about those #defines.

I guess we cannot assume this. Given the arch_prctl() options, they can 
do this by themselves.

Let me include those defines on the next version.

Thanks,
Chang

Re: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Bagas Sanjaya]

On 6/30/22 05:42, Chang S. Bae wrote:
> Explain steps to enable the dynamic feature with a code example.
> 
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> Changes from v1:
> * Update the description without mentioning CPUID & XGETBV (Dave Hansen).
> ---
>  Documentation/x86/xstate.rst | 42 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/Documentation/x86/xstate.rst b/Documentation/x86/xstate.rst
> index 5cec7fb558d6..c439901419fb 100644
> --- a/Documentation/x86/xstate.rst
> +++ b/Documentation/x86/xstate.rst
> @@ -64,6 +64,48 @@ the handler allocates a larger xstate buffer for the task so the large
>  state can be context switched. In the unlikely cases that the allocation
>  fails, the kernel sends SIGSEGV.
>  
> +AMX TILE_DATA enabling example
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The following steps dynamically enable TILE_DATA:
> +

This should be "Below is the example of how userspace applications enable
TILE_DATA dynamically:"

> +  1. An application first needs to determine the feature support::
> +

Better say "The application first needs to query the kernel for AMX
support".

> +        #include <asm/prctl.h>
> +        #include <sys/syscall.h>
> +        #include <stdio.h>
> +        #include <unistd.h>
> +
> +        #define ARCH_GET_XCOMP_SUPP  0x1021
> +
> +        #define XFEATURE_XTILECFG    17
> +        #define XFEATURE_XTILEDATA   18
> +        #define XFEATURE_MASK_XTILE ((1 << XFEATURE_XTILECFG) | (1 << XFEATURE_XTILEDATA))
> +
> +        unsigned long features;
> +        long rc;
> +
> +        ...
> +
> +        rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_SUPP, &features);
> +
> +        if (!rc && (features & XFEATURE_MASK_XTILE) == XFEATURE_MASK_XTILE)
> +            printf("AMX is available.\n");
> +
> +  2. After determining support for AMX, an application must explicitly ask
> +     permission to use it::
> +

Shorter is "After that,..."

> +        #define ARCH_REQ_XCOMP_PERM  0x1023
> +
> +        ...
> +
> +        rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_PERM, XFEATURE_XTILEDATA);
> +
> +        if (!rc)
> +            printf("AMX is ready for use.\n");
> +
> +Note this example does not include the sigaltstack preparation.
> +

I guess "application" here means userspace application, right?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2 1/1] Documentation/x86: Add the AMX enabling example

[Chang S. Bae]

On 6/29/2022 8:03 PM, Bagas Sanjaya wrote:

<snip>
> 
> I guess "application" here means userspace application, right?

Yes.

Thanks for the review! Will pick your sentences.

Chang