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

[Brijesh Singh]

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.