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