Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.03.2023 22:51, Andrew Cooper wrote:
> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
> to not operate on objects of differing lifetimes, so structs
> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
> 
> But this does mean that we now have
> 
>   cpu_policy->basic.$X
>   cpu_policy->feat.$Y
>   cpu_policy->arch_caps.$Z
> 
> and plenty of code now reads
> 
>   d->arch.cpu_policy->feat.$Y
> 
> instead of
> 
>   d->arch.cpuid->feat.$Y
> 
> The latter can be half-fixed with some union magic (see patch 9 commit
> message).  The former can be fixed by putting cpuid/msr infixes in cpu_policy,
> which is doable but very invasive, and would make plenty of code read
> 
>   d->arch.cpu_policy->cpuid.feat.$Y
> 
> and the two obviously shouldn't be done together.
> 
> So, RFC.  Does this code layout look ok?  If we want to make changes with
> naming, now is very much the right time to get them sorted.
> 
> Patches 1-8 are pretty ready to go.  Patch 9 is the remainder to take out the
> temporary hacks, and I'm still in the process of merging the system policy
> derivation.
> 
> Andrew Cooper (9):
>   x86: Rename struct cpu_policy to struct old_cpuid_policy
>   x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields

Nit: I guess the last closing brace wants moving forward a little.

>   x86: Rename struct cpuid_policy to struct cpu_policy
>   x86: Merge struct msr_policy into struct cpu_policy
>   x86: Merge the system {cpuid,msr} policy objects
>   x86: Merge a domain's {cpuid,msr} policy objects
>   x86: Merge xc_cpu_policy's cpuid and msr objects
>   x86: Drop struct old_cpu_policy

With the small comments on individual patches taken care of one way or
another, up to here:

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

>   RFC: Everything else
> 
>  tools/fuzz/cpu-policy/afl-policy-fuzzer.c     |  15 +-
>  .../fuzz/x86_instruction_emulator/fuzz-emul.c |   2 +-
>  tools/libs/guest/xg_cpuid_x86.c               |  48 +-
>  tools/libs/guest/xg_private.h                 |   5 +-
>  tools/tests/cpu-policy/test-cpu-policy.c      |  50 +-
>  tools/tests/tsx/test-tsx.c                    |  58 +-
>  tools/tests/x86_emulator/Makefile             |   2 +-
>  tools/tests/x86_emulator/test_x86_emulator.c  |   2 +-
>  tools/tests/x86_emulator/x86-emulate.c        |   2 +-
>  tools/tests/x86_emulator/x86-emulate.h        |   2 +-
>  xen/arch/x86/Makefile                         |   1 +
>  xen/arch/x86/cpu-policy.c                     |  67 +++
>  xen/arch/x86/cpu/common.c                     |   4 +-
>  xen/arch/x86/cpu/mcheck/mce_intel.c           |   2 +-
>  xen/arch/x86/cpu/vpmu_intel.c                 |   4 +-
>  xen/arch/x86/cpuid.c                          | 101 ++--
>  xen/arch/x86/domain.c                         |  18 +-
>  xen/arch/x86/domctl.c                         |  51 +-
>  xen/arch/x86/hvm/emulate.c                    |   2 +-
>  xen/arch/x86/hvm/hvm.c                        |  38 +-
>  xen/arch/x86/hvm/ioreq.c                      |   4 +-
>  xen/arch/x86/hvm/mtrr.c                       |   2 +-
>  xen/arch/x86/hvm/svm/svm.c                    |  18 +-
>  xen/arch/x86/hvm/svm/svmdebug.c               |   2 +-
>  xen/arch/x86/hvm/vlapic.c                     |   2 +-
>  xen/arch/x86/hvm/vmx/vmx.c                    |  12 +-
>  xen/arch/x86/hvm/vmx/vvmx.c                   |   2 +-
>  xen/arch/x86/include/asm/cpu-policy.h         |  18 +
>  xen/arch/x86/include/asm/cpuid.h              |  10 -
>  xen/arch/x86/include/asm/domain.h             |   4 +-
>  xen/arch/x86/include/asm/guest_pt.h           |   4 +-
>  xen/arch/x86/include/asm/msr.h                |  13 +-
>  xen/arch/x86/include/asm/paging.h             |   2 +-
>  xen/arch/x86/mm/mem_sharing.c                 |   3 +-
>  xen/arch/x86/mm/shadow/hvm.c                  |   2 +-
>  xen/arch/x86/msr.c                            |  98 +---
>  xen/arch/x86/pv/domain.c                      |   2 +-
>  xen/arch/x86/pv/emul-priv-op.c                |   6 +-
>  xen/arch/x86/pv/ro-page-fault.c               |   2 +-
>  xen/arch/x86/sysctl.c                         |  77 +--
>  xen/arch/x86/traps.c                          |   2 +-
>  xen/arch/x86/x86_emulate.c                    |   2 +-
>  xen/arch/x86/x86_emulate/x86_emulate.c        | 166 +++---
>  xen/arch/x86/x86_emulate/x86_emulate.h        |   6 +-
>  xen/arch/x86/xstate.c                         |   4 +-
>  xen/include/public/domctl.h                   |   4 +-
>  xen/include/public/sysctl.h                   |   4 +-
>  xen/include/xen/lib/x86/cpu-policy.h          | 540 +++++++++++++++++-
>  xen/include/xen/lib/x86/cpuid.h               | 475 ---------------
>  xen/include/xen/lib/x86/msr.h                 | 104 ----
>  xen/lib/x86/cpuid.c                           |  12 +-
>  xen/lib/x86/msr.c                             |   6 +-
>  xen/lib/x86/policy.c                          |   8 +-
>  53 files changed, 986 insertions(+), 1104 deletions(-)
>  create mode 100644 xen/arch/x86/cpu-policy.c
>  create mode 100644 xen/arch/x86/include/asm/cpu-policy.h
>  delete mode 100644 xen/include/xen/lib/x86/cpuid.h
>  delete mode 100644 xen/include/xen/lib/x86/msr.h
>