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

Re: [PATCH] x86/cpuid: prevent decreasing of hypervisor max leaf on migration


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 2 Nov 2021 13:32:45 +0100
  • 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=8oc/4IFw5LjvE/9ui1454RPN/ASpTqNF5Dbd+iFKODY=; b=aSDjMv5WHqWYyebqEJUerR+jPinrJlN9Syelf1LaVkxMlgVFSGgALJd9goi2nCgpVQkHZQddZ/ZNGHIcE7jIQRx6MOH3Ts9sshcgVyXLwqtj1e3Z4rvlHp9NlE5THNiW09lKN3IZnQXRpKqM2Y1601uBbMbWtHExSPZnp9R2QtmxBK6oyY4Gq43qS5Bu1yAg2aVTgJ3hNtS/1R5gcW0wRmgLqsMCO4WWSDmc9E6glbQS7YEfHMK2rGMHBwnVszIDj/wj0MMjXjYsS0w4Fm/yVsBXvxO9RNh/AfT83q9BlE/S5P/lTiRU+A976oGbiaM/0m1o2WMncZxKPGXjWUW0PA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nXw6j9YaIuH8Prf5NbZiNX5nVbiyn7Xq7NEZBdr/rlelTYXxDQImwjljaFSsYWDADJcMvn/vuseMRbHLjo+IgDxAbWHck3cuha/uqCodVDfTvuCClucNEtF2tApLpvhgLxzogWqL59+OS0fOOrDyCcHsSO30GxYpIqiNFGIUKvQraF9s9JLXiV0F6HuL1kY69k1oJDZijuaJa4PKDXq7UH5sXPdQf0i2USb/MzipNcxRTGQnKMz+vpSpqhtTlgbBWlwvFjFopTrOJ7VyzgiZ78zJ3cFA0gJLv/CJURwGcMSB/mrm7JcPmaVV7XRy7dlYzxOsR06sjvRTtUnF+rXdgw==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 02 Nov 2021 12:33:11 +0000
  • Ironport-data: A9a23:h1Wv7a8I4+dKobsPZ6MgDrUDa3mTJUtcMsCJ2f8bNWPcYEJGY0x3n WcdW23TOfjcYmCnLtF/aYm/8htVvZ6GnNJiGwFlrCg8E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si9AttENlFEkvU2ybuOU5NXsZ2YhGmeIdA970Ug6wrdh2NYx6TSEK1jlV e3a8pW31GCNg1aYAkpMg05UgEoy1BhakGpwUm0WPZinjneH/5UmJMt3yZWKB2n5WuFp8tuSH I4v+l0bElTxpH/BAvv9+lryn9ZjrrT6ZWBigVIOM0Sub4QrSoXfHc/XOdJFAXq7hQllkPgpz Y9oqIWuEjwXEfL1htUZUisEPBpHaPguFL/veRBTsOSWxkzCNXDt3+9vHAc9OohwFuRfWD8Us 6ZCcXZUM07F17neLLGTE4GAguw5K8bmJsUHs2xIxjDFF/c2B5vERs0m4PcFgmdh25ESRJ4yY eI4TGtmYUnwbidWK1RJUI8Mzf2l3EfgJmgwRFW9+vNsvjm7IBZK+LnyMvLFd9qSX8JXk02E4 GXc8AzREhwccdCS1zeB2natnfPU2zP2XpoIE7+1/eIsh0ecrkQRAhALUVqwodGil1WzHdlYL iQ85S4GvaU0skuxQbHAswaQ+SDe+ERGApwJTrN8uFrlJrfoDxixXm1eayNYdocdiuBpfCIb0 l67w/zSPGk62FGKck61+rCRpDK0HCEaK24eeCMJJTc4D8nfTJIb1UyWEIs6eEKhppisQGyrn WjWxMQrr+xL1ZZj6kmtwbzQb9tATLDtRxV92AjYV3nNAuhRNN/8PNzABbQ2AJ99wGelorup4 Chsdyu2trlm4XSxeMqlGrxl8FaBvK7tDdEkqQQzd6TNDhz0k5JZQahe4StlOGBiOdsedDnib Sf74F0KucACZiP1N/UuP+pd7vjGK4C6TLwJsdiPNrJzjmVZLlfbrEmCm2bJhwgBb3TAYYlgY MzGIK5A/F4RCLh9zSreegvu+eRD+8zK/kuKHcqT503+idK2PSfJIZ9YYArmRr1ot8us/VSKm +uzwuPXkn2zpsWlOXKJmWPSRHhXRUUG6Wfe8pcKK7XcflI+cIzjYteIqY4cl0Vet/09vs/D/ 22nW18ez1z6hHbdLh6NZGwlY7TqNauTZ1pnVcD1FVr3iXUlf6i166ITK8k+cbU9rbQxxv9oV fgVPc6HB60XGDjA/j0ca7j7rZBjK0v31V7fYXL9bWhtZYNkSizI5sTgIlnl+h4RA3flrsA5u bChiF/WGMJRWwR4Ac/KQ/uz1Fft72MFked/UhKQcNlecUnh6qZwLCn1gqNlKs0AM0yblDCby xyXEVETouyU+90599zAhKalqYa1ErQhQhoGTjeDtbvvbHvU5Guux4NEQd2kRzGFWTOm4rima MVU0+r4bK8NkmFVvtcuCL1s168/uYfi/ucI0gR+EXzXRF23Ebc8cGKe1MxCu6ARlL9UvQy6B hCG9tVAYOjbPcrkFBgaJRY/b/TF3vYRw2GA4fMwKUT8xSl24LvYDhkCY0jS0HRQfOlvLYco4 eY9o8pHuQWwhy0jPsuCki0JpX+HKWYNUvl/u5wXaGMxZtHHFr2WjUTgNxLL
  • Ironport-hdrordr: A9a23:78rRY6D9bFDKsRvlHehIsceALOsnbusQ8zAXPh9KJiC9I/b1qy nxppkmPH/P6Qr4WBkb6Le90Y27MAnhHP9OkPIs1NKZMjUO11HYTr2KgbGSpgEIXheOi9K1tp 0QDZSWaueAdGSS5PySiGLTc6dCsai6GeKT9J/jJh9WPH5XgspbnmFE42igYylLrF4sP+tEKH PQ3LsNmxOQPVAsKuirDHgMWObO4/XNiZLdeBYDQzoq8hOHgz+E4KPzV0Hw5GZSbxp/hZMZtU TVmQ3w4auu99m91x/nzmfWq7BbgsHoxNdvDNGFzuIVNjLvoAC1Y5kJYczJgBkF5MWUrHo6mt jFpBkte+x19nPqZ2mw5SDg3gHxuQxenUPK+Bu9uz/OsMb5TDU1B45qnoRCaCbU7EImoZVVzL 9L93jxjesYMTrw2ADGo/TYXRBjkUS55VA4l/QIsnBZWYwCLJdMsI0k+l9PGptoJlO01GkeKp gvMCjg3ocUTbvDBEqp/FWHgebcEkjbJy32A3Tr4aeuon1rdHMQ9Tpu+CVQpAZFyHsHceg22w 3zCNUdqFh/dL5nUUtDPpZyfSKWMB2FffueChPbHbzYfJt3Tk4l7aSHp4kI2A==
  • Ironport-sdr: zT8X36NM/GYZrNEHseUBvgTynUUM87krxyzE5WpMJAbK88/Ys/4VrQb/GQopzQmmXpQpu/AuxF HJgtI0b/blr+KkFzR+T31scUIFQ8uYuoJqbcAEM8OXUb+lEokrYGrILW+y59z+CO7EORpOeFFO UQcyQ4hGu0GhE2RUyAEXdVPQdiM+ueCZMLabxK44V8ASjgYgxpsQdlLyS06+3DO/1Kvixoj9m8 SsmM/73cH5qhgNYxJuSlHT8ZT0S7cHm1jE1PfrH1qYsXG0BbXFUqazpM254TjIfN+MF34lWMoL KvBqjpKMBPH/5vkKUrt0jDs9
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Nov 02, 2021 at 01:00:07PM +0100, Jan Beulich wrote:
> On 27.10.2021 16:00, Roger Pau Monne wrote:
> > In order to be compatible with previous Xen versions, and not change
> > max hypervisor leaf as a result of a migration, keep the clamping of
> > the maximum leaf value provided to XEN_CPUID_MAX_NUM_LEAVES, instead
> > of doing it based on the domain type. Also set the default maximum
> > leaf without taking the domain type into account. The maximum
> > hypervisor leaf is not migrated, so we need the default to not regress
> > beyond what might already be reported to a guest by existing Xen
> > versions.
> 
> While this is the missing description to the patch I had submitted
> back in May upon Andrew's request, I have to admit that I don't
> consider this a satisfactory explanation. Shouldn't hypervisor
> leaves undergo similar leveling as other leaves? I.e. upon
> migration leaves or individual bits should neither disappear nor
> appear?

Indeed, but hypervisor max leaves is not properly migrated, as
hv{,2}_limit is not set unless explicitly passed by the toolstack.

> I continue to consider it at least suspicious that HVM guests get
> 5 leaves reported when only 4 are really meaningful to them.

That's indeed fine to fix, but we would need to start saving/restoring
hypervisor max leaf unconditionally as part of the cpuid policy (ie:
set hv{,2}_limit at domain create to the default limit), and populate
it with 5 for backwards compatibility on restore if not set.

> I
> see this has gone in, so I'm likely to trip up on this again in
> the future. Might result in the same patch again then if I don't
> end up doing archeology at that point ...

Maybe there's some comment we could add in the code to make this
clearer? I didn't realize either when reviewing as didn't pay close
attention to how hv{,2}_limit is used.

I hope the above comments help clarify why the current behavior was
an issue.

Regards, Roger.



 


Rackspace

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