Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Bae, Chang Seok" <chang.seok.bae@intel.com>
To:     Thomas Gleixner <tglx@linutronix.de>
CC:     "bp@suse.de" <bp@suse.de>, "Lutomirski, Andy" <luto@kernel.org>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "Brown, Len" <len.brown@intel.com>,
        "lenb@kernel.org" <lenb@kernel.org>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "Macieira, Thiago" <thiago.macieira@intel.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v10 10/28] x86/fpu/xstate: Update the XSTATE save function
 to support dynamic states
Thread-Topic: [PATCH v10 10/28] x86/fpu/xstate: Update the XSTATE save
 function to support dynamic states
Thread-Index: AQHXmcps/AuzuQhndE+G6RIIKQI9gau+ghQAgAH0JgCAABcSAA==
Date:   Sat, 2 Oct 2021 22:54:25 +0000
Message-ID: <0F4DCBED-7A0B-4C0C-A63A-3C7E9AC065D5@intel.com>
References: <87pmsnglkr.ffs@tglx>
In-Reply-To: <87pmsnglkr.ffs@tglx>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?wtF9JPN2cdorgmIsGURGEe3KhhQdUHR9vnh040TN49diegLa1wiFssmI97s1?=
 =?us-ascii?Q?pPQ5v0I6N8sebjdDMyRN3yX6BNSu7nSIkel+GEmsb8evJ2RKlOWB2z4tQV75?=
 =?us-ascii?Q?yGmvc3Q8U67vdi3pP2blCT+t18ciZr6KdaB78eK0wdxT8KGGfL9lctXs87QK?=
 =?us-ascii?Q?0pVosvLd1moQYQ5WKGkiv5rGme8zBlPgR/GLE3eEtypGVKokefwEog/zNAqQ?=
 =?us-ascii?Q?+lmWVaaHZ9MpQ+wZeo0VHyDwC/HWs2AkSItZAjpB00vnfEyiXzRxP480yjL2?=
 =?us-ascii?Q?pGCl9L3UxY3H3WYjBCL/Wjo3P6kHYkdfX1QMgXEflwum8U+/+NjI12hLsJHI?=
 =?us-ascii?Q?FSlUShep3jOxMXHzpOxwiOJODU1YdFMj72KyYljlBEassf3J0X06yVuE1qi3?=
 =?us-ascii?Q?WAQgjYxMqso/h9zmSvwOzTamB0AXgEullUzLwg+nLwjfmXiKXy0NiFlhKrD/?=
 =?us-ascii?Q?D2FcvavNZFZunJnUAg2FUCVU7izvJ2EYACC8Q5lIcMg479dayvGIt0ndSoBC?=
 =?us-ascii?Q?27mlaoa39yd6U2V4wDIjQAuXY2xeoiRnV3yqL5U/3IJcUOtXxr4tZ82PrcQ2?=
 =?us-ascii?Q?IyxAQFyVjCEHXFThBD1FIu4ab3nyG6uK5z7ZPFlxditVFPyXMB5ObEKjN2lv?=
 =?us-ascii?Q?21gEIZDomDrbIoWE5t15uTmqIIwpmFjVq/99tlPtyGYEEU9rpmeRIEb/Jr8s?=
 =?us-ascii?Q?l33oo3DNaQOWFaM90hrV+j7PvfHRxiNt+eCjLhcjQDpL3ev4kO9wfsp5Q0r/?=
 =?us-ascii?Q?4LcYyUmzetxv0QE0PrMYiCb/9S/jvgMrgCCqbdWw0awxD4bIETFiKl9Mf19z?=
 =?us-ascii?Q?WHZdBwqz8RexMi9A730bBcEBlCeBSSIS2Rvknjx8xWbBUl84+ASrg6/qyOMZ?=
 =?us-ascii?Q?AVOT78zfMZ1DfWT3teYAchJC0/pDXbKf4ho4fF0OnfkHCusn59jGI8D3k1NX?=
 =?us-ascii?Q?oVbZ66f5x39rqToRa5ysDAaUiw0bmiAt3TxrduVXw5YtieQHTUuRbbbjZnxr?=
 =?us-ascii?Q?RMYynLXNiaXj/trKLlXAwGLvB7xOa9xCjOOGC5KtLzjhEHDUjQgT8+NB/bB8?=
 =?us-ascii?Q?YUmlw1yDDlysM25V7nPwp+kFl7LlZ11K3E6zr4v5y/T5hZ0jt5NYoTrAQOgj?=
 =?us-ascii?Q?kLRlNNf/7ZQabn9MkBlD8EwKkoSN1tlQGGiBOTqCsKDxQEHG5uwi9XzD686H?=
 =?us-ascii?Q?0TxFH0b7CNu0Bz2gvVoQBhw1e+wpoWlKETMcfaPrLg4FPQkwgwr3tWFF8S2U?=
 =?us-ascii?Q?mf4mgn8Ji/EODA1h+slt2mQLYiVpLNp5DpPtnD2Ist35ejqH36wPQvXCGhWx?=
 =?us-ascii?Q?jklR96vmhYmCBq1LAK1o/LJz?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2D220912796B244FB88E134BCE5F74EF@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4855.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ac2a30d1-9778-421e-61da-08d985f7953f
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Oct 2021 22:54:25.5189
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: xekiAV7jSZRhp3Tjbz8XeAtSxY/L/sC5PTBR6rklhSCAVyvtJPMZqUQXbYlDSqVfFmJgNYHcXZ/4NTMCVDmVK9OUGzs0+QB27Um1QB5jWQI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5062
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Oct 2, 2021, at 14:31, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, Oct 01 2021 at 17:41, Thomas Gleixner wrote:
>> On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote:
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 74dde635df40..7c46747f6865 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -9899,11 +9899,16 @@ static void kvm_save_current_fpu(struct fpu *fp=
u)
>>> 	 * KVM does not support dynamic user states yet. Assume the buffer
>>> 	 * always has the minimum size.
>=20
> I have to come back to this because that assumption is just broken.
>=20
> create_vcpu()
>   vcpu->user_fpu =3D alloc_default_fpu_size();
>   vcpu->guest_fpu =3D alloc_default_fpu_size();
>=20
> vcpu_task()
>   get_amx_permission()
>   use_amx()
>     #NM
>     alloc_larger_state()
>   ...
>   kvm_arch_vcpu_ioctl_run()
>     kvm_arch_vcpu_ioctl_run()
>       kvm_load_guest_fpu()
>         kvm_save_current_fpu(vcpu->arch.user_fpu);
>           save_fpregs_to_fpstate(fpu);         <- Out of bounds write
>=20
> Adding a comment that KVM does not yet support dynamic user states does
> not cut it, really.
>=20
> Even if the above is unlikely, it is possible and has to be handled
> correctly at the point where AMX support is enabled in the kernel
> independent of guest support.

I see.=20

> You have two options:
>=20
>  1) Always allocate the large buffer size which is required to
>     accomodate all possible features.
>=20
>     Trivial, but waste of memory.
>=20
>  2) Make the allocation dynamic which seems to be trivial to do in
>     kvm_load_guest_fpu() at least for vcpu->user_fpu.
>=20
>     The vcpu->guest_fpu handling can probably be postponed to the
>     point where AMX is actually exposed to guests, but it's probably
>     not the worst idea to think about the implications now.

FWIW, the proposed KVM patch for AMX looks to take (1) here [1] and
Paolo said [2]:

    Most guests will not need the whole xstate feature set.  So perhaps you=
=20
    could set XFD to the host value | the guest value, trap #NM if the=20
    host XFD is zero, and possibly reflect the exception to the guest's XFD=
=20
    and XFD_ERR.

    In addition, loading the guest XFD MSRs should use the MSR autoload=20
    feature (add_atomic_switch_msr).

And then I guess discussion goes on..

Thanks,
Chang

[1] https://lore.kernel.org/kvm/20210207154256.52850-4-jing2.liu@linux.inte=
l.com/
[2] https://lore.kernel.org/kvm/ae5b0195-b04f-8eef-9e0d-2a46c761d2d5@redhat=
.com/