Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <897b503e-48d1-bc4b-1e68-42792d589ddb@amd.com>
Date:   Thu, 5 Jan 2023 16:54:23 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.1
Subject: Re: [PATCH RFC v7 25/64] crypto: ccp: Add support to initialize the
 AMD-SP for SEV-SNP
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>,
        Michael Roth <michael.roth@amd.com>
Cc:     kvm@vger.kernel.org, linux-coco@lists.linux.dev,
        linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org,
        linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com,
        jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com,
        ardb@kernel.org, pbonzini@redhat.com, seanjc@google.com,
        vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com,
        luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com,
        pgonda@google.com, peterz@infradead.org,
        srinivas.pandruvada@linux.intel.com, rientjes@google.com,
        dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de,
        vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com,
        tony.luck@intel.com, marcorr@google.com,
        sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com,
        dgilbert@redhat.com, harald@profian.com,
        Brijesh Singh <brijesh.singh@amd.com>
References: <20221214194056.161492-1-michael.roth@amd.com>
 <20221214194056.161492-26-michael.roth@amd.com> <Y7VtOQa5Hd0oMdfh@kernel.org>
From:   "Kalra, Ashish" <ashish.kalra@amd.com>
In-Reply-To: <Y7VtOQa5Hd0oMdfh@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N1ErRTBBSGpxSE5NU2tudlpjaW9yQW4rOHRYVmlKekNCLzBIeDZJTjNUbHJr?=
 =?utf-8?B?ek9BaXoyckZZNjFITy9XOXRZcGIvUmdxNy9Xb1ZZbmx3dXQwSXdwbU9WS3Z1?=
 =?utf-8?B?NHRaekMwWkRxSUFPWGw2cXNvZGdhQ0Y2YzlKUjZHeVRyK00zdi8zcnRaQ0ZT?=
 =?utf-8?B?QjB3anZvR21VcEdpbHZZdUJFc0p6Z0dLTjRGeW9uaGJiUytTRjZQbXFUU1pp?=
 =?utf-8?B?dkI0Sk5WRS82d2daSk9ZVHZISkpJR1F1WVlxZ01WcVpJU0lSU1NDMVZzZHd1?=
 =?utf-8?B?STNUc0ozOFBFTnlHREVmdGhybm9pRit4ZmdrU1o4ckVFV3NiRDNhdWFBaHdX?=
 =?utf-8?B?L3dTU0ljRzZ0dC9ENExyNEZwWWgzK2lvNHpLNjZNTWJoODZFb0hhTUJ3dGo0?=
 =?utf-8?B?WE52MFZnN1pkeDV6M2JNblk5N3R1S05RWlJ6MDU3TFJmd0hJRytQVkpiZ1Mr?=
 =?utf-8?B?Zi9MZE1QVVdmY0JsdFRqQTlzNkZkK29PaEcxQlZtandhL1RuT0QyTGFUY3BQ?=
 =?utf-8?B?Z29oUzYrQ1dqYSs1TU5kWXI3UmZtSE1ZOWg4dmpjWUtQWlFwaWVxNHZIWnlw?=
 =?utf-8?B?Q2plS2lYU1IwUTlPMUVkTlg5ZmV6OXhTTXNGM0lBOXg2dWtTbHF0Vlk3ZTAv?=
 =?utf-8?B?cnZibTV6SG1uM05pUVNCaFlCeEQySlJpbGNOUkNuRUIyWnliMWVqU3RNOXli?=
 =?utf-8?B?K0ZoclRlM21XcWJ3R0FYY2EzRDQvVE9rd3VXeXBwWWhjVFpEMXFVdXhwckZx?=
 =?utf-8?B?QVBNWTRMaFN2UzI0VmtYaXZkRGRwOHJXcUtTRENKdWx5ZlFiRFFGc1NLR3Vo?=
 =?utf-8?B?SG9mMmZqK091NTlqWHluWE4yNGhxVmEwTHBaSGExbGQ2bUNOL1NpTVZ3QTV5?=
 =?utf-8?B?TndORFplWVNBdTdXK0M2ajEzMFBEa2g2akV5dk9QUFdjeFNOSkN6TWNKUlpl?=
 =?utf-8?B?VXRObmFyNEJtTUtiUVAvMFdKUG95azZiUUViVHVlY2hKVE5BcmlXSVowQmxs?=
 =?utf-8?B?aGZ2bkR1Z1B2ek9DSnNCU1UwVGtSaHVCU1JHZ2pjYW04ci8rUGx6dURlOVZz?=
 =?utf-8?B?TXVNb2JkUTV1UEJnOGh0VUFSM1YwU3UyK3R6T3BLRWxacHFWL2NqTjMxV2Jj?=
 =?utf-8?B?QnlzaWZrUGJRZHM4aU1QK0Z2eDA3UWc0aEFSdmxHQjd4UUxrZjdEdEZZU0VO?=
 =?utf-8?B?Y3crQk83dG9GRjVKQkIwdFZVeGlYb05ISU5pelRjUy9VSUh4MHBHN2dCSHQ1?=
 =?utf-8?B?TndBOGdRREdYd0g1K2EzUEMrZUdSelZlNXVXbHRiNE5oOUhWdVd0TnBwQ216?=
 =?utf-8?B?cW14V0F6MWd5U0RTSEVSK2ZrWWxxQ1I0cTlMdjBqYU93TlRtbHR2NzVyNkRj?=
 =?utf-8?B?M2REcDRIRXlLeGNkbmpnU1VldnhEd3p3dUMraWgxWnlpZ1pQc3Z3R05DY0xw?=
 =?utf-8?B?cHNWNGZlVU5BZnRZdTJTTnZWZHdOS29QdTFpSHpUL01qYlBQUzNYVlc5aExw?=
 =?utf-8?B?ZUxYVEd3MW5UK1NXbjhQbVlrZjFvSEMxcVFmbjZZNHRTVktMQldDZmliRVYw?=
 =?utf-8?B?ZTRCWHR6SWEyWXVZOXcrblFnK2tOaXNoeDRPbEkxQkxlVTZ3dkVJOG9yL3pv?=
 =?utf-8?B?dkNvZWE5L2E1Zy9vMWhtWFIwTkkrb1U0MzFQcnZnZjd5ZDZjb3hYU2k0OFNy?=
 =?utf-8?B?NG44RzZkL1FIS0RiWHNZMzVTa2k2U1VLbUxaMVBUWlVSUjJ5NDc2b0JiL3R2?=
 =?utf-8?B?aGpHRmpFanUza3B2Y0xxZkxBeStkTEtQVW1jVmtOUTR2czNJbjVNcUZmbHJG?=
 =?utf-8?B?RkdtMldqRjcrYkNOK2RSL01QZW9Vamd1WC9oVzREM09HUDJPSHRzQmg4SGIx?=
 =?utf-8?B?dmlBQ2V0RjN2OE82QjAzZEd6UXVjWGJrZ1pvTE4yNmsxR08vV1lHWEp3VHZn?=
 =?utf-8?B?amJnYjRWWURNVzhWNCtxQWpPMXBta2FlUGpUWkNyc0ZqNUFpUWcyVy9lQURk?=
 =?utf-8?B?ZGMwSS9EWVVKbDBnb3NKNlptWElMK1JCaUl5dFEzcXhmcUNKdERNdkhOeUtV?=
 =?utf-8?B?WG1yT1JaTzhXbmRSWmJxOE8vdjRWUGc3TDluRlZ6OGtlb09UVzNuNnVmUzBE?=
 =?utf-8?Q?VMTcBKL+PBIJEtY2ugXW/qkBj?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dae056b2-858a-4b7a-4074-08daef6fcd4e
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2767.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jan 2023 22:54:29.3243
 (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: fEBSIKXTn4gyL+L/W2gplR7LClAeJ2jAwYwi9NlFl4TAlb0GASR/W25BumY4YvA/NPC9oB13OiQN93ZDxUegtQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7253
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

Hello Jarkko,

On 1/4/2023 6:12 AM, Jarkko Sakkinen wrote:
> On Wed, Dec 14, 2022 at 01:40:17PM -0600, Michael Roth wrote:
>> +	/*
>> +	 * If boot CPU supports SNP, then first attempt to initialize
>> +	 * the SNP firmware.
>> +	 */
>> +	if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) {
>> +		if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) {
>> +			dev_err(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n",
>> +				SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
>> +		} else {
>> +			rc = sev_snp_init(&error, true);
>> +			if (rc) {
>> +				/*
>> +				 * Don't abort the probe if SNP INIT failed,
>> +				 * continue to initialize the legacy SEV firmware.
>> +				 */
>> +				dev_err(sev->dev, "SEV-SNP: failed to INIT error %#x\n", error);
>> +			}
>> +		}
>> +	}
> 
> I think this is not right as there is a dep between sev init and this,
> and there is about a dozen of call sites already __sev_platform_init_locked().
> 

sev_init ?

As this is invoked during CCP module load/initialization, shouldn't this 
get invoked before any other call sites invoking 
__sev_platform_init_locked() ?

Thanks,
Ashish

> Instead there should be __sev_snp_init_locked() that would be called as
> part of __sev_platform_init_locked() flow.
> 
> Also TMR allocation should be moved inside __sev_platform_init_locked,
> given that it needs to be marked into RMP after SNP init.
> 
> BR, Jarkko
>