Re: [PATCH v2] KVM/PPC Patch for KVM issue in real mode

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Balbir Singh <bsingharora@gmail.com> writes:

> Some KVM functions for book3s_hv are called in real mode.
> In real mode the top 4 bits of the address space are ignored,
> hence an address beginning with 0xc0000000+offset is the
> same as 0xd0000000+offset. The issue was observed when
> a kvm memslot resolution lead to random values when
> access from kvmppc_h_enter(). The issue is hit if the
> KVM host is running with a page size of 4K, since
> kvzalloc() looks at size < PAGE_SIZE. On systems with
> 64K the issue is not observed easily, it largely depends
> on the size of the structure being allocated.
>
> The proposed fix moves all KVM allocations for book3s_hv
> to kzalloc() until all structures used in real mode are
> audited. For safety allocations are moved to kmalloc
> space. The impact is a large allocation on systems with
> 4K page size.

We did such access using *real_vmalloc_addr(void *x). So you are
suggesting here is we don't do that for all code path ?

Do you have a stack dump for which you identified the issue ?

>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
>  Changelog v2:
>    Fix build failures reported by the kbuild test robot
>    http://www.spinics.net/lists/kvm/msg141727.html
>
>  arch/powerpc/include/asm/kvm_host.h | 19 +++++++++++++++++++
>  include/linux/kvm_host.h            | 11 +++++++++++
>  virt/kvm/kvm_main.c                 |  2 +-
>  3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index f15713a..53f5172 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -734,6 +734,25 @@ struct kvm_vcpu_arch {
>  #define __KVM_HAVE_ARCH_WQP
>  #define __KVM_HAVE_CREATE_DEVICE
>  
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> +#define __KVM_HAVE_ARCH_VZALLOC_OVERRIDE

do we need that OVERRIDE ? We usually have HAVE_ARCH_KVM_VZALLOC
or just say #ifndef kvm_arch_vzalloc ?


> +
> +/*
> + * KVM uses some of these data structures -- the ones
> + * from kvzalloc() in real mode. If the data structure
> + * happens to come from a vmalloc'd range then its access
> + * in real mode will lead to problems due to the aliasing
> + * issue - (top 4 bits are ignore).
> + * A 0xd000+offset will point to a 0xc000+offset in realmode
> + * Hence we want our data structures from come from kmalloc'd
> + * regions, so that we don't have these aliasing issues
> + */
> +static inline void *kvm_arch_vzalloc(unsigned long size)
> +{
> +	return kzalloc(size, GFP_KERNEL);
> +}
> +#endif


....
-aneesh