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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sean,

Thanks for your time.

I have cloned the kernel many times since the time the patch was
posted, and have not seen the issue again.

One thing I distinctly remember that when the build was breaking, it
was with staging-drivers disabled. Since then, I have disabled the
staging-drivers. Today, I again enabled staging-drivers, and build
went fine.

So, I am ok with archiving this patch. We can revisit if someone else
reports this/similar issue.


Thanks and Regards,
Ajay


On Thu, Oct 14, 2021 at 12:30 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, Sep 17, 2021, Ajay Garg wrote:
> > From: ajay <ajaygargnsit@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
>
> 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.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux