Skip to content

Commit

Permalink
Remove of use of volatile and make memory allocation only for the lif…
Browse files Browse the repository at this point in the history
…etime of a function that needs it

- Do not use volatile for variables that will not be modified in a way the compiler cannot detect

- Make the buffer used for topologyEntry structures allocated and freed when topology is being built, rather than being allocated for the lifetime of the driver load

- Change macOS TopologyEntry struct to use int rather than uint, which is consistent with the other platforms in this project, as well as macOS's functions.

- Instead of detecting at run time how big of a buffer we will need when issuing a sysctl, always use 4 bytes, which is specified in the docs and examples in kernel source code (see /usr/include/sys/sysctl.h)
  • Loading branch information
nealsid committed Sep 7, 2022
1 parent c022f7a commit 3ce12eb
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 46 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,5 @@ latex/
*.vcxproj.user
.vs/
.idea/
build
src/simdjson
74 changes: 39 additions & 35 deletions src/MacMSRDriver/PcmMsr/PcmMsr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ inline void WRMSR(uint32_t msr, uint64_t value)

void cpuReadMSR(void* pIData){
pcm_msr_data_t* data = (pcm_msr_data_t*)pIData;
volatile uint cpu = cpu_number();
int cpu = cpu_number();
if(data->cpu_num == cpu)
{
data->value = RDMSR(data->msr_num);
Expand All @@ -49,7 +49,7 @@ void cpuReadMSR(void* pIData){

void cpuWriteMSR(void* pIDatas){
pcm_msr_data_t* idatas = (pcm_msr_data_t*)pIDatas;
volatile uint cpu = cpu_number();
int cpu = cpu_number();
if(idatas->cpu_num == cpu)
{
WRMSR(idatas->msr_num, idatas->value);
Expand All @@ -58,7 +58,7 @@ void cpuWriteMSR(void* pIDatas){

void cpuGetTopoData(void* pTopos){
topologyEntry* entries = (topologyEntry*)pTopos;
volatile uint cpu = cpu_number();
int cpu = cpu_number();
int info[4];
entries[cpu].os_id = cpu;
cpuid(0xB, 1, info[0], info[1], info[2], info[3]);
Expand Down Expand Up @@ -86,50 +86,34 @@ bool PcmMsrDriverClassName::start(IOService* provider){

return success;
}
uint32_t PcmMsrDriverClassName::getNumCores()

int32_t PcmMsrDriverClassName::getNumCores()
{
size_t size;
char* pParam;
uint32_t ret = 0;
if(!sysctlbyname("hw.logicalcpu", NULL, &size, NULL, 0))
int32_t ncpus = 0;
size_t ncpus_size = sizeof(ncpus);
if(sysctlbyname("hw.logicalcpu", &ncpus, &ncpus_size, NULL, 0))
{
if(NULL != (pParam = (char*)IOMalloc(size)))
{
if(!sysctlbyname("hw.logicalcpu", (void*)pParam, &size, NULL, 0))
{
if(sizeof(int) == size)
ret = *(int*)pParam;
else if(sizeof(long) == size)
ret = (uint32_t) *(long*)pParam;
else if(sizeof(long long) == size)
ret = (uint32_t) *(long long*)pParam;
else
ret = *(int*)pParam;
}
IOFree(pParam, size);
}
IOLog("%s[%p]::%s() -- sysctl failure retrieving hw.logicalcpu",
getName(), this, __FUNCTION__);
ncpus = 0;
}
return ret;

return ncpus;
}

bool PcmMsrDriverClassName::init(OSDictionary *dict)
{
num_cores = getNumCores();
bool result = super::init(dict);
topologies = 0;
if(result && num_cores != 0)
{
topologies = (topologyEntry*)IOMallocAligned(sizeof(topologyEntry)*num_cores, 32);

if (result) {
num_cores = getNumCores();
}
return (result && topologies && num_cores != 0);

return result && num_cores;
}

void PcmMsrDriverClassName::free()
{
if (topologies)
{
IOFreeAligned(topologies, sizeof(topologyEntry)*num_cores);
}
super::free();
}

Expand Down Expand Up @@ -182,14 +166,34 @@ IOReturn PcmMsrDriverClassName::writeMSR(pcm_msr_data_t* idata){
return ret;
}

IOReturn PcmMsrDriverClassName::buildTopology(topologyEntry* odata, uint32_t input_num_cores){
IOReturn PcmMsrDriverClassName::buildTopology(topologyEntry* odata, uint32_t input_num_cores)
{
size_t topologyBufferSize;

// TODO figure out when input_num_cores is used rather than num_cores
if (os_mul_overflow(sizeof(topologyEntry), (size_t) num_cores, &topologyBufferSize))
{
return kIOReturnBadArgument;
}

topologyEntry *topologies =
(topologyEntry *)IOMallocAligned(topologyBufferSize, 32);

if (topologies == nullptr)
{
return kIOReturnNoMemory;
}

mp_rendezvous_no_intrs(cpuGetTopoData, (void*)topologies);

for(uint32_t i = 0; i < num_cores && i < input_num_cores; i++)
{
odata[i].core_id = topologies[i].core_id;
odata[i].os_id = topologies[i].os_id;
odata[i].socket = topologies[i].socket;
}

IOFreeAligned(topologies, topologyBufferSize);
return kIOReturnSuccess;
}

Expand Down
5 changes: 2 additions & 3 deletions src/MacMSRDriver/PcmMsr/PcmMsr.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class PcmMsrDriverClassName : public IOService
virtual bool handleIsOpen(const IOService* forClient) const override;
virtual void handleClose(IOService* forClient, IOOptionBits opts) override;

virtual uint32_t getNumCores();
virtual int32_t getNumCores();

virtual IOReturn incrementNumInstances(uint32_t* num_instances);
virtual IOReturn decrementNumInstances(uint32_t* num_instances);
Expand All @@ -36,8 +36,7 @@ class PcmMsrDriverClassName : public IOService
private:
// number of providers currently using the driver
uint32_t num_clients = 0;
uint32_t num_cores;
topologyEntry *topologies;
int32_t num_cores;
};

#ifdef DEBUG
Expand Down
16 changes: 8 additions & 8 deletions src/MacMSRDriver/PcmMsr/UserKernelShared.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ typedef struct {
// The topologyEntry struct that is used by PCM
typedef struct
{
uint32_t os_id;
uint32_t thread_id;
uint32_t core_id;
uint32_t tile_id;
uint32_t socket;
uint32_t native_cpu_model;
uint32_t core_type; // This is an enum in the userland structure.
uint32_t padding;
int32_t os_id;
int32_t thread_id;
int32_t core_id;
int32_t tile_id;
int32_t socket;
int32_t native_cpu_model;
int32_t core_type; // This is an enum in the userland structure.
int32_t padding;
} topologyEntry;

enum {
Expand Down

0 comments on commit 3ce12eb

Please sign in to comment.