Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

[Brijesh Singh <brijesh.singh@amd.com>]

On 10/11/2017 03:04 PM, Borislav Petkov wrote:
> On Wed, Oct 11, 2017 at 02:49:55PM -0500, Brijesh Singh wrote:
>> This is OK for now. But in future if FW steals another bit from reserved1
>> field to expose a new flag then 'owner' name will no longer be valid. If you
>> don't to use bit field then we have to use some generic name instead of
>> naming the field as 'owner'. Please note that its not just userspace, KVM
>> driver also uses the same fields and it may also need to know which bit
>> position to use.
> 
> So what is this field called in the spec?
> 

See Section 5.5.2 [1]

[1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf

The bit0 is named as 'owner' and bits 7:1 is left blank (no name is given).


>> This is a tricky one. The 32-bit are packed as:
>>
>> BIT0 - config.es
>> BIT1-23: reserved
>> BIT24-31: build
> 
> Is that what the firmware gives?
> 

Yes

> Then it is easy:
> 
> 	<firmware_field_name> &= GENMASK(23, 1);
> 
> and then userspace can pick apart bit 0 and bit24-31.
> 

The bit0 is named as config.es and bit1-23 is left blank and bit31-24 is 
named as build.

The current 'struct sev_data_status' matches with the firmware names and 
the bit fields. Only thing I did was the fields with no name is called 
as "reservedX"


> Just use one raw struct which the firmware gives you and the rest is
> done by the sw. Like clearing reserved fields before copying to user.
> 
> You don't want to be updating that struct layout later: think of old
> qemu with new kernel and all those different configurations.
>