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

Re: [PATCH 3/3] x86/PV: drop "vcpu" local variable from show_guest_stack()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 18 Oct 2021 16:35:36 +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=f4ipIxAZmgfh4+f6trev402Zd5Xdi5moSq+db0QEPK0=; b=EZyQLiReIS2kS2ogavoaL5Icz7bpfQ+oq1YALQUF7O/aPNq/WapUJHqsQeMOraailILphxadwmqfEoWB6BDxxH+hbdGdDUcsJ6xN75LjUD0Z5iMsVWUag8qTdPjEHrAJVRs5lbSBn/N2lvcXQ05J9GHcmDI6Y+xT5OpCwo04kM5r91eypUmAmveWHCTjFpeE7s21YLUZwh5uUY5uml0BFWrbHcBMEvYhWcam2aBPd02JMSTVo2Fio3iXejT64dcEzRfeIBQKoCRgNgrWzUsOMfqmor8Vg+TBWOQyDrtRggnNLsGqxmPYxOtKEsD0fBpxCS7LQ9LTt+YCHH20P0k0ww==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nfgljEinTwZIhuFI4l/UJnzK1ghwwn1zxwoV2ODVNgsZqIT+mMd3k6Xz2N1uDIGs1BV0Vx2NfYC4NfzSg125/aklxPJWkRgruFpL5WlXppGz6PxCMvGO15MbtnRHb7n2EsbXzHo+9Wp3FC/t8SupMu0uCcY30WX4b8SkDq68ygnpPqCDIYVp+/KVsneIVYP9THBbIOeLyOwMBCgDJolEBkW4xhnttODaY3lBzUXlNQBIvOtEkD4Ax0QQD4PoJWWT0kmqkRuFSSKWljA45WHD47BLN4D5nPKR+hDCfCNoIORvFgu0sYRHsk+KdfqHRNepRMywuysqoIch7NjFUX5OSQ==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 18 Oct 2021 14:36:06 +0000
  • Ironport-data: A9a23:ygDCIq7+RqnLjtXMRf8KIwxRtOPAchMFZxGqfqrLsTDasY5as4F+v mIbUGHXP/+NZDPxL40kboSx9k1U65HSx9RnT1FprX9mHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuV3zyIQUBUjclkfJKlYAL/En03FVIMpBsJ00o5wrdh294w27BVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Z6 4p0rZzhFz8SG5LiisUsfhIEESAlBPgTkFPHCSDXXc27ykTHdz3nwul0DVFwNoodkgp1KTgQr 7pCcmlLN03dwbLtqF64YrAEasALNs7kMZlZonh95TrYEewnUdbIRKCiCdpwgWpg2pkeQ6m2i 8wxUz03Myacei91GnwSA78exNWul0T+bGgNwL6SjfVuuDWCpOBr65DyNPLFd9rMQt9a9m6Iq 2SD82nnDxUyMN2E1SHD4n+qnvXIny7wRMQVDrLQ3vxgjUCXx2cTIAYLTlb9qv684nNSQPoGd RZSoHB36/Fvqgr7FbERQiFUvlan4ScEW4sOK9Yf1y+f65v56FqbBzM9G2sphMMdiOc6Qjkj1 1msltzvBCByvLD9dU9x5ot4vhvpZnBLdT5qiTssCFJfuYG68d5bYgfnF447SMaIYsvJ9SYcK txghBM1gKkPloY13qG/8EGvb9mE98WREFBdCuk6WAuYAuJFiGyNOtTABbvzt68owGOlor+p5 ilsdy+2t7hmMH11vHbRKNjh5Znwjxp/DBXSgER0A74q/Cm39niocOh4uW8leBs4b5deJWOzM Sc/XD+9ArcJbRNGiocsO+qM5zkCl/C8RbwJqNiFBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzKq6R4YIhIf0/llKeHr5FuZdyn3xW7T6DFPjTkkX8uZLDNSH9dFvwGAbXBgzPxPjf+1u9H hc2H5bi9iizp8WlPnCIrNJOdA1TRZX5bLivw/Fqmie4ClMOMEkqCuPLwKNnfIpgnq9PkfzP8 G37UUhdoGcTT1WZQelTQnw8Or7pQ7hlqnc3YX4lMVqygiBxaoez9qYPMZAweOB/puBkyPd1S dgDetmBXasTGmiWpWxFYMmvtpFmeTSqmRmKY3ivbg8gcsMyXAfO4NLlIFfirXFcEiqtuMIii LS8zQeHE4EbTgFvAZ+OOvKixl+8p1YHn+d2UxeaK9VfYhy0ooNrNzbwnrk8JMRVcUfPwT6T1 gC3BxYEpLaS/99poYeR3a3d9tWnCepzGEZeDlL317fuOHmI5HenzK9BTP2MIWLXWlTr9fjwf u5S1fz9bqEKxQ4Yr4pmHr935qsi/N+z9aRCxwFpEXiXPVSmDrRsfiuP0cVV7/Afw7ZYvU29W 16V+8kcMrKMYZu3HFkULQsjT+KCyfBLxWWCsaVreB33tH1t4b6KcUROJB3d2iVSIYx8PJ4h3 ep86tUd7Bayi0ZyP9uL5syOG79g8pDUv30bi6wn
  • Ironport-hdrordr: A9a23:+bn4IKFdUTE012s1pLqFeZHXdLJyesId70hD6qkvc3Nom52j+/ xGws536faVslcssHFJo6HmBEClewKnyXcT2/htAV7CZnichILMFu9fBOTZsl/d8kHFh4tgPO JbAtRD4b7LfClHZKTBkXCF+r8bqbHtmsDY5pav854ud3ATV0gJ1XYGNu/xKDwReOApP+tcKH LKjfA32AZINE5nI/iTNz0gZazuttfLnJXpbVovAAMm0hCHiXeN5KThGxaV8x8CW3cXqI1Svl Ttokjc3OGOovu7whjT2yv66IlXosLozp9mCNaXgsYYBz3wgkKDZZhnWZeFoDcpydvfpGoCoZ 3pmVMNLs5z43TeciWcpgbs4RDp1HIU53rr2Taj8DPeiP28YAh/J9tKhIpffBecwVEnpstA3K VC2H/cn4ZLDDvb9R6Np+TgZlVPrA6ZsHAimekcgzh0So0FcoJcqoQZ4Qd8DIoAJiTn84oqed MeTf003MwmM29yUkqp+1WGmLeXLzAO91a9MwY/U/WuontrdCsT9Tpe+CQd9k1wva7VBaM0od gtn8xT5cVzp/QtHNBA7dE6ML2K41z2MGHx2V2pUCHa/YE8SjnwQs3Mkf8IDN/DQu1+8HJ1ou WZbG9l
  • Ironport-sdr: A+r86Y1FkF+CwN89RHLvtMRr9Ym8kMCK1TJr+xFqclXuqi2MkObSauBqNHdlR7EppD4FXITwH2 cJS2dN7MuCo6gfoAW28TlDzhLllQK2L6jEP5vtz6t1gJL4eO7VG77EaQT7q5arzTsyjgz5wW90 Eg8se1NgGkQ2j5c3ZAd/eymxoVP+bSkSMqTwR+jQJw8GBjFvnEYUcv3EcaU/QmgNsfQGuYI5GA TAK+6ft87nr3uq9tgcJuT1Y92DUr5qALUgcfcUZSmNAP5GXZHJ4WeLOHpVVdO22SbiX2lo+N+D QlqT58V2dF3+4Vdsl77vOkaz
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Sep 29, 2021 at 11:43:32AM +0200, Jan Beulich wrote:
> It's not really needed and has been misleading me more than once to try
> and spot its "actual" use(s). It should really have been dropped when
> the 32-bit specific logic was purged from here.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

As it makes the current code clearer. I have one question/concern
below.

> 
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -327,16 +327,13 @@ static void show_guest_stack(struct vcpu
>  
>      if ( v != current )
>      {
> -        struct vcpu *vcpu;
> -
>          if ( !guest_kernel_mode(v, regs) )
>          {
>              printk("User mode stack\n");
>              return;
>          }
>  
> -        vcpu = maddr_get_owner(read_cr3()) == v->domain ? v : NULL;
> -        if ( !vcpu )
> +        if ( maddr_get_owner(read_cr3()) != v->domain )

Wouldn't it be more accurate to check that the current loaded cr3
matches the one used by v?

AFAICT we don't load the cr3 from v, so it's still possible to have
diverging per-vcpu page tables and thus end up dumping wrong data?

Thanks, Roger.



 


Rackspace

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