Make BPF stack size overridable

[niclashedam]

Summary

With the emergence of TP 4091 for NVMe, eBPF can be used to offload programs to computational storage processors.
This change introduces the possibility of overriding the default stack size of 512 bytes for non-kernel runtime environments.

[ast]

makes sense to me. Curious what kind of stack limits in HW do you envision?

[niclashedam]

Hello Alexei,
I appreciate your prompt attention. Determining the maximum stack size is difficult, but a reasonable estimate could be an increase of two orders of magnitude.
We developed a computational storage processor at the IT University of Copenhagen that executes eBPF programs. During our evaluation of loop versus loop-unrolling performance on hardware, we discovered that the stack is easily prone to exhaustion and that kernel characteristics no longer apply when executing on hardware.
We plan on conducting additional research to comprehend the characteristics of eBPF on computational storage.

[niclashedam]

I removed the llvm namespace since it is already known through the using statement.
I furthermore made the parameter unsigned since a stack will never be negative.

[niclashedam]

Sorry for the revision spam.
It looks like using an unsigned integer had adverse effects on the tests. I am reverting to signed integers but keeping the namespace change.

[yonghong-song]

LGTM. How do you verify bpf program in your environment?

[niclashedam]

Thanks for the review. I have a colleague at the IT University of Copenhagen doing a PhD on program verification for computational storage. However, for now, it's an open question.
We do some runtime checks, but the real issue is that when compiling BPF for computational storage, the device characteristics (stack size, helper functions, etc.) are unknown. As such, the verification must happen on the device, and the question is how we efficiently do that and how we convey potential verification problems back to the host.

[yonghong-song]

In D147707#4255094, @niclashedam wrote:

Thanks for the review. I have a colleague at the IT University of Copenhagen doing a PhD on program verification for computational storage. However, for now, it's an open question.
We do some runtime checks, but the real issue is that when compiling BPF for computational storage, the device characteristics (stack size, helper functions, etc.) are unknown. As such, the verification must happen on the device, and the question is how we efficiently do that and how we convey potential verification problems back to the host.

Okay, thanks for explanation. The patch certainly okay as it doesn't change the current default funcitonality. Not exactly sure how the decide will verifier the program but that is a separate issue.

[niclashedam]

@eddyz87 Ping.

[eddyz87]

In D147707#4273800, @niclashedam wrote:

@eddyz87 Ping.

Sorry, since Yonghong and Alexei commented on this diff I hadn't thought that my "accept" is necessary.
The diff seem to be fine.

[niclashedam]

Hi Eddy. Thanks for the review! I assume your review was the thing @ast was waiting for. Now we can get it merged!

[eddyz87]

I will push these changes today.

[eddyz87]

In D147707#4274146, @niclashedam wrote:

Hi Eddy. Thanks for the review! I assume your review was the thing @ast was waiting for. Now we can get it merged!

Could you please update your profile with email, so that I can set commit author? (or ping me at [e-mail]).
I'm following this guide and it suggests setting authors name + email, also commit history suggests that this is the norm.

[niclashedam]

In D147707#4275238, @eddyz87 wrote:

In D147707#4274146, @niclashedam wrote:

Hi Eddy. Thanks for the review! I assume your review was the thing @ast was waiting for. Now we can get it merged!

Could you please update your profile with email, so that I can set commit author? (or ping me at [e-mail]).
I'm following this guide and it suggests setting authors name + email, also commit history suggests that this is the norm.

There should already be an e-mail on my profile. If you can't see it, here's the info.

Niclas Hedam <[e-mail]>

[eddyz87]

In D147707#4275300, @niclashedam wrote:

In D147707#4275238, @eddyz87 wrote:

In D147707#4274146, @niclashedam wrote:

Hi Eddy. Thanks for the review! I assume your review was the thing @ast was waiting for. Now we can get it merged!

Could you please update your profile with email, so that I can set commit author? (or ping me at [e-mail]).
I'm following this guide and it suggests setting authors name + email, also commit history suggests that this is the norm.

There should already be an e-mail on my profile. If you can't see it, here's the info.

Niclas Hedam <[e-mail]>

Thanks, the info is not shown in the profile page for some reason.

llvm/lib/Target/BPF/BPFRegisterInfo.cpp

 #include "BPFSubtarget.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 
 #define GET_REGINFO_TARGET_DESC
 #include "BPFGenRegisterInfo.inc"
 using namespace llvm;
 
+static cl::opt<int>
+    BPFStackSizeOption("bpf-stack-size",
+                       cl::desc("Specify the BPF stack size limit"),
+                       cl::init(512));
+
 BPFRegisterInfo::BPFRegisterInfo()
     : BPFGenRegisterInfo(BPF::R0) {}
 
 const MCPhysReg *
 BPFRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   return CSR_SaveList;
 }
 
 BitVector BPFRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   BitVector Reserved(getNumRegs());
   markSuperRegs(Reserved, BPF::W10); // [W|R]10 is read only frame pointer
   markSuperRegs(Reserved, BPF::W11); // [W|R]11 is pseudo stack pointer
   return Reserved;
 }
 
 static void WarnSize(int Offset, MachineFunction &MF, DebugLoc& DL)
 {
-  if (Offset <= -512) {
-      const Function &F = MF.getFunction();
-      DiagnosticInfoUnsupported DiagStackSize(F,
-          "Looks like the BPF stack limit of 512 bytes is exceeded. "
-          "Please move large on stack variables into BPF per-cpu array map.\n",
-          DL);
-      F.getContext().diagnose(DiagStackSize);
+  if (Offset <= -BPFStackSizeOption) {
+    DiagnosticInfoUnsupported DiagStackSize(
+        F,
+        "Looks like the BPF stack limit is exceeded. "
+        "Please move large on stack variables into BPF per-cpu array map. For "
+        "non-kernel uses, the stack can be increased using -mllvm "
+        "-bpf-stack-size.\n",
   }
 }
 
 bool BPFRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
                                           int SPAdj, unsigned FIOperandNum,
                                           RegScavenger *RS) const {
   assert(SPAdj == 0 && "Unexpected");