* [PATCH] KVM: X86: Add missing KVM_AMD dependency
@ 2018-10-05 18:46 Guenter Roeck
2018-10-05 20:41 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2018-10-05 18:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Guenter Roeck, Brijesh Singh, Borislav Petkov
Building an image with KVM_AMD=y, CRYPTO_DEV_SP_PSP=y, and
CRYPTO_DEV_CCP_DD=m fails with the following error messages.
arch/x86/kvm/svm.c:6287: undefined reference to `sev_issue_cmd_external_user'
arch/x86/kvm/svm.o: In function `sev_unbind_asid':
arch/x86/kvm/svm.c:1747: undefined reference to `sev_guest_deactivate'
arch/x86/kvm/svm.c:1750: undefined reference to `sev_guest_df_flush'
arch/x86/kvm/svm.c:1759: undefined reference to `sev_guest_decommission'
Analysis shows that commit 59414c9892208 ("KVM: SVM: Add support for
KVM_SEV_LAUNCH_START command") added a dependency of KVM_AMD on
CRYPTO_DEV_CCP_DD if CRYPTO_DEV_SP_PSP is enabled: If CRYPTO_DEV_CCP_DD
is built as module, KVM_AMD must be built as module as well.
Fixes: 59414c9892208 ("KVM: SVM: Add support for KVM_SEV_LAUNCH_START command")
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
arch/x86/kvm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 1bbec387d289..a8eff6910ea3 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -74,6 +74,7 @@ config KVM_INTEL
config KVM_AMD
tristate "KVM for AMD processors support"
depends on KVM
+ depends on !CRYPTO_DEV_SP_PSP || (CRYPTO_DEV_SP_PSP && CRYPTO_DEV_CCP_DD)
---help---
Provides support for KVM on AMD processors equipped with the AMD-V
(SVM) extensions.
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-05 18:46 [PATCH] KVM: X86: Add missing KVM_AMD dependency Guenter Roeck
@ 2018-10-05 20:41 ` Paolo Bonzini
2018-10-05 22:03 ` Guenter Roeck
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2018-10-05 20:41 UTC (permalink / raw)
To: Guenter Roeck
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Brijesh Singh, Borislav Petkov
On 05/10/2018 20:46, Guenter Roeck wrote:
> Analysis shows that commit 59414c9892208 ("KVM: SVM: Add support for
> KVM_SEV_LAUNCH_START command") added a dependency of KVM_AMD on
> CRYPTO_DEV_CCP_DD if CRYPTO_DEV_SP_PSP is enabled: If CRYPTO_DEV_CCP_DD
> is built as module, KVM_AMD must be built as module as well.
>
> Fixes: 59414c9892208 ("KVM: SVM: Add support for KVM_SEV_LAUNCH_START command")
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Borislav Petkov <bp@suse.de>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
This should be handled by
config KVM_AMD_SEV
def_bool y
bool "AMD Secure Encrypted Virtualization (SEV) support"
depends on KVM_AMD && X86_64
depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
---help---
Provides support for launching Encrypted VMs on AMD processors.
Maybe this works as well? I haven't tested it yet:
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 89c4c5aa15f1..55f10b17d044 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -441,9 +441,13 @@ static inline bool svm_sev_enabled(void)
static inline bool sev_guest(struct kvm *kvm)
{
+#ifdef CONFIG_KVM_AMD_SEV
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
return sev->active;
+#else
+ return false;
+#endif
}
static inline int sev_get_asid(struct kvm *kvm)
Thanks,
Paolo
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-05 20:41 ` Paolo Bonzini
@ 2018-10-05 22:03 ` Guenter Roeck
2018-10-05 22:18 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2018-10-05 22:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Brijesh Singh, Borislav Petkov
On Fri, Oct 05, 2018 at 10:41:55PM +0200, Paolo Bonzini wrote:
> On 05/10/2018 20:46, Guenter Roeck wrote:
> > Analysis shows that commit 59414c9892208 ("KVM: SVM: Add support for
> > KVM_SEV_LAUNCH_START command") added a dependency of KVM_AMD on
> > CRYPTO_DEV_CCP_DD if CRYPTO_DEV_SP_PSP is enabled: If CRYPTO_DEV_CCP_DD
> > is built as module, KVM_AMD must be built as module as well.
> >
> > Fixes: 59414c9892208 ("KVM: SVM: Add support for KVM_SEV_LAUNCH_START command")
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Cc: Borislav Petkov <bp@suse.de>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> This should be handled by
>
> config KVM_AMD_SEV
> def_bool y
> bool "AMD Secure Encrypted Virtualization (SEV) support"
> depends on KVM_AMD && X86_64
> depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
> ---help---
> Provides support for launching Encrypted VMs on AMD processors.
>
Unfortunately it doesn't. It disables KVM_AMD_SEV, but that doesn't prevent
the calls.
> Maybe this works as well? I haven't tested it yet:
>
I am sure there are many possible solutions. I would personally prefer one
that enforces KVM_AMD=m with CRYPTO_DEV_CCP_DD=m, but that is just me.
Thanks,
Guenter
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 89c4c5aa15f1..55f10b17d044 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -441,9 +441,13 @@ static inline bool svm_sev_enabled(void)
>
> static inline bool sev_guest(struct kvm *kvm)
> {
> +#ifdef CONFIG_KVM_AMD_SEV
> struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
>
> return sev->active;
> +#else
> + return false;
> +#endif
> }
>
> static inline int sev_get_asid(struct kvm *kvm)
>
> Thanks,
>
> Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-05 22:03 ` Guenter Roeck
@ 2018-10-05 22:18 ` Paolo Bonzini
2018-10-06 20:43 ` Guenter Roeck
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2018-10-05 22:18 UTC (permalink / raw)
To: Guenter Roeck
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Brijesh Singh, Borislav Petkov
On 06/10/2018 00:03, Guenter Roeck wrote:
>> This should be handled by
>>
>> config KVM_AMD_SEV
>> def_bool y
>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>> depends on KVM_AMD && X86_64
>> depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
>> ---help---
>> Provides support for launching Encrypted VMs on AMD processors.
>>
> Unfortunately it doesn't. It disables KVM_AMD_SEV, but that doesn't prevent
> the calls.
Yes, exactly - that's why I mentioned the sev_guest patch that should
cull all the SEV code from a !KVM_AMD_SEV build.
>> Maybe this works as well? I haven't tested it yet:
>>
> I am sure there are many possible solutions. I would personally prefer one
> that enforces KVM_AMD=m with CRYPTO_DEV_CCP_DD=m, but that is just me.
Well, KVM_AMD=y is a relatively unusual choice to begin with. The
question is whether then you want to disable this choice completely when
CRYPTO_DEV_CCP_DD=m, or just disable SEV.
My patch is a good idea anyway, if I may say so :), because it culls a
lot of code from a !KVM_AMD_SEV build. But if it is not enough, we
certainly have to do something else about the failure you're reporting.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-05 22:18 ` Paolo Bonzini
@ 2018-10-06 20:43 ` Guenter Roeck
2018-10-08 11:27 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2018-10-06 20:43 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Brijesh Singh, Borislav Petkov
Hi Paolo,
On 10/05/2018 03:18 PM, Paolo Bonzini wrote:
> On 06/10/2018 00:03, Guenter Roeck wrote:
>>> This should be handled by
>>>
>>> config KVM_AMD_SEV
>>> def_bool y
>>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>>> depends on KVM_AMD && X86_64
>>> depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
>>> ---help---
>>> Provides support for launching Encrypted VMs on AMD processors.
>>>
>> Unfortunately it doesn't. It disables KVM_AMD_SEV, but that doesn't prevent
>> the calls.
>
> Yes, exactly - that's why I mentioned the sev_guest patch that should
> cull all the SEV code from a !KVM_AMD_SEV build.
>
>>> Maybe this works as well? I haven't tested it yet:
>>>
>> I am sure there are many possible solutions. I would personally prefer one
>> that enforces KVM_AMD=m with CRYPTO_DEV_CCP_DD=m, but that is just me.
>
> Well, KVM_AMD=y is a relatively unusual choice to begin with. The
It is common enough that we are not the only ones affected. Also, even a
"relatively unusual choice" should, in my opinion, not result in a build
error. Never mind, I'll just apply the suggested workaround and configure
CRYPTO_DEV_CCP_DD=y. I need to do that anyway, after all, if I want to keep
KVM_AMD=y.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-06 20:43 ` Guenter Roeck
@ 2018-10-08 11:27 ` Paolo Bonzini
2018-10-08 14:52 ` Singh, Brijesh
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2018-10-08 11:27 UTC (permalink / raw)
To: Guenter Roeck
Cc: Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Brijesh Singh, Borislav Petkov
On 06/10/2018 22:43, Guenter Roeck wrote:
>>
>>>> Maybe this works as well? I haven't tested it yet:
>>>>
>>> I am sure there are many possible solutions. I would personally
>>> prefer one
>>> that enforces KVM_AMD=m with CRYPTO_DEV_CCP_DD=m, but that is just me.
>>
>> Well, KVM_AMD=y is a relatively unusual choice to begin with. The
>
> It is common enough that we are not the only ones affected. Also, even a
> "relatively unusual choice" should, in my opinion, not result in a build
> error.
Of course not! The question is whether to solve it by disabling
KVM_AMD_SEV (which is what the current code attempts to do, and my patch
should fix that) or KVM_AMD (your patch).
Paolo
> Never mind, I'll just apply the suggested workaround and configure
> CRYPTO_DEV_CCP_DD=y. I need to do that anyway, after all, if I want to keep
> KVM_AMD=y.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-08 11:27 ` Paolo Bonzini
@ 2018-10-08 14:52 ` Singh, Brijesh
2018-10-08 17:32 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: Singh, Brijesh @ 2018-10-08 14:52 UTC (permalink / raw)
To: Paolo Bonzini, Guenter Roeck
Cc: Singh, Brijesh, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel, Borislav Petkov
On 10/08/2018 06:27 AM, Paolo Bonzini wrote:
> On 06/10/2018 22:43, Guenter Roeck wrote:
>>>
>>>>> Maybe this works as well? I haven't tested it yet:
>>>>>
>>>> I am sure there are many possible solutions. I would personally
>>>> prefer one
>>>> that enforces KVM_AMD=m with CRYPTO_DEV_CCP_DD=m, but that is just me.
>>>
>>> Well, KVM_AMD=y is a relatively unusual choice to begin with. The
>>
>> It is common enough that we are not the only ones affected. Also, even a
>> "relatively unusual choice" should, in my opinion, not result in a build
>> error.
>
> Of course not! The question is whether to solve it by disabling
> KVM_AMD_SEV (which is what the current code attempts to do, and my patch
> should fix that) or KVM_AMD (your patch).
IMHO, Paolo's patch make sense; it removes all the SEV specific code
when KVM_AMD_SEV=n. It saves ~4K in text section.
Paolo,
Does it make sense to move all the SEV specific code in svm-sev.c ?
I am looking to add SEV migration support very soon, and can see
myself adding more SEV command handling which will grow svm.c further.
-Brijesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-08 14:52 ` Singh, Brijesh
@ 2018-10-08 17:32 ` Borislav Petkov
2018-10-09 16:26 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2018-10-08 17:32 UTC (permalink / raw)
To: Singh, Brijesh
Cc: Paolo Bonzini, Guenter Roeck, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel
On Mon, Oct 08, 2018 at 02:52:46PM +0000, Singh, Brijesh wrote:
> Does it make sense to move all the SEV specific code in svm-sev.c ?
> I am looking to add SEV migration support very soon, and can see
> myself adding more SEV command handling which will grow svm.c further.
Amen to that - svm.c is pretty huge already. And if you end up moving
facilities, you can simply call it sev.c
Just my 2¢.
Thx.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] KVM: X86: Add missing KVM_AMD dependency
2018-10-08 17:32 ` Borislav Petkov
@ 2018-10-09 16:26 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2018-10-09 16:26 UTC (permalink / raw)
To: Borislav Petkov, Singh, Brijesh
Cc: Guenter Roeck, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, kvm,
linux-kernel
On 08/10/2018 19:32, Borislav Petkov wrote:
> On Mon, Oct 08, 2018 at 02:52:46PM +0000, Singh, Brijesh wrote:
>> Does it make sense to move all the SEV specific code in svm-sev.c ?
>> I am looking to add SEV migration support very soon, and can see
>> myself adding more SEV command handling which will grow svm.c further.
>
> Amen to that - svm.c is pretty huge already. And if you end up moving
> facilities, you can simply call it sev.c
Creating arch/x86/kvm/{mmu,vmx,svm}/ has been on my todo list for a
while. If you would like to split the SEV bits, that would be a good start.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-09 16:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 18:46 [PATCH] KVM: X86: Add missing KVM_AMD dependency Guenter Roeck
2018-10-05 20:41 ` Paolo Bonzini
2018-10-05 22:03 ` Guenter Roeck
2018-10-05 22:18 ` Paolo Bonzini
2018-10-06 20:43 ` Guenter Roeck
2018-10-08 11:27 ` Paolo Bonzini
2018-10-08 14:52 ` Singh, Brijesh
2018-10-08 17:32 ` Borislav Petkov
2018-10-09 16:26 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).