linux-kernel - RE: [PATCH Part2 v6 03/49] x86/sev: Add the host SEV-SNP initialization support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Jul 2022 03:56:25 +0000
From:   "Kalra, Ashish" <Ashish.Kalra@....com>
To:     Borislav Petkov <bp@...en8.de>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "jroedel@...e.de" <jroedel@...e.de>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "hpa@...or.com" <hpa@...or.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "slp@...hat.com" <slp@...hat.com>,
        "pgonda@...gle.com" <pgonda@...gle.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "srinivas.pandruvada@...ux.intel.com" 
        <srinivas.pandruvada@...ux.intel.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "dovmurik@...ux.ibm.com" <dovmurik@...ux.ibm.com>,
        "tobin@....com" <tobin@....com>,
        "Roth, Michael" <Michael.Roth@....com>,
        "vbabka@...e.cz" <vbabka@...e.cz>,
        "kirill@...temov.name" <kirill@...temov.name>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "marcorr@...gle.com" <marcorr@...gle.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "alpergun@...gle.com" <alpergun@...gle.com>,
        "dgilbert@...hat.com" <dgilbert@...hat.com>,
        "jarkko@...nel.org" <jarkko@...nel.org>
Subject: RE: [PATCH Part2 v6 03/49] x86/sev: Add the host SEV-SNP
 initialization support

[AMD Official Use Only - General]

Hello Boris,

>> +	 * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
>> +	 * information on memory requirement.

>That section number will change over time - if you want to refer to some section just use its title so that people can at least grep for the relevant text.

This will all go into sev.c, instead of the header file, as this is non-architectural and per-processor and the structure won't be exposed to the rest
of the kernel.  The above PPR reference and potentially in future an architectural method of reading the RMP table entries will be moved into it.

>> +	 */
>> +	nr_pages = totalram_pages();
>> +	calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + 
>> +RMPTABLE_CPU_BOOKKEEPING_SZ;

>use totalram_pages() directly and get rid of nr_pages.
Ok.

>> +	 * kexec boot.
>> +	 */
>> +	rdmsrl(MSR_AMD64_SYSCFG, val);
>> +	if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> +		goto skip_enable;
>> +
>> +	/* Initialize the RMP table to zero */
>> +	memset(start, 0, sz);

>Do I understand it correctly that in the kexec case the second, kexec-ed kernel is reusing the previous kernel's RMP table so it should not be cleared?
I believe that with kexec and after issuing the shutdown command, the RMP table needs to be fully initialized, so we should be re-initializing the RMP
table to zero here.

>>
>> +
>> +static int __init snp_rmptable_init(void) {
>> +	if (!boot_cpu_has(X86_FEATURE_SEV_SNP))

>cpu_feature_enabled
Ok.

>> +		return 0;
>> +
>> +	if (!iommu_sev_snp_supported())
>> +		goto nosnp;
>> +
>> +	if (__snp_rmptable_init())
>> +		goto nosnp;
>> +
>> +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", 
>> +__snp_enable, NULL);
>> +
>> +	return 0;
>> +
>> +nosnp:
>> +	setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
>> +	return 1;
>> +}
>> +
>> +/*
>> + * This must be called after the PCI subsystem. This is because 
>> +before enabling
>> + * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
>> + * The iommu_sev_snp_support() is used for checking the feature, and 
>> +it is
>> + * available after subsys_initcall().

>I'd much more appreciate here a short formulation explaining why is IOMMU needed for SNP rather than the obvious.

Yes, IOMMU is enforced for SNP to ensure that HV cannot program DMA directly into guest private memory. In case of SNP,
the IOMMU makes sure that the page(s) used for DMA are HV owned.

Thanks,
Ashish