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

[Borislav Petkov]

On Wed, Oct 11, 2017 at 03:10:49PM -0500, Brijesh Singh wrote:
> 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"

Ok, I see it. So what you actually wanna do is:

struct sev_data_status {
        u8 api_major;                           /* Out */
        u8 api_minor;                           /* Out */
        u8 state;                               /* Out */
        u8 flags;                               /* Out */
        u32 config;                             /* Out */
        u32 guest_count;                        /* Out */
} __packed;

as this is exactly what the firwmare gives you. Theoretically, you
could've also done:

struct sev_data_status {
	u64 first_qword;
	u32 second_dword;
};

but you have the fields mostly defined already and that would be too
confusing.

What I mean is, once you've gotten the command buffer, then you can pick
fields apart in software.

	owner	  = status.flags & 1;
	config_es = status.config & 1;
	build	  = (status.config >> 24) & 0xff;

This way, if new fields get added, you don't have to change the struct
definitions - *especially* if they're visible to userspace - and users
of that struct can be extended to understand the new fields.

And before you copy the struct to userspace, you can simply clear out
the reserved fields as nothing should rely on them having a particular
value, because, well, they're reserved, doh.

Makes sense?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--