Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH Part2 v5 23/45] KVM: SVM: Add KVM_SNP_INIT command
To:     Marc Orr <marcorr@google.com>
Cc:     x86 <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        kvm list <kvm@vger.kernel.org>, linux-coco@lists.linux.dev,
        linux-mm@kvack.org, linux-crypto@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>, tony.luck@intel.com,
        sathyanarayanan.kuppuswamy@linux.intel.com
References: <20210820155918.7518-1-brijesh.singh@amd.com>
 <20210820155918.7518-24-brijesh.singh@amd.com>
 <CAA03e5GaROVunr9JkBeULx2KN5kkE8TU_sjBANtAj7E-V6sN_w@mail.gmail.com>
From:   Brijesh Singh <brijesh.singh@amd.com>
Message-ID: <8a74e454-3485-1e95-a1ea-0f79889abcb9@amd.com>
Date:   Mon, 13 Sep 2021 06:32:37 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
In-Reply-To: <CAA03e5GaROVunr9JkBeULx2KN5kkE8TU_sjBANtAj7E-V6sN_w@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dXNxVFpXQkU2RGNpWFF0L3VxcEtTWCs2dXhtSXluaHUweGo5VEY5M2MvV0Qv?=
 =?utf-8?B?MDkzNmFRTEYwdnpzVVJ2emh0WHV2YlNzSGNZMkdDVUFKVG5zTTZPb0NPMEgw?=
 =?utf-8?B?N1E2c2pQTlhmVkwyYTJNODQ1TjJTSWVlZjA5VlRFV0NZcHNZZmthZGRFTHRk?=
 =?utf-8?B?akEyZTJHZFAzN3JqeHhQWHhXTWlBQ3FmVS9lMWJ3QkNWOVpKMHBFRTY1M0lD?=
 =?utf-8?B?OFMrQVpDWnMybkg1anh3THVVa0c5dk4rRHhzRWtYUkZYdFBHWnBhbmw5TGh2?=
 =?utf-8?B?M0JwZS9vdW9ucjVJdUVXRHBaRVRqdFFHRUVWeUNGQ3pkcS94ZFMyeGt3d2c0?=
 =?utf-8?B?eWVWeVpkNEtoM1Zpd0svdUV6RldCTEJyTUp0L1h6UUJKU1VuL0pmQjB3RGxR?=
 =?utf-8?B?R0dvS1BDY25SdjlGR0FMTzdMRnRHNHh3K1BrRmp1RHU3bjdGNkt2NGtMcm5a?=
 =?utf-8?B?RmdJTisyUEI2Y0hDbzlqOXNoTmNuWVNPWUxJNExGKzQ1N0pHUVBQaGs4eWpC?=
 =?utf-8?B?b0dSYnhFd1ZBRTNPTHB6UVVieGZXS1Y3cEhyeFdMVGdTVVBZcXlFL2pMOG95?=
 =?utf-8?B?MFBpQnZDUC9zbzRxZkN4ZFJ3SlpLQ0gyNXR6R2o5Q0RxK0h0VVF3ZEM4ZGxy?=
 =?utf-8?B?NEdja0lEVEl4K1RRaGVtZGZEV2srdFYyYXlFQXdTbEpjS0JpcGhtTi9XZG1Z?=
 =?utf-8?B?b3lVTWNhMlhxVWJFeUgvUitVMjhheEdSdXpHVDhDUUlEYnZlN0lJV2hMc2oz?=
 =?utf-8?B?RUxhd0xQcjhiNkc1OE9sNmxJUWtHTzNSODdJdGN2MHBMVWR6VWlTMUNzYzNl?=
 =?utf-8?B?dEYwdWdKdEVlU2JVU3N3WGdUaDZReHdGdEZ2d1MyLzdIOFRKU2pZdzNDTHFm?=
 =?utf-8?B?VTVLcVE2bW5LbUszQVBiTllNai9ZTzNXbkRaVVZ2WC9sSDV0amxyazdNdzdI?=
 =?utf-8?B?R0NiYkZmZWNkSHdscDdoTE5zK0s4Q3k3UTJkeU9UY3hXbjlyYy9vY2Q1OEdh?=
 =?utf-8?B?Z0FvWlRHVmEwMWRUUXFucU5FWDZMd3NmVmMxdHk3UGNzUkRCSEg3dWdUNXZi?=
 =?utf-8?B?NHdINmloTEVEaFQrZUFRL1lJV1pPbkRjYWNzUDR1cDdrME5TZWlYY3hVNG9m?=
 =?utf-8?B?MlFlK1pRcC9yK3hFOWRqOFFCSHJVL2w3VmZGSzNHeEo2cWNIWWtueTFWZHVB?=
 =?utf-8?B?aGljNllONjExdXJxcGlxZ3Z1YzFPWDkxSS9Eazl3RnFxOHMvZU1ZRVpmVGlU?=
 =?utf-8?B?akwyTHovZmJSdUZ5SEVZSVBMazc1U3MwUXpPNGU0NnY1cTZOVStmekIxdXNY?=
 =?utf-8?B?a1BaVldyTDZLWTl6M2JMTXhEVm1OcTNvbUtuN1FGejlWckJXdHYwbURXb1h5?=
 =?utf-8?B?dldkMFdnSjQ4a25lVVk2WVVuaStVYkNTTE1HZUIyNGxpTzErQ1VRcmN6WHRM?=
 =?utf-8?B?R1NkbkhQSTlFcDRzbzVEUi9zZ1pVWHhDY2VSTk1PN0dKT2R0enNFNENNc3BW?=
 =?utf-8?B?N1huUHl1cDlxOFB1eHNWenp0YlBMalNIQnlBNHZVeEVyczRhREhOZjdLWDlz?=
 =?utf-8?B?Y3FEaVJnMkZNT1Fyc3RwQk5TQnpOaGFQNGhNK3V3eklqdGpicU12WXhqTFBt?=
 =?utf-8?B?aktGOGYyT2J1M3NrQTBGQXZ5Z2FldXFwbENaOHNvQ0YrWi9UU0NRY1llTE9N?=
 =?utf-8?B?UHNOSnpYVXVyNVB2Z3g5VTA5dFl6dzY1T3NrNVlFWUx1VlNNdCtDaDZBU08y?=
 =?utf-8?Q?C6UpqsrPO0c/NRPBk2cXxKix8/fAKgCYvpkSvuo?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7b2612ca-95c6-4c79-63ac-08d976aa3221
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2021 11:32:41.0550
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fqSLV8j4Callt1Qq0zsMWkmxIRc8+KUl2cYknFL/k1/NOIKnic7DTiQJwu9v1EFri9sWK/DBsx33YKe95oKu9g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB2365
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org


On 9/9/21 10:32 PM, Marc Orr wrote:
> On Fri, Aug 20, 2021 at 9:00 AM Brijesh Singh <brijesh.singh@amd.com> wrote:
>> The KVM_SNP_INIT command is used by the hypervisor to initialize the
>> SEV-SNP platform context. In a typical workflow, this command should be the
>> first command issued. When creating SEV-SNP guest, the VMM must use this
>> command instead of the KVM_SEV_INIT or KVM_SEV_ES_INIT.
>>
>> The flags value must be zero, it will be extended in future SNP support to
>> communicate the optional features (such as restricted INT injection etc).
>>
>> Co-developed-by: Pavan Kumar Paluri <papaluri@amd.com>
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  .../virt/kvm/amd-memory-encryption.rst        | 27 ++++++++++++
>>  arch/x86/include/asm/svm.h                    |  2 +
>>  arch/x86/kvm/svm/sev.c                        | 44 ++++++++++++++++++-
>>  arch/x86/kvm/svm/svm.h                        |  4 ++
>>  include/uapi/linux/kvm.h                      | 13 ++++++
>>  5 files changed, 88 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
>> index 5c081c8c7164..7b1d32fb99a8 100644
>> --- a/Documentation/virt/kvm/amd-memory-encryption.rst
>> +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
>> @@ -427,6 +427,33 @@ issued by the hypervisor to make the guest ready for execution.
>>
>>  Returns: 0 on success, -negative on error
>>
>> +18. KVM_SNP_INIT
>> +----------------
>> +
>> +The KVM_SNP_INIT command can be used by the hypervisor to initialize SEV-SNP
>> +context. In a typical workflow, this command should be the first command issued.
>> +
>> +Parameters (in/out): struct kvm_snp_init
>> +
>> +Returns: 0 on success, -negative on error
>> +
>> +::
>> +
>> +        struct kvm_snp_init {
>> +                __u64 flags;
>> +        };
>> +
>> +The flags bitmap is defined as::
>> +
>> +   /* enable the restricted injection */
>> +   #define KVM_SEV_SNP_RESTRICTED_INJET   (1<<0)
>> +
>> +   /* enable the restricted injection timer */
>> +   #define KVM_SEV_SNP_RESTRICTED_TIMER_INJET   (1<<1)
>> +
>> +If the specified flags is not supported then return -EOPNOTSUPP, and the supported
>> +flags are returned.
>> +
>>  References
>>  ==========
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 44a3f920f886..a39e31845a33 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -218,6 +218,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>>  #define SVM_NESTED_CTL_SEV_ENABLE      BIT(1)
>>  #define SVM_NESTED_CTL_SEV_ES_ENABLE   BIT(2)
>>
>> +#define SVM_SEV_FEAT_SNP_ACTIVE                BIT(0)
>> +
>>  struct vmcb_seg {
>>         u16 selector;
>>         u16 attrib;
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 50fddbe56981..93da463545ef 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -235,10 +235,30 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
>>         sev_decommission(handle);
>>  }
>>
>> +static int verify_snp_init_flags(struct kvm *kvm, struct kvm_sev_cmd *argp)
>> +{
>> +       struct kvm_snp_init params;
>> +       int ret = 0;
>> +
>> +       if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data, sizeof(params)))
>> +               return -EFAULT;
>> +
>> +       if (params.flags & ~SEV_SNP_SUPPORTED_FLAGS)
>> +               ret = -EOPNOTSUPP;
>> +
>> +       params.flags = SEV_SNP_SUPPORTED_FLAGS;
>> +
>> +       if (copy_to_user((void __user *)(uintptr_t)argp->data, &params, sizeof(params)))
>> +               ret = -EFAULT;
>> +
>> +       return ret;
>> +}
>> +
>>  static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>  {
>> +       bool es_active = (argp->id == KVM_SEV_ES_INIT || argp->id == KVM_SEV_SNP_INIT);
>>         struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
>> -       bool es_active = argp->id == KVM_SEV_ES_INIT;
>> +       bool snp_active = argp->id == KVM_SEV_SNP_INIT;
>>         int asid, ret;
>>
>>         if (kvm->created_vcpus)
>> @@ -249,12 +269,22 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>                 return ret;
>>
>>         sev->es_active = es_active;
>> +       sev->snp_active = snp_active;
>>         asid = sev_asid_new(sev);
>>         if (asid < 0)
>>                 goto e_no_asid;
>>         sev->asid = asid;
>>
>> -       ret = sev_platform_init(&argp->error);
>> +       if (snp_active) {
>> +               ret = verify_snp_init_flags(kvm, argp);
>> +               if (ret)
>> +                       goto e_free;
>> +
>> +               ret = sev_snp_init(&argp->error);
>> +       } else {
>> +               ret = sev_platform_init(&argp->error);
>> +       }
>> +
>>         if (ret)
>>                 goto e_free;
>>
>> @@ -600,6 +630,10 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>>         save->pkru = svm->vcpu.arch.pkru;
>>         save->xss  = svm->vcpu.arch.ia32_xss;
>>
>> +       /* Enable the SEV-SNP feature */
>> +       if (sev_snp_guest(svm->vcpu.kvm))
>> +               save->sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>> +
>>         return 0;
>>  }
>>
>> @@ -1532,6 +1566,12 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
>>         }
>>
>>         switch (sev_cmd.id) {
>> +       case KVM_SEV_SNP_INIT:
>> +               if (!sev_snp_enabled) {
>> +                       r = -ENOTTY;
>> +                       goto out;
>> +               }
>> +               fallthrough;
>>         case KVM_SEV_ES_INIT:
>>                 if (!sev_es_enabled) {
>>                         r = -ENOTTY;
>> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
>> index 01953522097d..57c3c404b0b3 100644
>> --- a/arch/x86/kvm/svm/svm.h
>> +++ b/arch/x86/kvm/svm/svm.h
>> @@ -69,6 +69,9 @@ enum {
>>  /* TPR and CR2 are always written before VMRUN */
>>  #define VMCB_ALWAYS_DIRTY_MASK ((1U << VMCB_INTR) | (1U << VMCB_CR2))
>>
>> +/* Supported init feature flags */
>> +#define SEV_SNP_SUPPORTED_FLAGS                0x0
>> +
>>  struct kvm_sev_info {
>>         bool active;            /* SEV enabled guest */
>>         bool es_active;         /* SEV-ES enabled guest */
>> @@ -81,6 +84,7 @@ struct kvm_sev_info {
>>         u64 ap_jump_table;      /* SEV-ES AP Jump Table address */
>>         struct kvm *enc_context_owner; /* Owner of copied encryption context */
>>         struct misc_cg *misc_cg; /* For misc cgroup accounting */
>> +       u64 snp_init_flags;
> This field never gets set anywhere. Should it get set in
> `verify_snp_init_flags()`?

Actually the supported flag value is zero, so didn't update it. But to
make code cleaner I will set the flag after the negotiation.


>
>>  };
>>
>>  struct kvm_svm {
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index d9e4aabcb31a..944e2bf601fe 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1712,6 +1712,9 @@ enum sev_cmd_id {
>>         /* Guest Migration Extension */
>>         KVM_SEV_SEND_CANCEL,
>>
>> +       /* SNP specific commands */
>> +       KVM_SEV_SNP_INIT,
>> +
>>         KVM_SEV_NR_MAX,
>>  };
>>
>> @@ -1808,6 +1811,16 @@ struct kvm_sev_receive_update_data {
>>         __u32 trans_len;
>>  };
>>
>> +/* enable the restricted injection */
>> +#define KVM_SEV_SNP_RESTRICTED_INJET   (1 << 0)
>> +
>> +/* enable the restricted injection timer */
>> +#define KVM_SEV_SNP_RESTRICTED_TIMER_INJET   (1 << 1)
>> +
>> +struct kvm_snp_init {
>> +       __u64 flags;
>> +};
>> +
>>  #define KVM_DEV_ASSIGN_ENABLE_IOMMU    (1 << 0)
>>  #define KVM_DEV_ASSIGN_PCI_2_3         (1 << 1)
>>  #define KVM_DEV_ASSIGN_MASK_INTX       (1 << 2)
>> --
>> 2.17.1
>>
>>