linux-kernel - Re: [PATCH v3] x86/sev: Add SEV-SNP guest feature negotiation 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] [day] [month] [year] [list]
Message-ID: <46ec426b-e6bb-5286-77c2-1c4cf1fdda93@amd.com>
Date:   Wed, 4 Jan 2023 09:56:21 +0530
From:   "Nikunj A. Dadhania" <nikunj@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, kvm@...r.kernel.org,
        mingo@...hat.com, tglx@...utronix.de, dave.hansen@...ux.intel.com,
        seanjc@...gle.com, pbonzini@...hat.com, thomas.lendacky@....com,
        michael.roth@....com, stable@...nel.org
Subject: Re: [PATCH v3] x86/sev: Add SEV-SNP guest feature negotiation support



On 03/01/23 19:10, Borislav Petkov wrote:
> On Mon, Jan 02, 2023 at 02:08:10PM +0530, Nikunj A Dadhania wrote:
>> The hypervisor can enable various new features (SEV_FEATURES[1:63])
>> and start the SNP guest. Some of these features need guest side
>> implementation. If any of these features are enabled without guest
>> side implementation, the behavior of the SNP guest will be undefined.
>> The SNP guest boot may fail in a non-obvious way making it difficult
>> to debug.
>>
>> Instead of allowing the guest to continue and have it fail randomly
>> later, detect this early and fail gracefully.
>>
>> SEV_STATUS MSR indicates features which hypervisor has enabled. While
> 					 ^
> 					 the

Sure.

> 
>> booting, SNP guests should ascertain that all the enabled features
>> have guest side implementation. In case any feature is not implemented
>> in the guest, the guest terminates booting with SNP feature
>> unsupported exit code.
>>
>> More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR
>>
>> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F40332_4.05.pdf&data=05%7C01%7Cnikunj.dadhania%40amd.com%7C6575db7c0d8f4f136d1f08daed902274%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638083500715058552%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=weTrXWfPxDBdu9OsFZ4FxlvlgbhhG%2F985%2Bii%2BM8vh6I%3D&reserved=0
>>
>> Fixes: cbd3d4f7c4e5 ("x86/sev: Check SEV-SNP features support")
>> CC: Borislav Petkov <bp@...en8.de>
>> CC: Michael Roth <michael.roth@....com>
>> CC: Tom Lendacky <thomas.lendacky@....com>
>> CC: <stable@...nel.org>
>> Signed-off-by: Nikunj A Dadhania <nikunj@....com>
> 
> ...
> 
>> diff --git a/Documentation/x86/amd-memory-encryption.rst b/Documentation/x86/amd-memory-encryption.rst
>> index a1940ebe7be5..b8b6b87be995 100644
>> --- a/Documentation/x86/amd-memory-encryption.rst
>> +++ b/Documentation/x86/amd-memory-encryption.rst
>> @@ -95,3 +95,38 @@ by supplying mem_encrypt=on on the kernel command line.  However, if BIOS does
>>  not enable SME, then Linux will not be able to activate memory encryption, even
>>  if configured to do so by default or the mem_encrypt=on command line parameter
>>  is specified.
>> +
>> +Secure Nested Paging (SNP):
> 
> No ":"
> 

Done

>> +===========================
> 
> <---- newline here.

Done

> 
>> +SEV-SNP introduces new features (SEV_FEATURES[1:63]) which can be enabled
>> +by the hypervisor for security enhancements. Some of these features need
>> +guest side implementation to function correctly. The below table lists the
>> +expected guest behavior with various possible scenarios of guest/hypervisor
>> +SNP feature support.
>> +
>> ++---------------+---------------+---------------+---------------+
>> +|Feature Enabled|  Guest needs  |   Guest has   |  Guest boot   |
>> +|     by HV     |implementation |implementation |   behavior    |
>> ++---------------+---------------+---------------+---------------+
>> +|      No       |      No       |      No       |     Boot      |
>> +|               |               |               |               |
>> ++---------------+---------------+---------------+---------------+
>> +|      No       |      Yes      |      No       |     Boot      |
>> +|               |               |               |               |
>> ++---------------+---------------+---------------+---------------+
>> +|      No       |      Yes      |      Yes      |     Boot      |
>> +|               |               |               |               |
>> ++---------------+---------------+---------------+---------------+
>> +|      Yes      |      No       |      No       |   Boot with   |
>> +|               |               |               |feature enabled|
>> ++---------------+---------------+---------------+---------------+
>> +|      Yes      |      Yes      |      No       | Graceful Boot |
>> +|               |               |               |    Failure    |
>> ++---------------+---------------+---------------+---------------+
>> +|      Yes      |      Yes      |      Yes      |   Boot with   |
>> +|               |               |               |feature enabled|
>> ++---------------+---------------+---------------+---------------+
> 
> sphinx is not happy about that table for some reason. I always find the error
> messages cryptic though:

sphinx uses spaces before multi-line text as block quote. Also, found that after the table header it needs line with "=" and not "-".

> Documentation/x86/amd-memory-encryption.rst:110: WARNING: Block quote ends without a blank line; unexpected unindent.
> Documentation/x86/amd-memory-encryption.rst:110: WARNING: Block quote ends without a blank line; unexpected unindent.
> Documentation/x86/amd-memory-encryption.rst:122: WARNING: Block quote ends without a blank line; unexpected unindent.
> Documentation/x86/amd-memory-encryption.rst:128: WARNING: Block quote ends without a blank line; unexpected unindent.
> 
> You can repro by doing "make htmldocs".

Fixed, 

+-----------------+---------------+---------------+------------------+
| Feature Enabled | Guest needs   | Guest has     | Guest boot       |
| by the HV       | implementation| implementation| behaviour        |
+=================+===============+===============+==================+
|      No         |      No       |      No       |     Boot         |
|                 |               |               |                  |
+-----------------+---------------+---------------+------------------+
|      No         |      Yes      |      No       |     Boot         |
|                 |               |               |                  |
+-----------------+---------------+---------------+------------------+
|      No         |      Yes      |      Yes      |     Boot         |
|                 |               |               |                  |
+-----------------+---------------+---------------+------------------+
|      Yes        |      No       |      No       | Boot with        |
|                 |               |               | feature enabled  |
+-----------------+---------------+---------------+------------------+
|      Yes        |      Yes      |      No       | Graceful boot    |
|                 |               |               | failure          |
+-----------------+---------------+---------------+------------------+
|      Yes        |      Yes      |      Yes      | Boot with        |
|                 |               |               | feature enabled  |
+-----------------+---------------+---------------+------------------+


Regards
Nikunj