[fixing Christoffer's email address]
Thanks for looping us in.
On 04/05/18 10:11, Christian Borntraeger wrote:
> you should also add the arm/mips/power maintainers so that they can review their
> struct kvm_arch if any of the content is passed directly to the hardware (e.g. with
> __pa). A quick looks seems to indicate that there is no issue.
Well, there is a small issue, at least on arm64. Only VAs that are part
of the linear mapping can easily be mapped at EL2 (so that we can play
our kern_to_hyp_va trick on ARMv8.0 cores), and the kvm structure is
assumed to be physically contiguous.
Switch to a vmalloc'ed structure breaks these two requirements since, as
you noticed it, the kvm struct is more than a single page.
I'd suggest you let each architecture buy into this, or provide a
kmalloc-based kvm_arch_{alloc,free}_vm for architectures that cannot use
the vmalloc area for this. On arm64, we could use vmalloc on VHE-capable
cores, and stick to kmalloc for the rest.
>
> On 05/03/2018 07:59 PM, Marc Orr wrote:
>> The kvm struct is (currently) tens of kilo-bytes, which turns out to be a
>> large amount of memory to allocate contiguously via kzalloc. Thus, this
>> patch changes the kzalloc to a vzalloc, so that the memory allocation is
>> less likely to fail in resource-constrained environments.
>>
>> Signed-off-by: Marc Orr <marcorr@xxxxxxxxxx>
>
> s390 parts seem to work
>
> Acked-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>
>> Change-Id: Ide392e51991142d1325e06351231ac685e11d820
>
> what is that?
>
>
>> ---
>> arch/x86/kvm/svm.c | 4 ++--
>> arch/x86/kvm/vmx.c | 4 ++--
>> include/linux/kvm_host.h | 5 +++--
>> 3 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index b58787daf9f8..f990fda1b2f6 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -1838,13 +1838,13 @@ static void __unregister_enc_region_locked(struct kvm *kvm,
>>
>> static struct kvm *svm_vm_alloc(void)
>> {
>> - struct kvm_svm *kvm_svm = kzalloc(sizeof(struct kvm_svm), GFP_KERNEL);
>> + struct kvm_svm *kvm_svm = vzalloc(sizeof(struct kvm_svm));
>> return &kvm_svm->kvm;
>> }
>>
>> static void svm_vm_free(struct kvm *kvm)
>> {
>> - kfree(to_kvm_svm(kvm));
>> + vfree(to_kvm_svm(kvm));
>> }
>>
>> static void sev_vm_destroy(struct kvm *kvm)
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index aafcc9881e88..26ab153a293f 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9949,13 +9949,13 @@ STACK_FRAME_NON_STANDARD(vmx_vcpu_run);
>>
>> static struct kvm *vmx_vm_alloc(void)
>> {
>> - struct kvm_vmx *kvm_vmx = kzalloc(sizeof(struct kvm_vmx), GFP_KERNEL);
>> + struct kvm_vmx *kvm_vmx = vzalloc(sizeof(struct kvm_vmx));
>> return &kvm_vmx->kvm;
>> }
>>
>> static void vmx_vm_free(struct kvm *kvm)
>> {
>> - kfree(to_kvm_vmx(kvm));
>> + vfree(to_kvm_vmx(kvm));
>> }
>>
>> static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 6930c63126c7..cf7b6a3a4197 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -19,6 +19,7 @@
>> #include <linux/preempt.h>
>> #include <linux/msi.h>
>> #include <linux/slab.h>
>> +#include <linux/vmalloc.h>
>> #include <linux/rcupdate.h>
>> #include <linux/ratelimit.h>
>> #include <linux/err.h>
>> @@ -810,12 +811,12 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
>> #ifndef __KVM_HAVE_ARCH_VM_ALLOC
>> static inline struct kvm *kvm_arch_alloc_vm(void)
>> {
>> - return kzalloc(sizeof(struct kvm), GFP_KERNEL);
>> + return vzalloc(sizeof(struct kvm));
>> }
>>
>> static inline void kvm_arch_free_vm(struct kvm *kvm)
>> {
>> - kfree(kvm);
>> + vfree(kvm);
>> }
>> #endif
>>
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
