Re: [RFC PATCH v1 21/28] KVM: introduce KVM_SEV_ISSUE_CMD ioctl

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

Hi Paolo,

On 10/17/2016 03:14 PM, Paolo Bonzini wrote:
>> I am not sure if I fully understand this feedback. Let me summaries what
>> we have right now.
>>
>> At highest level SEV key management commands are divided into two sections:
>>
>> - platform  management : commands used during platform provisioning. PSP
>> drv provides ioctl's for these commands. Qemu will not use these
>> ioctl's, i believe these ioctl will be used by other tools.
>>
>> - guest management: command used during guest life cycle. PSP drv
>> exports various function and KVM drv calls these function when it
>> receives the SEV_ISSUE_CMD ioctl from qemu.
>>
>> If I understanding correctly then you are recommending that instead of
>> exporting various functions from PSP drv we should expose one function
>> for the all the guest command handling (something like this).
>
> My understanding is that a user could exhaust the ASIDs for encrypted
> VMs if it was allowed to start an arbitrary number of KVM guests.  So
> we would need some kind of control.  Is this correct?
>

Yes, there is limited number of ASIDs for encrypted VMs. Do we need to 
pass the psp_fd into SEV_ISSUE_CMD ioctl or can we handle it from Qemu 
itself ? e.g when user asks to transition a guest into SEV-enabled mode 
then before calling LAUNCH_START Qemu tries to open /dev/psp device. If 
open() returns success then we know user has permission to communicate 
with PSP firmware. Please let me know if I am missing something.

> If so, does /dev/psp provide any functionality that you believe is
> dangerous for the KVM userspace (which runs in a very confined
> environment)?  Is this functionality blocked through capability
> checks?
>

I do not see /dev/psp providing anything which would be dangerous to KVM 
userspace. It should be safe to access /dev/psp into KVM userspace.

> Thanks,
>
> Paolo
>
>
>> int psp_issue_cmd_external_user(struct file *filep,
>> 			    	int cmd, unsigned long addr,
>> 			    	int *psp_ret)
>> {
>> 	/* here we check to ensure that file->f_ops is a valid
>> 	 * psp instance.
>>           */
>> 	if (filep->f_ops != &psp_fops)
>> 		return -EINVAL;
>>
>> 	/* handle the command */
>> 	return psp_issue_cmd (cmd, addr, timeout, psp_ret);
>> }
>>
>> In KVM driver use something like this to invoke the PSP command handler.
>>
>> int kvm_sev_psp_cmd (struct kvm_sev_issue_cmd *input,
>> 		     unsigned long data)
>> {
>> 	int ret;
>> 	struct fd f;
>>
>> 	f = fdget(input->psp_fd);
>> 	if (!f.file)
>> 		return -EBADF;
>> 	....
>>
>> 	psp_issue_cmd_external_user(f.file, input->cmd,
>> 				    data, &input->psp_ret);
>> 	....
>> }
>>
>> Please let me know if I understood this correctly.
>>
>>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>>> ---
>>>>  arch/x86/include/asm/kvm_host.h |    3 +
>>>>  arch/x86/kvm/x86.c              |   13 ++++
>>>>  include/uapi/linux/kvm.h        |  125
>>>>  +++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 141 insertions(+)
>>>>
>>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>