On 03.06.2024 13:32, Sergiy Kibrik wrote: > If VMX/SVM disabled in the build, we may still want to have vPMU drivers for > PV guests. Yet in such case before using VMX/SVM features and functions we > have > to explicitly check if they're available in the build. For this puspose
Nit: It's not the first time that I see this apparent typo - I assume here and elsewhere "purpose" is meant? > --- a/xen/arch/x86/cpu/vpmu_amd.c > +++ b/xen/arch/x86/cpu/vpmu_amd.c > @@ -27,6 +27,7 @@ > #define is_pmu_enabled(msr) ((msr) & (1ULL << MSR_F10H_EVNTSEL_EN_SHIFT)) > #define set_guest_mode(msr) ((msr) |= (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT)) > #define is_overflowed(msr) (!((msr) & (1ULL << (MSR_F10H_COUNTER_LENGTH - > 1)))) > +#define is_svm_vcpu(v) (using_svm && is_hvm_vcpu(v)) Like for the earlier patch the implicit cpu_has_svm check you add here is redundant with us knowing we're dealing with AMD hardware here. Please consider switching to IS_ENABLED(). > --- a/xen/arch/x86/cpu/vpmu_intel.c > +++ b/xen/arch/x86/cpu/vpmu_intel.c > @@ -54,6 +54,8 @@ > #define MSR_PMC_ALIAS_MASK (~(MSR_IA32_PERFCTR0 ^ MSR_IA32_A_PERFCTR0)) > static bool __read_mostly full_width_write; > > +#define is_vmx_vcpu(v) ( using_vmx && is_hvm_vcpu(v) ) Nit (style): Unlike above, but like iirc you had it elsewhere, you have stray blanks here immediately inside the parentheses. Beyond that the comment above applies here, too (with s/AMD/Intel). Jan
