[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 8 Jun 2023 09:57:41 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Ve2oKjF/o8rEcHfMYjsFlKjExdHYjnictDl38XxXMEE=; b=nfQmAsMpVR1TV9x9QRq+/W1x5bjkIkvy47ydB5/QNL8gTGSeOKjTeO44s4+LF8aNSpbXuKGsg0HEUI8efyK9CXGSy/H06frs5lNFWM3HxLq57pn1ej7nN8z9bEvIcfWOhDANWmH8MWalcv+h1U1daS6W86FoA7JzamsOfUywcbkgSNZQUBbJq0rlptoP1ihG1Suip1/wjsyjbPskKpFGcodvjhuk62uAzD61NF5UqeQGMLKapwhIdYd2MsdGKwV+MlRAvFO3MztEKAoo9q5jNDwciMGIvbsVN73VjzfB0fOtiv8cYUsmmSwsnTH3B2KRKWaw8hXF0mNkh+SGgKfyBg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ix/EYztl/2t2Vxt2b0YOL2+ymai6PfZHyQaKQKMDlpjt9ZCMurvRSUTSnTVBdDXqbWz8oJDDpUzh02j6mPjZKfo1RenYQZbOSAa7JXID/yO+duUF+Zh0LvTZw5Og8C4HjVnpL9PjagjxAHQ0h6dGglr96zXT0/k3DRJZxwjJmQq4dvMIFEWrKKbajMAtu+QuI4T8gYCTb8xfp0Qkk7xqvpWNP8yWfmSBnNPNq53DB3fWslThdCi9DPyC/icQbwZr3NNNcF2IUeYda2a+shZpaVjeABttD7Asuq5hhpuaicIj9352rEsXJitawNT3qMH5Rr2d1Wp6/BVFY13wfNbO+w==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Jan Beulich <jbeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 08 Jun 2023 07:58:22 +0000
  • Ironport-data: A9a23:Xn2DRaDqs5S2NRVW/wjiw5YqxClBgxIJ4kV8jS/XYbTApDtxhGEAn GdLWG6GOPaJZGT9Kd1wOdmx/B8C6MLUyYVkQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nOHuGmYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbCRMs8pvlDs15K6p4G1A7gRnDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIw3LksXn1D8 sUjOBc0cAHb2tO1+J2xRbw57igjBJGD0II3nFhFlGucJ9B2BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTI+OxuvDC7IA9ZidABNPLPfdOHX4NNl1uwr WPa5WXpRBodMbRzzBLcqyjw2rSWw3iTtIQ6SJGEtf5tgFqqn2EwFAxJcXW3nte3hRvrMz5YA wlOksY0loAi+UruQtTjUhmQpH+fogVaS9dWC/c96gyG1uzT+QnxLmoOQyNFadcmnNQrXjFs3 ViM9/v2ARR/vbvTTmiSnop4thu3MCkRaGUENSkNSFJf58G5+d5ryBXSUtxkDai5yMXvHi39y CyLqy54gKgPickM1OOw+lWvby+Qm6UlhzUdvm3/Nl9JJCsjOeZJu6TABYDn0Mt9
  • Ironport-hdrordr: A9a23:SnGx8qA5hmvU987lHelo55DYdb4zR+YMi2TDt3oddfU1SL38qy nKpp4mPHDP5wr5NEtPpTniAtjjfZq/z/5ICOAqVN/PYOCPggCVxepZnOjfKlPbehEX9oRmpN 1dm6oVMqyMMbCt5/yKnDVRELwbsaa6GLjDv5a785/0JzsaE52J6W1Ce2GmO3wzfiZqL7wjGq GR48JWzgDQAkj+PqyAdx84t/GonayzqK7b
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Jun 07, 2023 at 04:48:42PM +0100, Andrew Cooper wrote:
> 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.

I wasn't specially concerned about this, it's an error path where the
second call will happen, and there's already a printk which will make
the cost of calling hvm_cr4_guest_valid_bits() negligible compared to
the printk.

> 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.

I will adjust as requested.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.