<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">On 22.01.2021 19:20, Paul Gofman wrote:<br>
</div>
<blockquote type="cite"
cite="mid:211bf638-b17e-d9f4-e1d7-c85e98cdc664@codeweavers.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div class="moz-cite-prefix">On 1/22/21 21:03, Jacek Caban wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e1cc26f2-e894-bbc4-9dfa-c9184c6d963f@codeweavers.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<div class="moz-cite-prefix">On 22.01.2021 18:44, Paul Gofman
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:937810cd-ca31-c2b2-4a97-7c5c44c76c43@codeweavers.com">
<blockquote type="cite" style="color: #007cff;">
<pre class="moz-quote-pre" wrap="">xsave is part of SSE2, not AVX, and it should ignore unsupported
requested features, so the patch should be fine as is on hardware
without AVX. xsave needs, however, to be enabled by OS, so we may need
a feature check if we want to support OSes without xsave enabled.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">There was a real bug with that:
<a class="moz-txt-link-freetext" href="https://bugs.winehq.org/show_bug.cgi?id=50271" moz-do-not-send="true">https://bugs.winehq.org/show_bug.cgi?id=50271</a>.
</pre>
</blockquote>
<p><br>
</p>
<p>Looking at the bug report, I think it was about missing AVX
support, not missing XSAVE support, so it should be fine with
my patches.</p>
<p><br>
</p>
</blockquote>
<p>It was faulting exactly on xrstor instruction, with the mask
being taken from what Linux gives us in its xsave structure. I
didn't check the mask that time, so I can't fully exclude the
possibility that it was the wrong mask somehow and it wouldn't
fail with a zero mask, but I got an impression that it is xrstor
instruction itself is unsupported. And apparently there wasn't
cpuid XSAVE flag set, that's how the fix there is helping. Also,
xsave instruction documentation explicitly refers XSAVE CPUID
feature flag.</p>
</blockquote>
<p><br>
</p>
<p>It's mentioned in exceptions section of documentation that you
linked:</p>
<p>"#UD If CPUID.01H:ECX.XSAVE[bit 26] = 0."</p>
<p>There is no AVX check. (There is also If "CR4.OSXSAVE[bit 18] =
0.", which is how OS may decide to support it or not).<br>
</p>
<br>
<p>I'm not sure why you conclude that XSAVE CPUID flag wasn't set in
case of the bug. With current code, both AVX and XSAVE flags to
set EnabledFeatures, so it wouldn't be set even if XSAVE is
supported but AVX not.<br>
</p>
<p><br>
</p>
<p>Jacek<br>
</p>
<p><br>
</p>
<p>[1]
<a class="moz-txt-link-freetext" href="https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf">https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf</a><br>
</p>
</body>
</html>