Code review comment for ~rafaeldtinoco/ubuntu/+source/qemu:lp1828495-bionic-devel-qemu

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

#12 - has again dropped CPUID_7_0_EBX_INTEL_PT which is correct.
But it wasn't mentioned in the patch description.

Here - as in the other patch dropping CPUID_7_0_EBX_INTEL_PT before (that was #5) I recommend the split of patches. That means as discussed on #7 I'd ask you to take #5 and #12 as-is from upstream.
And then add an extra cleanup patch (in an extra commit) dropping CPUID_7_0_EBX_INTEL_PT as this only got added in 2.12.
Again having this in an extra commit/patch will allow to more easily port it to Cosmic/Disco/Eoan.

In fact the "4c257911dcc7c4189768e9651755c849ce9db4e8" that I recommended to add as-is before would fix this. It removes it from icelake as well as cascade-lake. Not for lackign the feature, but for technically not being right to be on by default.
That meand #5 and #12 could become four patches:

8a11c62da914
c7a88b52f62
76e5a4d58357b9d077afccf7f7c82e17f733b722
4c257911dcc7c4189768e9651755c849ce9db4e8

And those would not even need any special 2.12 dependent cleanup.

review: Needs Fixing

« Back to merge proposal