Bit field extract with struct in c

Question

I uses these two methods to get the bit field information from registers. The location of the bit field that I need extract is given by Intel Manual. Just as the code below. But the results I got are different with these two methods.

I cannot find any problems for these two methods. But for my understanding, maximum_power filed should not be '0' as the first method (It is the value that Intel has already defined in the register.)

Method 1:

typedef struct rapl_parameters_msr_t {
  uint64_t thermal_spec_power        : 15;
  uint64_t                           : 1;
  uint64_t minimum_power             : 15;
  uint64_t                           : 1;
  uint64_t maximum_power             : 15;
  uint64_t                           : 1;
  uint64_t maximum_limit_time_window : 6;
  uint64_t                           : 10;
} rapl_parameters_msr_t;

uint64_t              msr;
read_msr(cpu, 0x614, &msr);

rapl_parameters_msr_t domain_msr = *(rapl_parameters_msr_t *)&msr;
printf("%ld\n", domain_msr.thermal_spec_power);  //print: 280
printf("%ld\n", domain_msr.minimum_power);  //print: 192
printf("%ld\n", domain_msr.maximum_power);  //print: 0
printf("%ld\n", domain_msr.maximum_limit_time_window);  //print: 16

Method 2:

uint64_t 
extractBitField(uint64_t inField, uint64_t width, uint64_t offset)
{
    uint64_t bitMask;
    uint64_t outField;

    if ((offset+width) == 32) 
    {
        bitMask = (0xFFFFFFFF<<offset);
    }
    else 
    {  /*Just keep the filed needs to be extrated*/
        bitMask = (0xFFFFFFFF<<offset) ^ (0xFFFFFFFF<<(offset+width));  

    }
    /*Move to the right most field to be calculated*/
    outField = (inField & bitMask) >> offset;
    return outField;
}
uint64_t              flags;
read_msr(cpu, 0x614, &flags);
printf("thermal power: %d\n", extractBitField(flags,15,0));  //print: 280
printf("minimum power: %d\n", extractBitField(flags,15,16));//print: 192
printf("maximum power: %d\n", extractBitField(flags,15,32));//print: 0
printf("time window: %d\n", extractBitField(flags,6,48));  //print: 0

Do you have any insights where the problem would be?

Update:

Sorry for the confused part. I changed all type to be uint64_t, and the method 2 gets 0 for both maximum power and time window..

If compiler would make possible wrong result for method 1, I am still doubted how much I can trust for the method 2 result..

The following is the bit represented documentation from Intel Manual:

Thermal Spec Power (bits 14:0)
Minimum Power (bits 30:16)
Maximum Power (bits 46:32)
Maximum Time Window (bits 53:48)

Thank you for David, this is the right version for 64 bit extraction.

uint64_t 
extractBitField(uint64_t inField, uint64_t width, uint64_t offset)
{
    uint64_t bitMask;
    uint64_t outField;

    if ((offset+width) == 64) 
    {
        bitMask = (0xFFFFFFFFFFFFFFFF<<offset);
    }
    else 
    {  /*Just keep the filed needs to be extrated*/
        bitMask = (0xFFFFFFFFFFFFFFFF<<offset) ^ (0xFFFFFFFFFFFFFFFF<<(offset+width));  

    }
    /*Move to the right most field to be calculated*/
    outField = (inField & bitMask) >> offset;
    return outField;
}
uint64_t              flags;
read_msr(cpu, 0x614, &flags);
printf("thermal power: %d\n", extractBitField(flags,15,0));  //print: 280
printf("minimum power: %d\n", extractBitField(flags,15,16));//print: 192
printf("maximum power: %d\n", extractBitField(flags,15,32));//print: 0
printf("time window: %d\n", extractBitField(flags,6,48));  //print: 16

Answer 1

The ordering of bits in C bitfields is implementation defined, so be careful if you plan on using them-- the order you think you're getting may not be what you actually are. Check your compiler's documentation to see how it handles this.

Also, your second function takes in a uint32 while your first example is using a 64 bit struct, so your types aren't matching up. Can you correct that and update your results?

edit: Additionally, you have the time windows defined as six bits in the first example and 15 in the second.

C99:6.7.2.1p10 An implementation may allocate any addressable storage unit large enough to hold a bit- field. If enough space remains, a bit-field that immediately follows another bit-field in a structure shall be packed into adjacent bits of the same unit. If insufficient space remains, whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined. The order of allocation of bit-fields within a unit (high-order to low-order or low-order to high-order) is implementation-defined. The alignment of the addressable storage unit is unspecified.

Answer 2

You have tried two ways to do the same thing, and I wouldn't trust either of them.

First, bit fields. Don't use them! The ordering of bit fields is unreliable, the behaviour of anything other than unsigned int is unreliable, the distribution of bit fields across struct members is unreliable. All these things can be fixed, but it just isn't worth it.

Second, shift and mask. This is the right way to do it but the code is wrong. You have a 32-bit mask (0xffffffff) shifted by 32 and 48 bits. Not a good idea at all.

So, what you need to do is write a simple reliable function that is an implementation of the signature given.

extractBitField(uint64_t inField, uint64_t width, uint64_t offset)

This is a good place to start. Write the function in a test program and unit test it until you are 100% certain it works exactly right. Step through with the debugger, check out all the shift combinations. Be absolutely sure you have it right.

When the test program works properly then transfer the function to the real program and watch it work first time.

I guess I could code that function for you but I don't think I will. You really need to go through the exercise so you learn how it works and why.