From mboxrd@z Thu Jan  1 00:00:00 1970
From: Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [RFC Part2 PATCH v3 13/26] KVM: SVM: Add KVM_SEV_INIT command
Date: Wed, 13 Sep 2017 11:23:26 -0500
Message-ID: <ade84a14-964f-8560-38cd-cc7f7878fa33@amd.com>
References: <20170724200303.12197-1-brijesh.singh@amd.com>
 <20170724200303.12197-14-brijesh.singh@amd.com>
 <20170913150636.fcjhbg7wdf2whmy2@pd.tnic>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: brijesh.singh@amd.com, linux-kernel@vger.kernel.org,
        x86@kernel.org, kvm@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Joerg Roedel <joro@8bytes.org>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?XCJSYWRpbSBLcsSNbcOhxZlcIg==?= <rkrcmar@redhat.com>,
        Tom Lendacky <thomas.lendacky@amd.com>
To: Borislav Petkov <bp@suse.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-sn1nam02on0061.outbound.protection.outlook.com ([104.47.36.61]:22946
        "EHLO NAM02-SN1-obe.outbound.protection.outlook.com"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1751607AbdIMQXe (ORCPT <rfc822;kvm@vger.kernel.org>);
        Wed, 13 Sep 2017 12:23:34 -0400
In-Reply-To: <20170913150636.fcjhbg7wdf2whmy2@pd.tnic>
Content-Language: en-US
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Boris,

thanks for the detail review.

On 09/13/2017 10:06 AM, Borislav Petkov wrote:
...

>> +static int sev_platform_get_state(int *state, int *error)
>> +{
>> +	int ret;
>> +	struct sev_data_status *data;
>> +
>> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> 
> It's a bit silly to do the allocation only for the duration of
> sev_platform_status() - just allocate "data" on the stack.
> 

I am okay with moving it on the stack but just to give context why
I went in this way. The physical address of data is given to the
device (in this case SEV FW). I was not sure if its okay to pass the
stack address to the device. Additionally, the FW spec requires us to
zero all the fields -- so we need to memset if we allocate it on the
stack.