- User Since
- Sep 8 2016, 3:21 PM (297 w, 8 h)
Mar 5 2022
I've read through the man page changes twice and couldn't find anything wrong. LGTM but maybe a read through by someone from docs would be a good idea.
Mar 2 2022
Both seem reasonable to me. I'll aim to run some more tests with this and look it over one more time in detail by Friday and flag if I find anything, but overall looks good to me. Thanks for working on this!
Overall the patch looks good to me. I've run all of this under ASAN + UBSAN (with the full patch set applied) and nothing was flagged in this code as problematic when running the DTrace test suite and the FreeBSD build. I feel like it might be a good idea to properly restructure the code around CTFv2 and CTFv3 to account for other versions in the API rather than relying on the "else" case always being v3, but aside from that LGTM. @markj: what do you think about the alignment warning/error propagation?
Feb 24 2022
The code looks good to me, but it probably needs man page changes like @debdrup mentioned. I also wonder if adding a comment at the top of the header somewhere simply stating what CTFv2 and CTFv3 are and how they differ at a high level makes sense. Perhaps in one or two short sentences so that if someone unfamiliar with CTF wants to simply use the header knows what they're working with without needing to read through a fairly detailed man page? Do you have any thoughts on this?
This looks good to me. Thanks!
The original CTF header has a bunch of useful comments describing the format. Perhaps these things should be included in this header too? While I appreciate that this is an import, those comments have helped me quite a bit when dealing with CTF and IMO should be included.
Dec 10 2021
Sep 23 2021
Sep 12 2021
Thanks for writing this up @mhorne, much appreciated! I've left a few comments from my experience of setting it up with QEMU + KVM today and the confusion I had while reading the existing documentation, as well as the confusion that I would probably have while reading this.
Sep 4 2021
@jhb Any chance you could push this? I've been using this for months and it's worked well for me.
Jun 16 2021
Jun 15 2021
May 27 2021
This seems reasonable to me.
May 16 2021
I've been using this locally and found it *extremely* useful, and frankly was quite surprised when I found out that EV_ENABLE/EV_DISABLE trashed udata originally.
May 13 2021
Remove a whole bunch of noise compared to the previous diff.
Update the diff with -U999999
Apr 1 2021
@markj 's comments.
Added -x libdir and -x syslibdir as suggested by @markj
Mar 27 2021
Mar 26 2021
Aug 27 2018
Why "additional"? The problem exists regardless since fbt_invop() returns after the first matching tracepoint. The problem was invisible before ifuncs since before that all tracepoints had distinct addresses. This patch just hides the problem in the common case.
I'm a little worried that this may cause additional confusion if someone expects multiple probes to fire at that point. I wonder if a better workaround would be to just add a warning message whenever DTrace is called, or perhaps when someone calls dtrace -l, in the man page or something along the lines until we fix? Is there a reason we can't fix this in 13 and MFC the changes?
May 8 2018
Preemptively remove a redundant check for INKERNEL(fp).
Apr 30 2018
Had a quick skim, but LGTM.
Apr 21 2018
Apr 12 2018
Is this good to land? Thanks!
Apr 6 2018
Address the other comments by @markj and fix a misleading word in the dtrace_probe_exit() comment.
Address comments by @markj.
Apr 4 2018
Apr 3 2018
Is there anything else I can do for this to land?
Mar 28 2018
Should I make any more changes for this to land? As an aside note: I've tried to make the interface as simple as possible to use with the args[0-2] being consistent.
Mar 27 2018
Update the diff based on comments from @markj.
Mar 26 2018
Mar 11 2018
Jan 12 2018
Address the comments by markj@ and add tests in safety/ for jailname and jid.
Update the diff to add more context. No actual code was changed.
Jan 11 2018
Jun 18 2017
Update the diff to reflect changes in HEAD with bhyve identification. This diff no longer requires it.
Jun 10 2017
Mar 3 2017
Update the diff to address the compilation errors due to the way clang 4.0 handles -Wstrict-prototypes.
Feb 2 2017
Dec 19 2016
This diff changes the following:
- Remove DTrace stuff as much as possible, create a prototype instead
- Remove the unnecessary static NULL assignments in the hc_dispatcher
- Reiterate the instructions on some parts of the code
Dec 3 2016
Point out an issue regarding mi_startup.
Address some style(9) issues, fix a comment.
Nov 27 2016
License bhyve_hypercall.S under the FreeBSD preferred license.
Remove the unused include in bhyve_hypercall.h
Address the points discussed at MeetBSD.
Oct 13 2016
Addresses a late noticed typo in the macro HC_CPUID_ID which in fact should be HV_CPUID_ID.
This patch addresses a couple of issues:
- Allow global enabling and disabling of hypercalls. The hypercall exists are by default disabled
- Add the necessary dispatchers and handlers for a hypervisor emulation layer
- Clean up the bhyve_hypercall.h using macros and revert to the calling convention that uses the stack
- Add a tunable which allows setting of the hypervisor mode
- Unimplemented hypercalls now cause an #UD fault
- CPUID_4000_0001 gives information in EAX whether hypercalls are enabled or not
Oct 3 2016
This patch should address some of the concerns brought on by Peter.
Oct 1 2016
Sep 30 2016
Thanks for the review Peter. I'll address these issues as soon as possible.
Added some explanations.