From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932728AbdJ3RtZ (ORCPT ); Mon, 30 Oct 2017 13:49:25 -0400 Received: from mail-cys01nam02on0084.outbound.protection.outlook.com ([104.47.37.84]:43424 "EHLO NAM02-CY1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932115AbdJ3RtW (ORCPT ); Mon, 30 Oct 2017 13:49:22 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=brijesh.singh@amd.com; Cc: brijesh.singh@amd.com, Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Herbert Xu , Gary Hook , Tom Lendacky , linux-crypto@vger.kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [Part2 PATCH v6.1 16/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support To: Borislav Petkov References: <20171020023413.122280-14-brijesh.singh@amd.com> <20171029204825.18260-1-brijesh.singh@amd.com> <20171030172150.3eibiqr44i43zbvd@pd.tnic> From: Brijesh Singh Message-ID: <1e89d599-b1d2-bb16-92f4-0409ee0a6ea0@amd.com> Date: Mon, 30 Oct 2017 12:49:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171030172150.3eibiqr44i43zbvd@pd.tnic> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: CY4PR04CA0056.namprd04.prod.outlook.com (10.171.243.149) To BY2PR12MB0147.namprd12.prod.outlook.com (10.162.82.20) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: af47a86f-d8e0-4a4c-d43c-08d51fbe8bb5 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(48565401081)(4534020)(4602075)(2017052603199);SRVR:BY2PR12MB0147; X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0147;3:WvNLhQoXAyRjuG2QdkKw31Kwvc4ZM28QDT3M6rNRKswbx+13C4maIfjjacqcAJSMq3DRG/n+qRQ6Ly3iz9rjMYHpgiD8R3PPSdk6oFHhqajO6gxNPWfHLeVjOu7XrJ217ov42VWTpC6wsvnPntmW4m9hYcRT5JIivvZctjVtT3+xhoaoIZ/t6c6ucDiyIK6up6SGBHLcmgsNupn/A9m1ybCj+Y4Z+kMGNpWyBhSRK1JAfQ7ttN83Q93b1MCs1nUG;25:4dfvwyKmhTsr4g+NUq79qe+1p6Q58imBL6XVsFdrJ3urAknWujPoZrDdZjj9Rk4OT3opjp/yFQnPBqDpyVwcoPNN2TS0LwcV1LOGid5CWuWJX+OuWr6S98Zk0aVeOpqwxOcNaCpzlVwjDd+59tCIHNsp+n3J+0v6985ZJA1XnLV8BjhdETF7UkCB/bQ8cegytlSvqDvJfQgE+q1SZ3rbXSaquS3uGh4xuNxqn6UN+8QOw24ruzKVNFiXvfbXm6+2muTfo6qnAeJBQeqR6PsmORp6bM4TrxHcLUdFYyLsEP0mE/zdA7Mzj0N18Do8etWYf6HWJfNeX+DwT4lzcQpx1A==;31:BbS70FRbSS2MKLuB0NScydCFdFT504sc7q3FjYMmcuL3mlPR50vCpUfqqNOOzSjKLUhDdaWoDmn3g1uB/Z7uxHMFdvYPBwZAd72SF/FxywSnuPa3DAOkvJHXfTgcJe9ypRSVknWevXy0p8szknXRrzTSFDasQCpmALxQS5d/jCq0GfUQg1WF7thfMfDjHa5rWuyMhiYbllzLjicvqhliG6VUtrzIm65FEwHQSLonAXs= X-MS-TrafficTypeDiagnostic: BY2PR12MB0147: X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0147;20:4WZ1DUi/Ew/WOWQY6P4qTvye3cY34Z3MQs5/kiA2nVxAU/fC1dTB/BAS6kiLzghBRX/wGLLXrPGl42+RhjYOXfcWt+/TGGMkbu0pWa4PrJdOr0enQWGNc+L3KvWO++v9XO1lmmL/vsC2KO+4YtIyI7U2vzja83r93Yij+b/FaXCQY9GVDnDaUFVGe9+/5BMdTPPm3cZfOXtck2C2ioYixSauwUc78L2uW4R0bmoNDOp2SncRl+UYgBhdz7IICgxlkMNwRFHwnwP6swBkryJoCn5+kFxxdgPlqu+dEobAit+/d5uSBvueYGnaQojPRrNekQaj5fzAIZZwrt+BdJkfL8pE3cOARhL3hWTqKkKi06ZgbQ7Vbos1+JKdixaStsgZr14HCDMyU8yuX1+UokPHlIQvQjTQnpkRYFnVqjsQe6zNp0nGjOZBT5+6e+tIPtzggE8RQGusxAiztwDMGVmdFvF6OHIo7XcYAcH3ZW7ngqGMRvfAEj3yWcj1kIYo7m06;4:2sMjm5DK0fvoa7C9MAJ1rZuTw5hddvQk20DkysFbtX+99WdDnL9KVGUqDmeIL7MhknJCLByrx75JKolNeL30cdIC3EkcK/ZjgkDaw3xdjSj8jkXm9BzOClEFsIx6wdEo26IBLYpI3VYMPQEvSkq8ABj7G5TwsKAoPqFfLfYFTI63PNDE0HTQYd2pgNH2LfPvNIKswmsfKyM6C+ns8Za/gJh/wWAyx5rp/ZFx8F3Fxy3XwSlHSx5c5kknRpdymJF6ALEYHM3H3UovPPAvRwGqdA== X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(3231020)(3002001)(10201501046)(100000703101)(100105400095)(6055026)(6041248)(20161123558100)(20161123560025)(20161123564025)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:BY2PR12MB0147;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:BY2PR12MB0147; X-Forefront-PRVS: 0476D4AB88 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(6049001)(39860400002)(376002)(346002)(24454002)(199003)(189002)(7736002)(33646002)(229853002)(8936002)(6916009)(90366009)(6486002)(77096006)(2950100002)(5660300001)(6666003)(65826007)(8676002)(101416001)(81156014)(81166006)(230700001)(6116002)(3846002)(105586002)(106356001)(58126008)(50466002)(189998001)(4326008)(16576012)(54906003)(316002)(97736004)(31686004)(6246003)(53546010)(64126003)(66066001)(65806001)(65956001)(50986999)(47776003)(53936002)(36756003)(16526018)(76176999)(478600001)(83506002)(68736007)(31696002)(54356999)(2906002)(305945005)(25786009)(86362001)(23676003);DIR:OUT;SFP:1101;SCL:1;SRVR:BY2PR12MB0147;H:[10.236.136.62];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCWTJQUjEyTUIwMTQ3OzIzOnMyakFoOVdINGtXdG1qM05nR2kzQmtScDRk?= =?utf-8?B?K3M2N0tpRkNCNWR6TTM2UitWbnZCUFB0QmFwK1VNVi8zMExRM0FyNExaWmE4?= =?utf-8?B?Ykl4dGxIVVZhRDhXcisyUXhjdEV4Zm5mcDE5NUk0NExMYXdmSGNlZGRBSXpm?= =?utf-8?B?ajJuY01mYUZ3N0xxMHpvK2F6WkwvVTdjditCZy9ONWdYR051ZEswWks2OGNK?= =?utf-8?B?dWtmeXNFZU1lU1doQ1ZvekdmRHRuMm9NdkR1R25PV1ZvcTBaRnNRYjQ0aTNx?= =?utf-8?B?dzd6Vjg5S3FodW5CNUZESUZVUmVRRHI0bm54am1DS1JHcERYRmpFMVNpcldB?= =?utf-8?B?N2Q5VUNhREYyOVo3MHRyR0VrRVRQSjFhZitUcjYyNFMzazYrMWRNQ05WbVRS?= =?utf-8?B?azZUOVdhWEFQZzBOQkc3aFRhb1B2RndGclo4emVRM3dZQk4wREt1d0QyTksz?= =?utf-8?B?K0tiUmc2c0J3WFpyMHFVV0JWQ3lpU1k5RTh5RVFYYnVYVEwzeGw5Y05uems0?= =?utf-8?B?WHMvNDk3MmVNN2lkSUh4L1NXM0dPK1BDNHB4azl6ckhkdXY2TzhLSFlkaE1o?= =?utf-8?B?MmpBTmZtRnpsUUZldUJJRWtWbkhzbkd4UEpjOHB2b2JPdVJERzhtb1NNalM4?= =?utf-8?B?U2t5b2VWMW5xaHpxRGoxa1ZUTitKaVNNajJnN3VkbjR3RnY3KzBrNGN5U2kz?= =?utf-8?B?MWRmN0FzcmJEUmlEblREa0VMbkxzNyt6UnNWS2hMdTdZTmwzZWJpaFR2VzFZ?= =?utf-8?B?TVhXWFcvVjlwL1hLQjdncDNmcjhtOEJjSVNuQWpsRklpMlh0VWtlR1NsZmVO?= =?utf-8?B?OEx0eU13dVAvWVcrUHM4TGU4SGQ0MU9KdGlvZDgrYmFQQjg3a21DaXl5YW9T?= =?utf-8?B?RnEwWU9ES3RlcThDVVJJanJ0NkRMUnNCa290bUg3cml2UlZ4WDBQbGIrSTMz?= =?utf-8?B?dnYvR1pMVGo2Ri9rMUVTeXRseGdhdEEvMm5Ld0o0MTlXczNDK2RFTENodk1V?= =?utf-8?B?KzV1Q0Zvc1Y5dzNNQkFlSEpsM05jblo1RUhDTjJIdzhwcTlwZmprZjNSSnVP?= =?utf-8?B?UUJvUUttQlc3enZuQmJhSnVsZ25tZzdPeis5TjNDc3ZUWklqdWNBV3FhaFpN?= =?utf-8?B?dFJ5b0h0cXozUngvNDhyTGhjZjlVNkFDL0N6S1ZZb1g5NnN4YWxzV0llZDk5?= =?utf-8?B?UjRoZm1CV203dm5rbjF6dmF5OEtRLytWcUVDcHFvWWZaalhodkswY1BhMnBn?= =?utf-8?B?RFlEMUdnbGtRbUh0NXNFOUNpZWNvbGhIN1BReVJZZHBOWFBWMElaOU5tL0tt?= =?utf-8?B?OWtnZjVMTWJTeFhtV0d5aTBRd0xnV20vL1d5Tmd1SnpLZklZZXlpNDZxSkJu?= =?utf-8?B?OHJvRTZCcmJ1dzhLcWYzb0Q0YjBYWDBwT1JjS0hqemF1akh1SUN2dlE5bDY4?= =?utf-8?B?Q1h6THYzZUc3SDhTUUNjc0ZyM2JHSDhoWEs0eTUwQ1U4d0xDYUEvNFJMVVJK?= =?utf-8?B?Q1JxNnBtY0FOQnZqSlczK2xHdDBRN21iQ1FROU0rZ1FmeVlmUWx6M2lKSmsv?= =?utf-8?B?Rm5XT2dac1EvcmZCalIzZFg5YkxUMmQ1SjAzMTd4VlZYSW1NanpNSHNXaE8y?= =?utf-8?B?NENhZ2N6OWE5SHNpWFU5bXRpY3hsWmJwNzljLzE0N1FjTnFUTUZrSEZ3K01j?= =?utf-8?B?MnJrYlJvc2J4WTk0YWFGVTVhQVdNSzkrcHB3ZG9sU2tNckpPSllsSDhKYm1V?= =?utf-8?B?bjlGOVpod2IyWHBjZ3RJTDR1U0E4UFYyYVdiQUtpeUU3MHFVQnNHZ0ljbUYw?= =?utf-8?B?cW9RdHpWZzRCSjZDTm1tU0p5ODlGRzNtdTl4dGEvTEJQOXIvNE94SFRnUGsy?= =?utf-8?Q?SnHpvYE5CJg=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0147;6:igLQyqea+rUKXU24Hp1MNU8FM0zLFIIPnaQljX+1boGqNo8wOc6eGqKFquLF5YFHDNHP4fajuPqlOdtgqRRMrK49eZ7RXe031EWfNue6CMzRmJNFw+Grmu2Sbj19p0K+B76L64F7P4NBaxGC/sqcEx9xo6vgPogDzrJNTUtnGE2B3X3JqiiMK2DX9g6mA5YOMlZ+bHd/JiSf9cFwhvX8fY9tnhcXzcBOL+Mx8lTLsX7XKGA1YoZ+2kyjw5pDMp1l4Xqh9j/Jk+j6TwDtTgmb/HWxEsp0YEqn0vGT1sL3fecyiezM4/qPItIANwLsROl8Gem8/wIcSO6JBP0ZiKA9pan0qzhBZZWzJoXuxELj4sQ=;5:oTgWM3OBoyPQCKU8fCs0jCeDeIujO3wqhzlrCsqW+NyuInaB2EL5MLJTQHKmW9VrRGYsrjbsYQpOcHCn8A+kt1T20+9/MQ8WFYoKp0jQyF63i8mDINJ6vVW4pysGpfLnnO0WNWNI/qd4cdMRu8TIrah8q7DSmRQPV0no2Kdq3u8=;24:HZynqbbIrGQd1f583BLJA0alYQv6dXkVyRfmeXBVin442lFz77ZU8qGmM4PE4lcdUqO1SI9Nas4qJgEy3N40V0uNgPyAAZGmt2b7Mo8q7Qg=;7:4fnYUzj2jml3nLjub84VDdIwzAafrWb8UB+ELZKHErthv3VYL19rTxBU+9LuCeIrAKQogwlzRGFHs+SrCjx2Vp4MfSXxBQTjk/LdBatj43a6NcvOlbrAjn6MdpNFRAjLtaE7R7pUVbWtmD3pXUTVHy+U7qyN1l1e05HKx2fsMVdyNG35QcWFt2jMaDVywF50yUOyozJvDWJ52Pi6IVL9EOnzghcJ3XMQ39bRrDx9Gp+Ujprek7EG2PBRhKPLMQfJ SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0147;20:xLP2O9pT8V42xvVJ8kG+y24wv1NmKtZOBIIp03eR2HxnhNoXwaifH9WkA7WJqtp1m7t23yhD0QTaEyD0lKt4Wc6C43phGf099fD696VnL2sCsnDZRsnOiMA/bKKJh2zq/+YkWjYUFlBTKtaw4z0gEQqJhYv1pIQZR77lhrpti2+4GjVb3poF2HDBfOvUAglzUy96KUGqaIEdg7KM7gpgi1BRm1wpZlGSXsUc9kmn1sdv1kIuFJKNmRGMxA74yNg7 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Oct 2017 17:49:18.2864 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: af47a86f-d8e0-4a4c-d43c-08d51fbe8bb5 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR12MB0147 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/30/2017 12:21 PM, Borislav Petkov wrote: ... > > Useless forward declarations. > Actually its helpful in other patches. I was trying to avoid making too many code movement in other patches to eliminate the forward declarations. I guess I can fix in v7. >> static struct psp_device *psp_alloc_struct(struct sp_device *sp) >> { >> struct device *dev = sp->dev; > > ... > >> +static int sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > > You can use the "__" prefix to denote that it is a lower-level helper: > > __sev_do_cmd > __sev_do_cmd_locked > > Ditto for the other locked functions. noted > ... > >> +static int sev_platform_init_locked(struct sev_data_init *data, int *error) >> +{ >> + struct psp_device *psp = psp_master; >> + struct sev_data_init *input = NULL; >> + int rc = 0; >> + >> + if (!psp) >> + return -ENODEV; >> + >> + if (psp->sev_state == SEV_STATE_INIT) >> + return 0; >> + >> + if (!data) { >> + input = kzalloc(sizeof(*input), GFP_KERNEL); >> + if (!input) >> + return -ENOMEM; >> + >> + data = input; >> + } > > You can do the allocation in the enclosing function, outside of the > critical region so that you can keep it shorter. > > Or even better: if you're going to synchronize the commands with a > mutex, you can define a static struct sev_data_init input in this file > which you always hand in and then you can save yourself the kmalloc > calls. > If the buffer is allocated on the stack then there is no guarantee that __pa() will gives us a valid physical address. IIRC, when CONFIG_VMAP_STACK=y then stack space is mapped similar to vmalloc'd storage and __pa() will not work. Since we need to pass the physical address to PSP hence variable allocated on the stack will not work. I can certainly move the allocation outside, but then it may increase the code size in other functions. If its not a big deal then I would prefer to keep what we have. ... >> + >> +int sev_platform_shutdown(int *error) >> +{ >> + if (error) >> + *error = 0; >> + >> + return 0; >> +} > > I'm guessing that that's just bare-bones and it will get filled up in > the next patches. Otherwise it looks pretty useless. > Well, we are not expanding in other patches. I was also debating on what to do with this function. Since we need sev_platform_init() hence it made sense to add sev_platform_shutdown() as well. If we add the function then I wanted to make sure that we set the *error = SUCCESS so that caller knows that function succeeded. > If it is just to block the user from sending SHUTDOWN to the PSP > master, just do that in the ioctl directly - no need to call some empty > functions. > The function is not used by userspace ioctl, its used by kvm drv when it launch/terminates the SEV guest. -Brijesh