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

Re: [PATCH v3 6/9] x86/HVM: convert hvm_virtual_to_linear_addr() to be remote-capable


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 23 Sep 2021 16:28:58 +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; bh=3GGkmR/LRh2cfPblNWsv8n5RSfXbfLzVOUnqQuL1NSI=; b=Q/hf/3JryyKFPNahtJhoUtsdWGDQ8/RWp8SWzRkGLbvC8QWn90NXSp6zAaQuf0ZvMoPJ1WVdm1K+2nTnOFEY16ehCpjVqy1Q+bn6NrVCO82K0+pZf979PHlJC1xGo71PIW/PIlTZOmxEVmEfa6ZbPkxQbvg0nbFWp1JOdn6QTKAIPIsNCvfLScTDQSJbj9WXeXmwCieEc9YF1uX8EqbaMx7UYVc+ZdlotWt8IQDAbD00hGx//8dukS10lKvq9z7f3SIBXv10SmtMiudJs8uXR7xrIsNv9+pgxCerQq8lRI+gqRJiHDqC6EwKOlg15OesDlRqdWZKEhpxqYbZ7kB1dg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YI1K0F2T0sC6McBTlBzRg0LeiT993zlVtn0sC/nQgqFL0SBRAzqt+8o8cBZ8hb80I7N2VK+Wujr8uFnrokE643l75+nd5QLvtJK8Q2M4+qD/499XhlQqxZ4jHMT3G0dT1DYRBdd8zIlrGSfXVUUfqNCJKBh7OcB3YBA1oeJGQG7Ap9YG9VZ4efOQwW9vHDK2TFXg/mMziBTUn1oJddXqI+6opqtYJ792Pqk2Q/T9DP/jfVclNPxzNibqK/TKHtXWYenlyk0PROtuuDukuNPrQ729m3scoEjSVpFMuXQBBEJl86ffDaQiGtlKx2j1AShVDq3qVWcy+tTGDnMdyoKOKA==
  • Authentication-results: esa3.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: Thu, 23 Sep 2021 14:29:09 +0000
  • Ironport-data: A9a23:kmtP4KvHueiG7jJgAKzZXRBDGufnVIhZMUV32f8akzHdYApBsoF/q tZmKTuGa/uNZGanc4h+a4+y8hsEsZ/UzNRqHVM/rihjHihA+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZQP0VOZigHtIQMsadUsxKbVIiGHpJZS5LwbZj29Y524PhWmthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ NpltJbuZSF5LIn32/lEUzdzNWYuDKF/weqSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DJoQQt2sm1TjEJf0nXYrCU+PB4towMDIY25sSQ6iCN 5RxhTxHVDnMSA1qFFcrOa0bjN32lCPUKyFggQfAzUYwyzeKl1EguFT3C/LKfvSaSMMTmVyXz krW8mK8DhwEOdi3zTue7mnqluLJhTn8Wo8ZCPu/7PECqF+Zy3EXCRYWfUCmuvT/gUm7M++zM GRNpHBo9/JrshX2EJ+tBHVUvUJooDYdUYR8Nb0ozDrO1/PUzS2/QTg1Qgd4PYlOWNANeRQm0 VqAntXMDDNpsaGIRX/1yop4vQ9eKgBOcjRfP3FsoR8tpoC5+dBu0kunosNLTfbt5uAZDw0c1 NxjQMIWvLwVkcdD/KGy51mvb9mE98WRE1ZdCuk6WAuYAuJFiGyNO9fABbvzt68owGOlor+p5 iNsdy+2trxmMH11vHbRKNjh5Znwjxp/DBXSgER0A74q/Cm39niocOh4uW8lfhwyap5fJ2W4M Sc/XD+9ArcJZxNGioctP+qM5zkCl/C8RbwJqNiOBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzK66R4YIhIf0/llKeHr5FuZdyn3xW7T6DFPjTkkX8uZLDNSH9dFvwGAbXBgzPxPjf+1u9H hc2H5bi9iizp8WkOXSIrd5PcwpaRZX5bLivw/Fqmie4ClMOMEkqCuPLwKNnfIpgnq9PkfzP8 G37UUhdoGcTT1WdQelTQnw8Or7pQ7hlqnc3YX4lMVqygiBxaoez9qYPMZAweOB/puBkyPd1S dgDetmBXasTGmiWpWxFYMmvtpFmeTSqmRmKY3ivbg8gcsMyXAfO4NLlIFfirXFcEiqtuMIii LS8zQeHE4EbTgFvAZ+OOvKixl+8p1YHn+d2UxeaK9VfYhy0ooNrNzbwnrk8JMRVcUfPwT6T1 gC3BxYEpLaS/99poYeR3a3d9tWnCepzGEZeDlL317fuOHmI5HenzK9BTP2MIWLXWlTr9fjwf u5S1fz9bqEKxQ4Yr4pmHr935qsi/N+z9aRCxwFpEXiXPVSmDrRsfiuP0cVV7/Afw7ZYvU29W 16V+8kcMrKMYZu3HFkULQsjT+KCyfBLxWWCsaVreB33tH1t4b6KcUROJB3d2iVSIYx8PJ4h3 ep86tUd7Bayi0ZyP9uL5syOG79g8pDUv30bi6wn
  • Ironport-hdrordr: A9a23:zzjD8al8takgczKmEU6GrXKrRYHpDfLW3DAbv31ZSRFFG/Fw9/ rCoB3U73/JYVcqKRUdcLW7UpVoLkmyyXcY2+cs1NSZLWzbUQmTXeJfBOLZqlWNJ8SXzIVgPM xbAspD4bPLbGSTjazBkXSF+9RL+qj6zEh/792usEuETmtRGt9dBx8SMHf9LqXvLjM2fqbQEv Cnl6x6jgvlQ1s7ROKhCEIIWuDSzue77q4PMXY9dmcaABDlt0LR1ILH
  • Ironport-sdr: 7IfbeGX0JCuLGRorB6pmow2vVVhXJ/8YS/eRR3uTqL1xpK4VED2/Sf2fGlTMsJ0VBzZGKbjxMf IRcKDM+mjaAYG5QVIy5ps51mF8p6Km9bpZiuqhTxyQzOxLrMYZL46VjpsRXRJo/Ct+N5yiGgb9 AB1pkg+utO8S1zm6twZDS5UM4QpSI7ExTSNltYKPvoxduIvdRwmWZHZr5X/MuxfSeOdMy3NeKu hdMt12abBVd+I7TYUli6iyqa7EIfLn7k3n+uzmDxQ4RvrU0WZGePYtnpnO+pNREGEqXPELoW4O 9sdUTY3phovAz9RHYZO+6DZo
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Sep 23, 2021 at 12:34:48PM +0200, Jan Beulich wrote:
> On 23.09.2021 10:09, Roger Pau Monné wrote:
> > On Tue, Sep 21, 2021 at 09:19:37AM +0200, Jan Beulich wrote:
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -2526,7 +2526,8 @@ int hvm_set_cr4(unsigned long value, boo
> >>      return X86EMUL_OKAY;
> >>  }
> >>  
> >> -bool_t hvm_virtual_to_linear_addr(
> >> +bool hvm_vcpu_virtual_to_linear(
> >> +    struct vcpu *v,
> >>      enum x86_segment seg,
> >>      const struct segment_register *reg,
> >>      unsigned long offset,
> >> @@ -2535,8 +2536,9 @@ bool_t hvm_virtual_to_linear_addr(
> >>      const struct segment_register *active_cs,
> >>      unsigned long *linear_addr)
> >>  {
> >> -    const struct vcpu *curr = current;
> >>      unsigned long addr = offset, last_byte;
> >> +    const struct cpu_user_regs *regs = v == current ? 
> >> guest_cpu_user_regs()
> >> +                                                    : &v->arch.user_regs;
> >>      bool_t okay = 0;
> > 
> > Since you change the function return type to bool, you should also
> > change the type of the returned variable IMO. It's just a two line
> > change.
> 
> Can do; I would have viewed this as an unrelated change: While the
> first change needed indeed is on an adjacent line (above), the other
> one isn't.
> 
> > Also is it worth adding some check that the remote vCPU is paused? Or
> > else you might get inconsistent results by using data that's stale  by
> > the time Xen acts on it.
> 
> I did ask myself the same question. I did conclude that even if the
> remote vCPU is paused at the time of the check, it may not be anymore
> immediately after, so the information returned might be stale anyway.
> I further looked at {hvm,vmx}_get_segment_register() to see what they
> did - nothing. It is only now that I've also checked
> svm_get_segment_register(), which - interestingly - has such a check.
> I will copy the ASSERT() used there.

Thanks, with that:

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

Roger.



 


Rackspace

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