On Fri, Oct 06, 2017 at 08:06:02PM -0500, Brijesh Singh wrote:
> The SEV_PLATFORM_STATUS command can be used by the platform owner to
> get the current status of the platform. The command is defined in
> SEV spec section 5.5.
>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: "Radim Krčmář" <rkrcmar@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Cc: Gary Hook <gary.hook@xxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: linux-crypto@xxxxxxxxxxxxxxx
> Cc: kvm@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> ---
> drivers/crypto/ccp/psp-dev.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
...
> @@ -198,6 +228,10 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)
> ret = sev_handle_cmd(SEV_CMD_FACTORY_RESET, 0, &input.error);
> break;
> }
> + case SEV_PLATFORM_STATUS: {
> + ret = sev_ioctl_platform_status(&input);
> + break;
> + }
What's with the curly brackets around the case: statements?
Anyway, here are some more improvements:
* you can get rid of the struct copying into out and the bitfields by
doing something like this:
ret = sev_do_cmd(SEV_CMD_PLATFORM_STATUS, data, &argp->error);
if (ret)
goto e_free;
/* Clear out reserved fields: */
data->owner &= BIT(0);
data->config &= BIT(0);
I'm not sure those are the ones you need to clear but you get
the idea - you simply poke holes in the reserved fields before
copying to userspace. If you need a more sophisticated mask, use
GENMASK/GENMASK_ULL.
And then you don't need struct sev_user_data_status and
simply remove the bitfields too.
* Also, a function should have a verb in the name, thus
sev_ioctl_do_platform_status().
---
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index d668045956cb..1479db533da0 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -176,9 +176,8 @@ static int sev_do_cmd(int cmd, void *data, int *psp_ret)
return ret;
}
-static int sev_ioctl_platform_status(struct sev_issue_cmd *argp)
+static int sev_ioctl_do_platform_status(struct sev_issue_cmd *argp)
{
- struct sev_user_data_status out;
struct sev_data_status *data;
int ret;
@@ -186,19 +185,15 @@ static int sev_ioctl_platform_status(struct sev_issue_cmd *argp)
if (!data)
return -ENOMEM;
- ret = sev_handle_cmd(SEV_CMD_PLATFORM_STATUS, data, &argp->error);
+ ret = sev_do_cmd(SEV_CMD_PLATFORM_STATUS, data, &argp->error);
if (ret)
goto e_free;
- out.api_major = data->api_major;
- out.api_minor = data->api_minor;
- out.state = data->state;
- out.owner = data->owner;
- out.config = data->config;
- out.build = data->build;
- out.guest_count = data->guest_count;
- if (copy_to_user((void __user *)(uintptr_t) argp->data,
- &out, sizeof(struct sev_user_data_status)))
+ /* Clear out reserved fields: */
+ data->owner &= BIT(0);
+ data->config &= BIT(0);
+
+ if (copy_to_user((void __user *)argp->data, data, sizeof(*data)))
ret = -EFAULT;
e_free:
@@ -226,10 +221,10 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)
ret = sev_do_cmd(SEV_CMD_FACTORY_RESET, 0, &input.error);
break;
- case SEV_PLATFORM_STATUS: {
- ret = sev_ioctl_platform_status(&input);
+ case SEV_PLATFORM_STATUS:
+ ret = sev_ioctl_do_platform_status(&input);
break;
- }
+
default:
ret = -EINVAL;
goto out;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 10b843cce75f..223942ba3e7e 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -144,11 +144,9 @@ struct sev_data_status {
u8 api_major; /* Out */
u8 api_minor; /* Out */
u8 state; /* Out */
- u8 owner : 1; /* Out */
- u8 reserved1 : 7;
- u32 config : 1; /* Out */
- u32 reserved2 : 23;
- u32 build : 8; /* Out */
+ u8 owner; /* Out */
+ u32 config; /* Out */
+ u32 build; /* Out */
u32 guest_count; /* Out */
} __packed;
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
