On Mon, Jan 31, 2022, Paolo Bonzini wrote: > On 1/28/22 16:42, Sean Christopherson wrote: > All in all I totally agree with patches 2-11 and will apply them (patch 2 to > 5.17 even, as a prerequisite to fix the AVIC race). Several of patches > 13-21 are also mostly useful as it clarifies the code, and the others I > guess are okay in the context of a coherent series though probably they > would have been rejected as one-offs. Yeah, the SEV changes in particular are a bit forced. The only one I care deeply about is mem_enc_op() => mem_enc_ioctl(). If the macro shenanigans are rejected, I'd say drop patches 20 and 21, drop most of 19, and maybe give 18 (svm=>avic) the boot as well. I'd prefer to keep patch 17 (TLB tweak) to clarify the scope of SVM's TLB flush. Many of the changelogs would need to be tweaked as well, i.e. a v2 is in order. > However, patches 12 and 22 are unnecessary uses of the C preprocessor in my > opinion. And 14 :-) I don't have a super strong opinion. I mostly worked on this because the idea had been discussed multiple times in the past. And because I wanted an excuse to rename vmx_free_vcpu => vmx_vcpu_free, which for some reason I can never find :-) I was/am concerned that the macro approach will make it more difficult to find a vendor's implementation, though forcing a conforming name will mitigate that to some degree. The pros, in order of importance (IMO) 1. Mostly forces vendor implementation name to match hook name 2. Forces new hooks to get an entry in kvm-x86-ops.h 3. Provides a bit of documentation for specialized hooks (APICv, etc...) 4. Forces vendors to explicitly define something for non-conforming hooks
