[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector ext
From: |
Jan Bobek |
Subject: |
Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions |
Date: |
Tue, 21 May 2019 11:28:57 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 5/20/19 8:30 AM, Alex Bennée wrote:
>
> I'm not sure where my test went wrong but I guess it's around xfeatures.
> The code says required argument but risu doesn't seem to stop me not
> specifying it. I suspect we should default to the most minimal x86_64 we
> can and explicitly enable extra features.
The argument is indeed required, that's taken care of by getopt: to
test that, one can simply specify --xfeatures as the last option on
the command-line. However, we don't check if the value successfully
parses into an integer; is it at all possible that --xfeatures
inadvertently swallowed the next part of your command-line? I shall
add this check in v3.
In any case, we currently default to SSE; this seems reasonable given
that it's an extension dating back some 20 years and pre-dates x86_64
by 4 years (1999 vs. 2003). Opinions?
> Storing xfeat in the stream is a good idea so people don't mix up their
> dumps but we probably need more validation when running in master mode
> that the feature you have enabled is actually available on the
> processor. Otherwise you'll potentially end up generating test streams
> on HW with no support and just get a bunch of undef noise ;-)
Correct me if I'm mistaken, but I believe this should be enforced by
xsave_feature_buf. There's a call to __get_cpuid_count to retrieve
location of a given XSAVE feature in memory, which is asserted to
complete successfully. I assume if the feature were not present, the
assertion would fail. I guess there's a point to be made about
release builds, in which the assert may have been optimized out; shall
I turn it into an error message instead?
> However the series is looking pretty good so far. Looking forward to the
> next iteration.
Once again, thanks a lot for the review, Alex!
-Jan
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [RISU v2 11/11] risu_reginfo_i386: accept named feature sets for --xfeature, (continued)
- [Qemu-devel] [RISU v2 11/11] risu_reginfo_i386: accept named feature sets for --xfeature, Jan Bobek, 2019/05/17
- [Qemu-devel] [RISU v2 08/11] configure: add i386/x86_64 architectures, Jan Bobek, 2019/05/17
- [Qemu-devel] [RISU v2 06/11] risu_i386: remove old unused code, Jan Bobek, 2019/05/17
- [Qemu-devel] [RISU v2 09/11] i386: Add avx512 state to reginfo_t, Jan Bobek, 2019/05/17
- Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions, Alex Bennée, 2019/05/18
- Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions, Alex Bennée, 2019/05/20
- Re: [Qemu-devel] [RISU v2 00/11] Support for i386/x86_64 with vector extensions,
Jan Bobek <=