Re: [PATCH] x86/hvm: print valid CR4 bits in case of error

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/06/2023 2:46 pm, Roger Pau Monne wrote:
> Some of the current users of hvm_cr4_guest_valid_bits() to check
> whether a CR4 value is correct don't print the valid mask, and thus
> the resulting error messages are not as helpful as they could be.
>
> Amend callers to always print the value of hvm_cr4_guest_valid_bits(),
> and take the opportunity of also adjusting all the users to use the
> same print formatter.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/domain.c       | 4 ++--
>  xen/arch/x86/hvm/hvm.c          | 8 ++++----
>  xen/arch/x86/hvm/svm/svmdebug.c | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index deec74fdb4f5..8951230a9f52 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -266,8 +266,8 @@ int arch_set_info_hvm_guest(struct vcpu *v, const 
> vcpu_hvm_context_t *ctx)
>  
>      if ( v->arch.hvm.guest_cr[4] & ~hvm_cr4_guest_valid_bits(d) )
>      {
> -        gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx\n",
> -                v->arch.hvm.guest_cr[4]);
> +        gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx (valid: %016lx)\n",
> +                v->arch.hvm.guest_cr[4], hvm_cr4_guest_valid_bits(d));

I suspect you want to call this once and store it in a variable.

It's a non-inline function which also isn't marked attr_const, so it
will get called twice.

Also, if you're extending like this, then you actually want

(valid %lx, rejected %lx)

passing in cr4 ^ valid for rejected.  It's almost always 1 bit which is
rejected at a time, and the xor form is easier to read, not least
because it matches the X86_CR4_blah constant of the bad feature.

~Andrew