Re: [PATCH] KVM: x86: (64-bit x86_64 machine) : fix -frame-larger-than warnings/errors.

[Sean Christopherson <seanjc@google.com>]

On Fri, Sep 17, 2021, Ajay Garg wrote:
> From: ajay <ajaygargnsit@gmail.com>
> 
> Issue :
> =======
> 
> In "kvm_hv_flush_tlb" and "kvm_hv_send_ipi" methods, defining
> "u64 sparse_banks[64]" inside the methods (on the stack), causes the
> stack-segment-memory-allocation to go beyond 1024 bytes, thus raising the
> warning/error which breaks the build.
> 
> Fix :
> =====
> 
> Instead of defining "u64 sparse_banks [64]" inside the methods, we instead
> define this array in the (only) client method "kvm_hv_hypercall", and then
> pass the array (and its size) as additional arguments to the two methods.

> Doing this, we do not exceed the 1024 bytes stack-segment-memory-allocation,
> on any stack-segment of any method.

This is a hack, and it's not guaranteed to work, e.g. if the compiler decided to
inline the helpers, then presumably this problem would rear its head again.

However, I don't think this is a problem any more.  gcc-10 and clang-11 are both
comfortably under 1024, even if I force both helpers to be inlined.  Neither
function has variables that would scale with NR_CPUS (and I verified high number
of NR_CPUS for giggles).  Can you try reproducing the behavior on the latest
kvm/queue?  I swear I've seen this in the past, but I couldn't find a commit that
"fixed" any such warning.

If it does repro, can you provide your .config and compiler version?  Maybe your
compiler is doing somethign funky?

> Signed-off-by: ajay <ajaygargnsit@gmail.com>

The SoB needs your full name.

> ---
>  arch/x86/kvm/hyperv.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 232a86a6faaf..5340be93daa4 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1750,7 +1750,8 @@ struct kvm_hv_hcall {
>  	sse128_t xmm[HV_HYPERCALL_MAX_XMM_REGISTERS];
>  };
>  
> -static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool ex)
> +static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc,
> +                            bool ex, u64 *sparse_banks, u32 num_sparse_banks)


>  {
>  	int i;
>  	gpa_t gpa;
> @@ -1762,10 +1763,11 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc, bool
>  	DECLARE_BITMAP(vcpu_bitmap, KVM_MAX_VCPUS);
>  	unsigned long *vcpu_mask;
>  	u64 valid_bank_mask;
> -	u64 sparse_banks[64];
>  	int sparse_banks_len;
>  	bool all_cpus;
>  
> +        memset(sparse_banks, 0, sizeof(u64) * num_sparse_banks);
> +

FWIW, the array size needs to be validated, there is other code in this function
that assumes it's at least 64 entries.