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

Re: x86/PV: (lack of) MTRR exposure


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 29 Apr 2022 12:53:00 +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=aDds1ie+iWLIsDLC6wx3t+BDr0rs9GWxIhLkxGO0BiE=; b=hhChodlEkQr3bRt3pER7HZic4JGCt1F0tbOEY3ZE+ZHuNjQOb3T2/EZoheXcpuXXPoZ2VOVASDPnzvXfm/U/JIKsuZlyT/fl6ZBLmQJx8KcQKVzmFVXI3pWTsytL1IGHtMVgQGM/TQ69iODfqa7qR9VJC2MAWc+k3C3VGdGHblgIdcKc42e5tqAqsxqXZzVj+8KqF9V5HAqIyM8DFLg1N3R2VIuC2j2lpkyTxwoR5GK/AwYKPcfaUahf1qwi99ImCtpkuQ+D/vCU073RyaCN8YWNNWOW5h5R78jNn7Daa9Vp4Ax+2AJDmgKnIq/o0DA1THq/rNzxzoAR99LVtgCbZg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fqgyWQpg0jp4pmSi5wPaznPxjBa8ORa0TjOdM8yocDozNZLt886c5afBYT9gLtdyUWi7miwbwwiFOSnkB94PLHnPLIWpVncER7GZOKl4nOBxZBhaCyPSjTmwFFSpMkKAldpd1RCP/hzhb17iF7C2nkgTMpufQ8hFWXKXHUqszNTK8zw7qm7cRlt/bAsYxX5Lm4YRC77VmY2SXD/bEsmDnMvg5MyExbwpe0tf0qTPubiy1e6UVdQ1ooYimzgd8XNZaxLl85WoD0b56BomRYQbViVDxNp4OKCPVmpXIPxDkr9AtHEhnySLG19tHPaZjdduPk2MoiQ5edans4iZwI8x0Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • Delivery-date: Fri, 29 Apr 2022 10:53:15 +0000
  • Ironport-data: A9a23:INWSv68zl8IdTOUtTNclDrUDl3+TJUtcMsCJ2f8bNWPcYEJGY0x3m DYdDGzQM62PNzTzeI8ga9zl9E4DuZ/Xy9ZjGgM5qy48E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si+Fa+Sn9T8mvU2xbuKU5NTsY0idfic5DnZ44f5fs7Rh2NQw3YLoW1nlV e7a+KUzBnf0g1aYDUpMg06zgEsHUCPa4W5wUvQWPJinjXeG/5UnJMt3yZKZdhMUdrJ8DO+iL 9sv+Znilo/vE7XBPfv++lrzWhVirrc/pmFigFIOM0SpqkAqSiDfTs/XnRfTAKtao2zhojx/9 DlCnaa9VAp5P7XNofwieit4FCshHohl04aSdBBTseTLp6HHW13F5qw3SWoRZMgf8OsxBnxS/ /sFLjxLdgqEm++93LO8TK9rm9gnK87oeogYvxmMzxmAVapgHc+FHvuMvIAGtNszrpkm8fL2f c0WZCApdB3dSxZOJk0WGNQ1m+LAanzXLGcG+A/M+fNfD2776F0v26e3bsfpc4bSAv18r2Kdl 2Dl4DGsav0dHJnFodafyVqujOLSmSLwWKoJCaa1sPVthTW71mEVTREbS1a/if24kVKlHcJSL VQO/SgjprR081akJvH/UAe/u2WspQMHVpxbFOhSwAuK0KvPpQGCGnIDUCVCefQhrsY9QTFs3 ViM9+4FHhRqubyRDHmar7GdqGrrPTBPdDBcIygZUQEC/t/v5pkpiQ7CRcpiF6jzicDpHTb3w HaBqy1Wa6gvsPPnHp6TpTjv6w9AbLCQJuLpzm07hl6Y0z4=
  • Ironport-hdrordr: A9a23:aCKcEKAqONs8r8TlHeglsceALOsnbusQ8zAXPh9KJCC9I/bzqy nxpp8mPH/P5wr5lktQ/OxoHJPwOU80kqQFmrX5XI3SJTUO3VHFEGgM1+vfKlHbak7DH6tmpN 1dmstFeaLN5DpB/KHHCWCDer5PoeVvsprY49s2p00dMT2CAJsQizuRZDzrcHGfE2J9dOcE/d enl4J6jgvlXU5SQtWwB3EDUeSGj9rXlKj+aRpDIxI88gGBgR6h9ba/SnGjr10jegIK5Y1n3X nOkgT/6Knmm/anyiXE32uWy5hNgtPuxvZKGcTJoMkILTfHjBquee1aKva/lQFwhNvqxEchkd HKrRtlF8Nv60nJdmXwmhfp0xmI6kdY15dPoWXo8kfLkIjcfnYXGsBBjYVWfl/y8Ew7puxx16 pNwiawq4dXJQmoplWy2/H4EzVR0makq3srluAey1ZFV5EFVbNXpYsDuGtIDZY7Gj7g4oxPKp ggMCjl3ocXTbqmVQGbgoE2q+bcHEjbXy32DnTqg/blkgS/xxtCvg4lLM92pAZ1yHtycegB2w 3+CNUYqFh/dL5pUUtDPpZwfSKWMB26ffueChPaHbzYfJt3SU7lmtrQ3Igfwt2MVdgh8KYS8a 6xJW+w81RCNn7TNQ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Apr 29, 2022 at 12:00:21PM +0200, Jan Beulich wrote:
> On 29.04.2022 10:00, Roger Pau Monné wrote:
> > On Thu, Apr 28, 2022 at 05:53:17PM +0200, Jan Beulich wrote:
> >> Hello,
> >>
> >> in the course of analyzing the i915 driver causing boot to fail in
> >> Linux 5.18 I found that Linux, for all the years, has been running
> >> in PV mode as if PAT was (mostly) disabled. This is a result of
> >> them tying PAT initialization to MTRR initialization, while we
> >> offer PAT but not MTRR in CPUID output. This was different before
> >> our moving to CPU featuresets, and as such one could view this
> >> behavior as a regression from that change.
> >>
> >> The first question here is whether not exposing MTRR as a feature
> >> was really intended, in particular also for PV Dom0. The XenoLinux
> >> kernel and its forward ports did make use of XENPF_*_memtype to
> >> deal with MTRRs. That's functionality which (maybe for a good
> >> reason) never made it into the pvops kernel. Note that PVH Dom0
> >> does have access to the original settings, as the host values are
> >> used as initial state there.
> >>
> >> The next question would be how we could go about improving the
> >> situation. For the particular issue in 5.18 I've found a relatively
> >> simple solution [1] (which also looks to help graphics performance
> >> on other systems, according to my initial observations with using
> >> the change), albeit its simplicity likely means it either is wrong
> >> in some way, or might not be liked for looking hacky and/or abusive.
> > 
> > I wonder whether the patch needs to be limited to the CPUID Hypervisor
> > bit being present.  If the PAT MSR is readable and returns a value !=
> > 0 then PAT should be available?
> 
> I simply didn't want to be too "aggressive". There may be reasons why
> they want things to be the way they are on native. All I really care
> about is that things are broken on PV Xen; as such I wouldn't even
> mind tightening the check to xen_pv_domain(). But first I need
> feedback from the maintainers there anyway.

Right, I did also consider suggesting to limit this to PV at first,
but I was unsure why native wouldn't also want such behavior.  Maybe
there's no hardware that has PAT but not MTRR, and hence this was
never considered.

> > I guess we should instead check that the current PAT value matches
> > what Linux expects, before declaring PAT enabled?
> 
> I don't think such a check is needed, the code ...
> 
> > Note there's already a comment in init_cache_modes that refers to Xen
> > in the check for PAT CPUID bit.
> 
> ... in __init_cache_modes() already does it the other way around:
> Adapt behavior to what is found in PAT.
> 
> >  We might want to expand that comment
> > (or add one to the new check you are adding).
> 
> I don't see what further information you would want to put there.

It would depend on how the check ends up looking I think.  If the
enabling is limited to also having the Hypervisor CPUID bit set then
the comment should likely mention why setting it on native is not
safe.

Anyway, let's see what maintainers think.

The other option would be to set some kind of hooks for Xen PV to be
able to report PAT as enabled (maybe a Xen PV implementation of
mtrr_ops?), but that seems like over-engineering it.  My main concern
with setting pat_bp_enabled to true is whether in the future such
setting could be used to gate PAT MSR writes.  It's already like this
for APs (in pat_init() -> pat_ap_init()), but we avoid that path
because we don't report MTRR support AFAICT.

Thanks, Roger.



 


Rackspace

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