Thread (3 messages) 3 messages, 2 authors, 2021-10-14

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

From: Sean Christopherson <seanjc@google.com>
Date: 2021-10-13 19:00:24

On Fri, Sep 17, 2021, Ajay Garg wrote:
From: ajay <redacted>

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 <redacted>
The SoB needs your full name.
quoted hunk ↗ jump to hunk
---
 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)
quoted hunk ↗ jump to hunk
 {
 	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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help