Fix pciedriver and portalmem to support newest kernel versions
#198
Conversation
jameyhicks-cmt
left a comment
jameyhicks-cmt
left a comment
There was a problem hiding this comment.
Thank you for submitting this PR. I have a few comments.
| "MemTagSize=16", "MemServerTags=8", | ||
| "DEFAULT_NOPROGRAM=1", | ||
| "CONNECTAL_BITS_DEPENDENCES=build/checkpoints/to_aws/mkTop.SH_CL_routed.dcp", "CONNECTAL_RUN_SCRIPT=$(CONNECTALDIR)/scripts/run.aws"], | ||
| "CONNECTAL_BITS_DEPENDENCES=build/checkpoints/to_aws/mkTop.SH_CL_routed.dcp", "CONNECTAL_RUN_SCRIPT=$(CONNECTALDIR)/scripts/aws/run.awsf1"], |
There was a problem hiding this comment.
I don't think this was ever really debugged, but it's good to fix the typo
cpp/poller.cpp
Outdated
| for (int i = 0; i < numWrappers; i++) { | ||
| if (!portal_wrappers) | ||
| fprintf(stderr, "Poller: No portal_instances revents=%d\n", portal_fds[i].revents); | ||
| // if (!portal_wrappers) |
There was a problem hiding this comment.
I don't think we want to comment out this code
There was a problem hiding this comment.
Fixed. Not sure why it was commented.
drivers/pcieportal/pcieportal.c
Outdated
| #include <linux/poll.h> /* poll_table, etc. */ | ||
| #include <asm/uaccess.h> /* copy_to_user, copy_from_user */ | ||
| #include <linux/dma-buf.h> | ||
| // #include <linux/dma-mapping.h> |
There was a problem hiding this comment.
have these files gone away? If so we need an appropriate ifdef around these lines
There was a problem hiding this comment.
Hm, I'm not sure yet. I was integrating parts of the changes implemented by @ljz-1109 and must have missed that.
I can look into this and introduce necessary #ifdefs.
There was a problem hiding this comment.
Fixed. Not sure why it was commented. Those files still remain to these days.
Those files are needed for the recent kernel API for setting up DMA. While they seem to be transitively included through other files (hence commenting out works), it's probably best to make those includes explicit.
pcie/pcieflat
Outdated
| counter = counter - 1 | ||
| buf = fcntl.ioctl(fd, BNOC_GET_TLP, ' ' * Tlp_len) | ||
| foo = '' | ||
| for x in buf: |
There was a problem hiding this comment.
wow, I'm sorry this code is so ugly. Seems like it should be buf.hex() or reverse(buf).hex() or something like that but if your changes work I am OK with them.
There was a problem hiding this comment.
Ditto previous comment. I'm having trouble understanding how this code works so I'm just taking the working change for granted now.
3 participants
RHEL7 has hit EOL, so do Amazon's old version of FPGA Developer AMI and F1 instances.
The new AMI (intended for F2 instances) runs on Ubuntu 20.04. I made the changes necessary for Connectal to compile successfully on Ubuntu 20.04. I also made sure that it compiles on Ubuntu 24.04 as well, since that's one of the most recent systems I expect most people will want to use.
If there are people with access to older machines who could try to compile (i.e.
sudo make install) and report any additional fixes needed, say, for Ubuntu 16.04, that would be appreciated.Unfortunately, I haven't quite figured out how to actually support synthesis for F2 instances yet.