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

Re: [PATCH v4 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD


  • To: xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 29 Apr 2022 18:11:25 +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=2t1EP7t4LaXUWLbcjhMeSu/sj0eW1jRKE9liAsTWpIs=; b=DYMina32b7Gc7s3Tps36QBgaDfkjtsDT7AMZbGf5JSZeNyB8RZVQc8OiuIs64sd4TvkDRcqk2NI5prNp4zU6G4imDTv3kUdpM9dKjxxf+GiqPmh+MSFrKpKHn3RjkdxrjMQTp3Zcnt1C9zv6DOr+NZbqMOR6vrLFu3RwJs4k6pASzm+bWb6IdgqzB/2gN6hAp8rJ5iWu3OgFEHWAiYfLPqYHAjrbLNN5JpgZytGvcGMZxmaqItLpNEuREhbNcXhPmttrcpNcrfosO8vvUw36JHD+xI9Awitdgl4xYJmq/mXvdrxHdzE6tqbFkUopq5o6O5TjWyEkUW/CrDXkE+GL7w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XcECdsA++g/oSAh3At5bVxQPm0R03chN5+XLr9I/g0m1CHZL1wnSHhmV/dXMIulallWWmXfuZSH20Phw7B9PNfDF/2VwzyRijQ7//KrG7p7QhDI7BFe5/ty/k0BStfRULu3aQB7d+/jypeonGeaMqAvCve7L9zmVj+V/ApZ7nxqUsgIytVIcoE43eurnRR62QrOJmzbG1YX6AFHx8JPW0C+lX1nEjBMWNKcNJ1zmaBIZ9sopKt9vFrhZraO8bj9YF64IKrWRQ+EJ8EaPydOfpmSEAw1EU2t5y1qU6g3S0yoNEwOSRd6cfN5PK/JJqcIBiP44Z+Yx+REoiVfPF46Z3w==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 29 Apr 2022 16:11:40 +0000
  • Ironport-data: A9a23:/NDqUarAfB83wlPnXTQV0fLprDxeBmJsZBIvgKrLsJaIsI4StFCzt garIBnXM/feNzb1LYt0boy29kwD65TTydAwTgRkriA3QSwT8ZuZCYyVIHmrMnLJJKUvbq7GA +byyDXkBJppJpMJjk71atANlVEliefQAOCU5NfsYkidfyc9IMsaoU8lyrdRbrJA24DjWVvR4 4Kq+KUzBXf+s9JKGjNMg068gEsHUMTa4Fv0aXRnOJinFHeH/5UkJMp3yZOZdhMUcaENdgKOf M7RzanRw4/s10xF5uVJMFrMWhZirrb6ZWBig5fNMkSoqkAqSicais7XOBeAAKv+Zvrgc91Zk b1wWZKMpQgBIL/PgKMgfEdjLyxPF78Y8+DKemGlmJnGp6HGWyOEL/RGKmgTZNRd0MAnRGZE+ LofNSwHaQ2Fi6Su2rWnR+Jwh8Mlas72IIcYvXImxjbcZRokacmbH+OWupkFjHFp2Zsm8fX2P qL1bRJ1axvNeVtXM0o/A5Mihua4wHL4dlW0rXrL9PRrvTOLnGSd1pCzM8PcIMOpXvxKl2Gdv UTd5Um+GVIFYYn3JT2ttyjEavX0tTP2XsceGaO18tZugUaP3SoDBRsOT1y5rPKlzEmkVLp3K UYZ5y4vpqga71GwQ5/2WBjQiGGAlg4RXZxXCeJS1e2W4q/d4gLcDG5USDdEMYUirJVvGmds0 UKVldT0AzApqKeSVX+W6raTq3W1JDQRKmgBIyQDSGPp/uXenW36tTqXJv4LLUJ/poed9e3Yq 9xSkBUDug==
  • Ironport-hdrordr: A9a23:D06Iyq9VVmzpgm402Z1uk+FRdb1zdoMgy1knxilNoENuH/Bwxv rFoB1E73TJYVYqN03IV+rwXZVoZUmsjaKdhrNhRotKPTOWwVdASbsP0WKM+V3d8kHFh41gPO JbAtJD4b7LfCdHZKTBkW6F+r8bqbHokZxAx92uqUuFJTsaF52IhD0JbjpzfHcGJjWvUvECZe ehD4d81nKdUEVSSv7+KmgOXuDFqdGOvJX6YSQeDxpizAWVlzun5JPzDhDdh34lInly6IZn1V KAvx3y562lvf3+4hjA11XL55ATvNf60NNMCOGFl8BQADTxjQSDYphnRtS5zXsIidDqzGxvvM jHoh8mMcg2w3TNflutqR+o4AXk2CZG0Q6U9bc16UGT0vDRdXYfMY5slIhZehzW5w4Lp9dnyp 9G2Gqfqt5+EQ7AtD6V3amHazha0m6P5VYym+8aiHJSFaEEbqVKkIAZ9ERJVL8dASPB7pw9Gu UGNrCS2B9vSyLbU5nlhBgt/DT1NU5DXCtuA3Jy9vB96gIm3UyQlCAjtYkidnRpzuNId3AL3Z WBDk1SrsA+ciYnV9MCOA4/e7rGNoXse2O7DIvAGyWvKEk4U0i92aLf0fES2NyAXqAu4d8bpK nhOWkowFLaPXieRPGz4A==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Apr 29, 2022 at 05:49:42PM +0200, Roger Pau Monné wrote:
> On Fri, Apr 29, 2022 at 12:59:58PM +0200, Jan Beulich wrote:
> > On 27.04.2022 12:47, Roger Pau Monne wrote:> Changes since v3:>  - Align 
> > ssbd per-core struct to a cache line.>  - Open code a simple spinlock to 
> > avoid playing tricks with the lock>    detector.>  - 
> > s/ssbd_core/ssbd_ls_cfg/.>  - Fix log message wording.>  - Fix define name 
> > and remove comment.>  - Also handle Hygon processors (Fam18h).>  - Add 
> > changelog entry.
> > What is this last line about?
> 
> Hm, seems like I forgot to do a patch refresh... So new version will
> have an entry about adding VIRT_SSBD support to HVM guests. Sorry for
> spoiling the surprise.
> 
> > > +bool __init amd_setup_legacy_ssbd(void)
> > > +{
> > > + unsigned int i;
> > > +
> > > + if ((boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18) ||
> > > +     boot_cpu_data.x86_num_siblings <= 1)
> > > +         return true;
> > > +
> > > + /*
> > > +  * One could be forgiven for thinking that c->x86_max_cores is the
> > > +  * correct value to use here.
> > > +  *
> > > +  * However, that value is derived from the current configuration, and
> > > +  * c->cpu_core_id is sparse on all but the top end CPUs.  Derive
> > > +  * max_cpus from ApicIdCoreIdSize which will cover any sparseness.
> > > +  */
> > > + if (boot_cpu_data.extended_cpuid_level >= 0x80000008) {
> > > +         ssbd_max_cores = 1u << MASK_EXTR(cpuid_ecx(0x80000008), 0xf000);
> > > +         ssbd_max_cores /= boot_cpu_data.x86_num_siblings;
> > > + }
> > > + if (!ssbd_max_cores)
> > > +         return false;
> > > +
> > > + ssbd_ls_cfg = xzalloc_array(struct ssbd_ls_cfg,
> > > +                             ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS);
> > > + if (!ssbd_ls_cfg)
> > > +         return false;
> > > +
> > > + for (i = 0; i < ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS; i++)
> > > +         /* Record initial state, also applies to any hotplug CPU. */
> > > +         if (opt_ssbd)
> > > +                 ssbd_ls_cfg[i].count = boot_cpu_data.x86_num_siblings;
> > 
> > Perhaps flip if() and for()?
> 
> Indeed, thanks.
> 
> > > +void amd_set_legacy_ssbd(bool enable)
> > > +{
> > > + const struct cpuinfo_x86 *c = &current_cpu_data;
> > > + struct ssbd_ls_cfg *status;
> > > +
> > > + if (c->x86 != 0x17 || c->x86_num_siblings <= 1) {
> > > +         BUG_ON(!set_legacy_ssbd(c, enable));
> > > +         return;
> > > + }
> > > +
> > > + BUG_ON(c->phys_proc_id >= AMD_FAM17H_MAX_SOCKETS);
> > > + BUG_ON(c->cpu_core_id >= ssbd_max_cores);
> > > + status = &ssbd_ls_cfg[c->phys_proc_id * ssbd_max_cores +
> > > +                       c->cpu_core_id];
> > > +
> > > + /*
> > > +  * Open code a very simple spinlock: this function is used with GIF==0
> > > +  * and different IF values, so would trigger the checklock detector.
> > > +  * Instead of trying to workaround the detector, use a very simple lock
> > > +  * implementation: it's better to reduce the amount of code executed
> > > +  * with GIF==0.
> > > +  */
> > > + while ( test_and_set_bool(status->locked) )
> > > +     cpu_relax();
> > > + status->count += enable ? 1 : -1;
> > > + ASSERT(status->count <= c->x86_num_siblings);
> > > + if (enable ? status->count == 1 : !status->count)
> > > +         BUG_ON(!set_legacy_ssbd(c, enable));
> > 
> > What are the effects of ASSERT() or BUG_ON() triggering in a GIF=0
> > region?
> 
> So AFAICT the BUG itself works, the usage of a crash kernel however
> won't work as it's booted with GIF==0.
> 
> Maybe we need to issue an stgi as part of BUG_FRAME if required?
> (maybe that's too naive...)

Well, better in panic() or kexec_crash() likely.

Roger.



 


Rackspace

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