Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

print_cpuid display incorrect data for at least Cortex-A35 #43

Closed
bennoleslie opened this issue Sep 25, 2020 · 2 comments
Closed

print_cpuid display incorrect data for at least Cortex-A35 #43

bennoleslie opened this issue Sep 25, 2020 · 2 comments
Labels
bug

Comments

@bennoleslie
Copy link

The function print_cpuid incorrectly calculates the CPU name associated with the part number.

The current code is:

    if ((CPUID_PART(cpuid) & 0xf00) == 0xC00) {
        printf("Cortex-A%d ", CPUID_PART(cpuid) & 0xff);
    } else if ((CPUID_PART(cpuid) & 0xf00) == 0xD00) {
        printf("Cortex-A5%d ", CPUID_PART(cpuid) & 0xff);
    } else {
        printf("Part: 0x%03x ", CPUID_PART(cpuid));

However, the Cortex-A35 part is 0xD04. The current logic prints that as Cortex-A54, which is obviously wrong.

I think the assumptions made in this printing is just incorrect. A switch-based look up table is probably most appropriate. Something like:

    switch (CPUID_PART(cpuid)) {
       case 0xD04:
           printf("Cortex-A35 ");
            break;
        default:
            printf("Part: 0x%03x ", CPUID_PART(cpuid));
     }

But, obviously, with additional switch statements for other, supported, CPUs.
@axel-h
Copy link
Member

axel-h commented Sep 27, 2020

Sounds reasonable to me. Can you make a pull request with a patch?

@axel-h axel-h added the bug label Jun 13, 2021
@axel-h axel-h mentioned this issue Aug 30, 2021
Closed
bennoleslie pushed a commit to BreakawayConsulting/seL4_tools that referenced this issue Aug 30, 2021
Fix ARM CPUID printing
7e2e159 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: At some level this will be a regression as it now only
prints the textual id for Cortex-A35 and no other CPU.

Improvements to list other CPUID mappings is welcome, unfortunately
the only platform I have access to for testing is Cortex-A35.
@bennoleslie bennoleslie mentioned this issue Aug 30, 2021
Merged
bennoleslie pushed a commit to BreakawayConsulting/seL4_tools that referenced this issue Aug 30, 2021
Fix ARM CPUID printing
80d87c8 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: At some level this will be a regression as it now only
prints the textual id for Cortex-A35 and no other CPU.

Improvements to list other CPUID mappings is welcome, unfortunately
the only platform I have access to for testing is Cortex-A35.

Signed-off-by: Ben Leslie <benno@brkawy.com>
@bennoleslie
Copy link
Author

OK. I finally made a pull request: #107

bennoleslie pushed a commit to BreakawayConsulting/seL4_tools that referenced this issue Aug 30, 2021
Fix ARM CPUID printing
2fbba70 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: At some level this will be a regression as it now only
prints the textual id for Cortex-A35 and no other CPU.

Improvements to list other CPUID mappings is welcome, unfortunately
the only platform I have access to for testing is Cortex-A35.

Signed-off-by: Ben Leslie <benno@brkawy.com>
bennoleslie pushed a commit to BreakawayConsulting/seL4_tools that referenced this issue Sep 2, 2021
Fix ARM CPUID printing
0d139a0 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: At some level this will be a regression as it now only
prints the textual id for Cortex-A35 and no other CPU.

Improvements to list other CPUID mappings is welcome, unfortunately
the only platform I have access to for testing is Cortex-A35.

Signed-off-by: Ben Leslie <benno@brkawy.com>
bennoleslie pushed a commit to BreakawayConsulting/seL4_tools that referenced this issue Sep 2, 2021
Fix ARM CPUID printing
113bb1e 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: Only the Cortex-A35 case in the switch statement has
been tested. Others cases are implemented from available
lists only and have not been validated on real hardware.

Sources:

https://en.wikipedia.org/wiki/Comparison_of_ARMv8-A_cores
seL4#107 (comment)

Signed-off-by: Ben Leslie <benno@brkawy.com>
@axel-h axel-h mentioned this issue Sep 2, 2021
Closed
lsf37 pushed a commit that referenced this issue Sep 2, 2021
@lsf37
Fix ARM CPUID printing
c5f0e54 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue #43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: Only the Cortex-A35 case in the switch statement has
been tested. Others cases are implemented from available
lists only and have not been validated on real hardware.

Sources:

https://en.wikipedia.org/wiki/Comparison_of_ARMv8-A_cores
#107 (comment)

Signed-off-by: Ben Leslie <benno@brkawy.com>
sleffler pushed a commit to AmbiML/sparrow-seL4_tools that referenced this issue Oct 11, 2022
@sleffler
Fix ARM CPUID printing
a69406c 
The existing print_cpuid function displays the incorrect value
for Cortex-A35. (See issue seL4#43).

This change makes CPUID determination an explicit switch statement
rather than attempting to automatically determine it through
masking certain bits.

Note: Only the Cortex-A35 case in the switch statement has
been tested. Others cases are implemented from available
lists only and have not been validated on real hardware.

Sources:

https://en.wikipedia.org/wiki/Comparison_of_ARMv8-A_cores
seL4#107 (comment)

Signed-off-by: Ben Leslie <benno@brkawy.com>
GitOrigin-RevId: c5f0e54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bennoleslie @axel-h