2014-09-30 21:07-0500, Wei Huang: > Hi Paolo and Gleb, > 2. CURRENT DESIGN > The design is similar to existing vPMU support. Many data structures such as > "struct kvm_pmc" and "struct kvm_pmu" are re-used for this design. > > Original pmu.c code was designed with Intel PMU in mind. To avoid the > confusion, the current design converts existing pmu.c into intel-pmu.c and > create a similar code, named amd-pmu.c. PMU function calls in x86.c are then > dispatched via function pointers (i.e. struct kvm_pmu_ops). The initial > setup is done in kvm_pmu_arch_init(). > > I have other ideas listed below. Please review and give some suggestion: > > (a) Merge intel-pmu.c and amd-pmu.c into pmu.c > * PMU will probe the architecture first before first usage; > * It will still use function pointers to dispatch global functions (e.g. > kvm_pmu_reset); > * For static functions that are the same (e.g. pmc_is_gp), we will use the > same copy; > * For static functions that are slightly different (e.g. global_idx_to_pmc), > we will use a shared version with if-else statement, depending on Intel or > AMD. (if-else forest is daunting.) > I think this approach is quite acceptable, except that there will be many > if-else in the code. Not clean enough. > > (b) Convert intel-pmu.c => vmx.c and convert amd-pmu.c => svm.c > * PMU function pointers will be created in kvm_x86_ops; > * The entry functions will be created inside vmx.c and svm.c respectively; I would be nicer to keep them in separate files and link to vmx/svm. > * There might be common functions defined in pmu.c. > > This design is viable too. But to be honest, it is a bit messy compared with > (a). This makes sense as we will use only vmx+intel_pmu and svm+amd_pmu, so we'd have less code loaded in both cases. I consider design (c) strictly better than the current one (2): (c) keep {intel,amd}-pmu.c and introduce pmu.[hc] that joins duplicate functions and wraps kvm_pmu_ops And if we decide to move VMX/SVM related code into their respective modules, we won't have to change callers. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html