Re: [RFC] vPMU support for AMD system

["Radim Krčmář" <rkrcmar@redhat.com>]

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.