Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 07/04/21 00:49, Sean Christopherson wrote:
This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd
command buffers by copying _all_ incoming data pointers to an internal
buffer before sending the command to the PSP. The SEV driver and KVM are
then converted to use the stack for all command buffers.
Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere
near enough about the PSP to give it the right input.
v2:
- Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs
sharing SEV context").
- Unconditionally copy @data to the internal buffer. [Christophe, Brijesh]
- Allocate a full page for the buffer. [Brijesh]
- Drop one set of the "!"s. [Christophe]
- Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary
patch (definitely feel free to drop the patch if it's not worth
backporting). [Christophe]
- s/intput/input/. [Tom]
- Add a patch to free "sev" if init fails. This is not strictly
necessary (I think; I suck horribly when it comes to the driver
framework). But it felt wrong to not free cmd_buf on failure, and
even more wrong to free cmd_buf but not sev.
v1:
- https://lkml.kernel.org/r/20210402233702.3291792-1-sea...@google.com
Sean Christopherson (8):
crypto: ccp: Free SEV device if SEV init fails
crypto: ccp: Detect and reject "invalid" addresses destined for PSP
crypto: ccp: Reject SEV commands with mismatching command buffer
crypto: ccp: Play nice with vmalloc'd memory for SEV command structs
crypto: ccp: Use the stack for small SEV command buffers
crypto: ccp: Use the stack and common buffer for status commands
crypto: ccp: Use the stack and common buffer for INIT command
KVM: SVM: Allocate SEV command structures on local stack
arch/x86/kvm/svm/sev.c | 262 +--
drivers/crypto/ccp/sev-dev.c | 197 +-
drivers/crypto/ccp/sev-dev.h | 4 +-
3 files changed, 196 insertions(+), 267 deletions(-)
Queued, thanks.
Paolo
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On Thu, Apr 15, 2021 at 01:15:59PM -0500, Tom Lendacky wrote:
> On 4/15/21 11:09 AM, Paolo Bonzini wrote:
> > On 07/04/21 20:00, Tom Lendacky wrote:
> >> For the series:
> >>
> >> Acked-by: Tom Lendacky
> >
> > Shall I take this as a request (or permission, whatever :)) to merge it
> > through the KVM tree?
>
> Adding Herbert. Here's a link to the series:
>
> https://lore.kernel.org/kvm/88eef561-6fd8-a495-0d60-ff688070c...@redhat.com/T/#m2bbdd12452970d3bd7d0b1464c22bf2f0227a9f1
>
> I'm not sure how you typically do the cross-tree stuff. Patch 8 has a
> requirement on patches 1-7. The arch/x86/kvm/svm/sev.c file tends to have
> more activity/changes than drivers/crypto/ccp/sev-dev.{c,h}, so it would
> make sense to take it through the KVM tree. But I think you need to verify
> that with Herbert.
I don't mind at all. Paolo you can take this through your tree.
Thanks,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 4/15/21 11:09 AM, Paolo Bonzini wrote:
> On 07/04/21 20:00, Tom Lendacky wrote:
>> For the series:
>>
>> Acked-by: Tom Lendacky
>
> Shall I take this as a request (or permission, whatever :)) to merge it
> through the KVM tree?
Adding Herbert. Here's a link to the series:
https://lore.kernel.org/kvm/88eef561-6fd8-a495-0d60-ff688070c...@redhat.com/T/#m2bbdd12452970d3bd7d0b1464c22bf2f0227a9f1
I'm not sure how you typically do the cross-tree stuff. Patch 8 has a
requirement on patches 1-7. The arch/x86/kvm/svm/sev.c file tends to have
more activity/changes than drivers/crypto/ccp/sev-dev.{c,h}, so it would
make sense to take it through the KVM tree. But I think you need to verify
that with Herbert.
Thanks,
Tom
>
> Paolo
>
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 07/04/21 20:00, Tom Lendacky wrote: For the series: Acked-by: Tom Lendacky Shall I take this as a request (or permission, whatever :)) to merge it through the KVM tree? Paolo
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 4/6/21 5:49 PM, Sean Christopherson wrote:
> This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd
> command buffers by copying _all_ incoming data pointers to an internal
> buffer before sending the command to the PSP. The SEV driver and KVM are
> then converted to use the stack for all command buffers.
>
> Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere
> near enough about the PSP to give it the right input.
>
> v2:
> - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs
> sharing SEV context").
> - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh]
> - Allocate a full page for the buffer. [Brijesh]
> - Drop one set of the "!"s. [Christophe]
> - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary
> patch (definitely feel free to drop the patch if it's not worth
> backporting). [Christophe]
> - s/intput/input/. [Tom]
> - Add a patch to free "sev" if init fails. This is not strictly
> necessary (I think; I suck horribly when it comes to the driver
> framework). But it felt wrong to not free cmd_buf on failure, and
> even more wrong to free cmd_buf but not sev.
>
> v1:
> -
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20210402233702.3291792-1-seanjc%40google.comdata=04%7C01%7Cthomas.lendacky%40amd.com%7Cecd38fba67954845323908d8f94e5405%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637533462102772796%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=SUN8Zp%2Fi%2BiHAjMSe%2Fjwvs9JmXg%2FRvi%2B8j01sLDipPg8%3Dreserved=0
>
> Sean Christopherson (8):
> crypto: ccp: Free SEV device if SEV init fails
> crypto: ccp: Detect and reject "invalid" addresses destined for PSP
> crypto: ccp: Reject SEV commands with mismatching command buffer
> crypto: ccp: Play nice with vmalloc'd memory for SEV command structs
> crypto: ccp: Use the stack for small SEV command buffers
> crypto: ccp: Use the stack and common buffer for status commands
> crypto: ccp: Use the stack and common buffer for INIT command
> KVM: SVM: Allocate SEV command structures on local stack
>
> arch/x86/kvm/svm/sev.c | 262 +--
> drivers/crypto/ccp/sev-dev.c | 197 +-
> drivers/crypto/ccp/sev-dev.h | 4 +-
> 3 files changed, 196 insertions(+), 267 deletions(-)
For the series:
Acked-by: Tom Lendacky
>
Re: [PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
On 4/6/21 5:49 PM, Sean Christopherson wrote:
> This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd
> command buffers by copying _all_ incoming data pointers to an internal
> buffer before sending the command to the PSP. The SEV driver and KVM are
> then converted to use the stack for all command buffers.
>
> Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere
> near enough about the PSP to give it the right input.
>
> v2:
> - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs
> sharing SEV context").
> - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh]
> - Allocate a full page for the buffer. [Brijesh]
> - Drop one set of the "!"s. [Christophe]
> - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary
> patch (definitely feel free to drop the patch if it's not worth
> backporting). [Christophe]
> - s/intput/input/. [Tom]
> - Add a patch to free "sev" if init fails. This is not strictly
> necessary (I think; I suck horribly when it comes to the driver
> framework). But it felt wrong to not free cmd_buf on failure, and
> even more wrong to free cmd_buf but not sev.
>
> v1:
> -
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20210402233702.3291792-1-seanjc%40google.comdata=04%7C01%7Cbrijesh.singh%40amd.com%7C051db746fc2048e06acb08d8f94e527b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637533462083069551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=bbNHBXMO1RWh8i4siTYkv4P92Ph5C7SnAZ3uTPsxgvg%3Dreserved=0
>
> Sean Christopherson (8):
> crypto: ccp: Free SEV device if SEV init fails
> crypto: ccp: Detect and reject "invalid" addresses destined for PSP
> crypto: ccp: Reject SEV commands with mismatching command buffer
> crypto: ccp: Play nice with vmalloc'd memory for SEV command structs
> crypto: ccp: Use the stack for small SEV command buffers
> crypto: ccp: Use the stack and common buffer for status commands
> crypto: ccp: Use the stack and common buffer for INIT command
> KVM: SVM: Allocate SEV command structures on local stack
>
> arch/x86/kvm/svm/sev.c | 262 +--
> drivers/crypto/ccp/sev-dev.c | 197 +-
> drivers/crypto/ccp/sev-dev.h | 4 +-
> 3 files changed, 196 insertions(+), 267 deletions(-)
>
Thanks Sean.
Reviewed-by: Brijesh Singh
[PATCH v2 0/8] ccp: KVM: SVM: Use stack for SEV command buffers
This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd
command buffers by copying _all_ incoming data pointers to an internal
buffer before sending the command to the PSP. The SEV driver and KVM are
then converted to use the stack for all command buffers.
Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere
near enough about the PSP to give it the right input.
v2:
- Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs
sharing SEV context").
- Unconditionally copy @data to the internal buffer. [Christophe, Brijesh]
- Allocate a full page for the buffer. [Brijesh]
- Drop one set of the "!"s. [Christophe]
- Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary
patch (definitely feel free to drop the patch if it's not worth
backporting). [Christophe]
- s/intput/input/. [Tom]
- Add a patch to free "sev" if init fails. This is not strictly
necessary (I think; I suck horribly when it comes to the driver
framework). But it felt wrong to not free cmd_buf on failure, and
even more wrong to free cmd_buf but not sev.
v1:
- https://lkml.kernel.org/r/20210402233702.3291792-1-sea...@google.com
Sean Christopherson (8):
crypto: ccp: Free SEV device if SEV init fails
crypto: ccp: Detect and reject "invalid" addresses destined for PSP
crypto: ccp: Reject SEV commands with mismatching command buffer
crypto: ccp: Play nice with vmalloc'd memory for SEV command structs
crypto: ccp: Use the stack for small SEV command buffers
crypto: ccp: Use the stack and common buffer for status commands
crypto: ccp: Use the stack and common buffer for INIT command
KVM: SVM: Allocate SEV command structures on local stack
arch/x86/kvm/svm/sev.c | 262 +--
drivers/crypto/ccp/sev-dev.c | 197 +-
drivers/crypto/ccp/sev-dev.h | 4 +-
3 files changed, 196 insertions(+), 267 deletions(-)
--
2.31.0.208.g409f899ff0-goog
