<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 3/14/2024 10:09 PM, Dave Hansen
wrote:<br>
</div>
<blockquote type="cite" cite="mid:24f71d52-0891-4cfc-8dec-9f13ed618eee@intel.com">
<pre class="moz-quote-pre" wrap="">Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On 3/14/24 09:29, Borislav Petkov wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">That argument breaks down a bit on the flags though:
xc.xfeat_flags = xstate_flags[i];
Because it comes _directly_ from CPUID with zero filtering:
cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx);
...
xstate_flags[i] = ecx;
So this layout is quite dependent on what's in x86's CPUID.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Yeah, no, this should not be copying CPUID flags - those flags should be
*translated* to independently defined flags which describe those
buffers.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Ditto for:
xc.xfeat_type = i;
Right now, that's bound to CPUID and XSAVE. "feat_type==10" can only
ever be PKRU and that's derived from the XSAVE architecture.
If you want this to be extensible to things outside of the XSAVE
architecture, it needs to be something actually extensible and not
entangled with XSAVE.
In other words "xc.xfeat_type" can enumerate XSAVE state components
being in the dump, but it should not be limited to XSAVE. Just as an
example:
enum feat_type {
FEATURE_XSAVE_PKRU,
FEATURE_XSAVE__YMM,
FEATURE_XSAVE_BNDREGS,
FEATURE_XSAVE_BNDCSR,
...
RANDOM_STATE_NOT_XSAVE
};
See how feat_type==1 is PKRU and *NOT* feat_type==10? That opens the
door to RANDOM_STATE_NOT_XSAVE or anything else you want. This would be
_actually_ extensible.</pre>
</blockquote>
<p><br>
</p>
<p>Thanks for the review.<br>
<span><span class="ui-provider a b c d e f g h i j k l m n o p q r s t u v w x y z ab ac ae af ag ah ai aj ak" dir="ltr">I will add new enum, instead of using "enum
xfeature". <br>
</span></span><span><span class="ui-provider a b c d e f g h i j k l m n o p q r s t u v w x y z ab ac ae af ag ah ai aj ak" dir="ltr">Currently we are retaining the flags field. The
value will be set to zero at this point, and the field will be
reserved for future use.<br>
GDB / LLDB would not require this field at this point. Do let
us know if this is not OK. <br>
<br>
-thanks,<br>
vigneshbalu.<br>
</span></span></p>
<blockquote type="cite" cite="mid:24f71d52-0891-4cfc-8dec-9f13ed618eee@intel.com">
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>