Address "make dudect more portable?" #36
Conversation
|
This is great @itzmeanjan . Thank you. Can you please list here the targets in which you tested it? (Maybe the test should be run I'll also test in a bunch of hardware and report back here. |
itzmeanjan
force-pushed
the
make-it-more-portable
branch
from
2fc690e
to
a964b6e
Compare
I've tested the patch, issuing
$ uname -srm
Linux 6.5.0-15-generic x86_64
$ uname -srm
Darwin 23.2.0 arm64
$ uname -srm
Linux 6.5.0-1009-raspi aarch64 |
There was a problem hiding this comment.
still need to test it in a couple of different changes but code review LGTM. Thanks a lot @itzmeanjan for this!!!
will merge soon after I get to test it
src/dudect.h
Outdated
| @@ -279,6 +289,61 @@ static inline int64_t cpucycles(void) { | |||
| return (int64_t)__rdtsc(); | |||
| } | |||
|
|
|||
| #elif defined __aarch64__ && defined __linux__ | |||
There was a problem hiding this comment.
| #elif defined __aarch64__ && defined __linux__ | |
| #elif defined(__aarch64__) && defined(__linux__) |
| To enforce CPU to complete all pending memory access operations, appearing before PMCCTR_EL0, we issue a | ||
| *D*ata *S*ynchronization *B*arrier instruction right before reading CPU cycle counter. | ||
|
|
||
| Note, issuing PMCCTR_EL0 instruction from the userspace will probably result in panicing with |
There was a problem hiding this comment.
I want to test this, but in any case: would we want to printf this helpful message in the _init function on aarch64? we can take it in another PR
There was a problem hiding this comment.
Yes, I think we should notify the user in some way that they are expected to do something special for this target. How do you feel about using #warning "...", which emits a warning message during program compilation time also ? See https://en.cppreference.com/w/c/preprocessor/error
src/dudect.h
Outdated
| return (int64_t)val; | ||
| } | ||
|
|
||
| #elif defined __APPLE__ |
There was a problem hiding this comment.
| #elif defined __APPLE__ | |
| #elif defined(__APPLE__) |
| To enforce CPU to complete all pending memory access operations, appearing before PMCCTR_EL0, we issue a | ||
| *D*ata *S*ynchronization *B*arrier instruction right before reading CPU cycle counter. | ||
|
|
||
| Note, issuing PMCCTR_EL0 instruction from the userspace will probably result in panicing with |
There was a problem hiding this comment.
Yes, I think we should notify the user in some way that they are expected to do something special for this target. How do you feel about using #warning "...", which emits a warning message during program compilation time also ? See https://en.cppreference.com/w/c/preprocessor/error
src/dudect.h
Outdated
| /* | ||
| Returns approximate measurement of CPU time consumed by the calling process (i.e., CPU time | ||
| consumed by all threads in the process). | ||
|
|
||
| See https://www.man7.org/linux/man-pages/man3/clock_gettime.3.html | ||
| */ | ||
| static inline int64_t cpucycles(void) { | ||
| const clockid_t clock_id = CLOCK_PROCESS_CPUTIME_ID; | ||
| struct timespec ts = {0}; | ||
|
|
||
| (void)clock_gettime(clock_id, &ts); | ||
|
|
||
| const uint64_t ns = (uint64_t)ts.tv_sec * (uint64_t)1e9 + (uint64_t)ts.tv_nsec; | ||
| return (int64_t)ns; | ||
| } |
There was a problem hiding this comment.
I'd like to get rid of this, this is giving me way too many false positives - too much in consistency. I'd just write a #error "..." with a message, which should say we don't yet support this target.
… and wider spectrum of non-x86_64 targets Signed-off-by: Anjan Roy <[email protected]>
itzmeanjan
force-pushed
the
make-it-more-portable
branch
from
a964b6e
to
3dc6585
Compare
|
👋 @oreparaz can we merge this ? |
Adds support for running
dudect-based constant-time tests on more targets i.e. aarch64 (running Linux kernel), Apple Silicon and more non-x86_64 targets.Partially addresses #33.