<div dir="ltr"><div dir="ltr"><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">...<br>
<br>
Running make check would reveal all these issues because every single<br>
test which involves parsing the cpu_map was failing due to multiple<br>
definitions of the same CPU model.<br>
<br></blockquote><div>I'd not have expected that the tests will exercise the new XMLs in any way.</div><div>Good to know and thanks for your feedback.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
And since this patch is adding several CPU models which are already<br>
supported by QEMU since 4.2.0, you need to update several existing test<br>
files for domaincapstest too. You can use<br>
<br>
    VIR_TEST_REGENERATE_OUTPUT=1 tests/domaincapstest<br>
<br>
to regenerate the files. Just make sure you review the changes before<br>
adding them to this commit.<br></blockquote><div><br></div><div>Without regenerating these I see as expected</div><div>FAIL: domaincapstest<br>FAIL: cputest<br></div><div><br></div></div><div class="gmail_quote"><div>Adding these 5 types to the qemu 4.2 and qemu 5.0 tests/domaincapsdata worked.</div><div>I've squashed that into the same patch - let me know if you'd prefer the domaincapsdata change as an individual patch instead.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Regenerating the test files will also be needed for cputest because I<br>
just pushed the "cputest: Add data for Intel(R) Core(TM) i7-8550U CPU<br>
without TSX". Doing so will nicely show that the computed host CPU model<br>
(in x86_64-cpuid-Core-i7-8550U-host.xml file) is<br>
Skylake-Client-noTSX-IBRS rather than Broadwell-noTSX-IBRS.<br></blockquote><div><br></div><div>Indeed:</div><div><div class="gmail_quote">1007 In '/home/paelzer/work/libvirt/libvirt-ubuntu-git/build/../tests/cputestdata/x86_64-cpuid-Core-i7-8550U-host.xml':<br>...<br>1009 Expect [Broadwell-noTSX-IBRS</model></div><div class="gmail_quote">...<br>1043 Actual [Skylake-Client-noTSX-IBRS</model> <div> <br></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
However, the CPU used for host-model (and reported in domain<br>
capabilities) as shown in x86_64-cpuid-Core-i7-8550U-guest.xml and<br>
x86_64-cpuid-Core-i7-8550U-json.xml will change from Skylake-Client-IBRS<br>
to Skylake-Client-noTSX-IBRS. As I said in my previous reply to this<br>
patch, I think these two CPU definitions should keep using the old<br>
Skylake-Client-IBRS model to make sure any domain with host-model CPU<br>
will always use the CPU models without noTSX for better compatibility<br>
between current and future version of libvirt.</blockquote><br>I see three changes:<br>x86_64-cpuid-Core-i7-8550U-host.xml: Broadwell-noTSX-IBRS -> Skylake-Client-noTSX-IBRS<br>x86_64-cpuid-Core-i7-8550U-guest.xml: Skylake-Client-IBRS -> Skylake-Client-noTSX-IBRS<br>This shows up twice in:<br>238) cpuTestGuestCPUID(x86_64): Core-i7-8550U<br>240) cpuTestGuestCPUID(x86_64): Core-i7-8550U<br>x86_64-cpuid-Core-i7-8550U-json.xml: Skylake-Client-IBRS -> Skylake-Client-noTSX-IBRS<br><br>So far so good and as expected.<br>But I have to beg your pardon and need to ask where such an override to continue to use<br>"Skylake-Client-IBRS + policy='disable' name='hle' policy='disable' name='rtm'" instead of just "Skylake-Client-noTSX-IBRS" would have to go?<div><br></div><div>Holding back v2 submission until this is solved ...</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This change should be in<br>
a separate patch, but in single series with the current patch.<br>
<br>
Jirka<br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">Christian Ehrhardt<br>Staff Engineer, Ubuntu Server<br>Canonical Ltd</div></div>