Add CPU features and fix CPU vendor #2
@ -110,11 +110,16 @@
|
||||
#define X86_EFLAGS_VIP_MASK 0x00100000
|
||||
#define X86_EFLAGS_ID_MASK 0x00200000
|
||||
|
||||
/* CPUID vendor names */
|
||||
#define CPUID_VENDOR_NAME_AMD "AuthenticAMD"
|
||||
#define CPUID_VENDOR_NAME_INTEL "GenuineIntel"
|
||||
|
||||
/* CPU vendor enumeration list */
|
||||
typedef enum _CPU_VENDOR
|
||||
{
|
||||
CPU_VENDOR_AMD = 0x68747541,
|
||||
|
||||
CPU_VENDOR_INTEL = 0x756e6547,
|
||||
CPU_VENDOR_INVALID,
|
||||
CPU_VENDOR_AMD,
|
||||
CPU_VENDOR_INTEL,
|
||||
CPU_VENDOR_UNKNOWN = 0xFFFFFFFF
|
||||
} CPU_VENDOR, *PCPU_VENDOR;
|
||||
|
||||
@ -144,7 +149,7 @@ typedef enum _CPUID_FEATURES
|
||||
CPUID_FEATURES_ECX_X2APIC = 1 << 21,
|
||||
CPUID_FEATURES_ECX_MOVBE = 1 << 22,
|
||||
CPUID_FEATURES_ECX_POPCNT = 1 << 23,
|
||||
CPUID_FEATURES_ECX_TSC = 1 << 24,
|
||||
CPUID_FEATURES_ECX_TSC_DEADLINE = 1 << 24,
|
||||
CPUID_FEATURES_ECX_AES = 1 << 25,
|
||||
CPUID_FEATURES_ECX_XSAVE = 1 << 26,
|
||||
CPUID_FEATURES_ECX_OSXSAVE = 1 << 27,
|
||||
@ -184,6 +189,92 @@ typedef enum _CPUID_FEATURES
|
||||
CPUID_FEATURES_EDX_PBE = 1 << 31
|
||||
} CPUID_FEATURES, *PCPUID_FEATURES;
|
||||
|
||||
Ghost marked this conversation as resolved
Outdated
belliash
commented
UINT, or maybe something smaller if just one bit used. UINT, or maybe something smaller if just one bit used.
![]()
Ghost
commented
Using BYTE instead of unsigned int for the bitfield Using BYTE instead of unsigned int for the bitfield
|
||||
/* CPU features, as reported by CPUID instruction */
|
||||
typedef union _CPU_FEATURES
|
||||
{
|
||||
UINT64 Value;
|
||||
union
|
||||
belliash
commented
It is not guaranteed that enum has unsigned underlying type. It is not guaranteed that enum has unsigned underlying type.
![]()
Ghost
commented
You pointed the struct in this case, was this a mis-comment? You pointed the struct in this case, was this a mis-comment?
belliash
commented
No, it's not a mistake. BOOLEAN is defined as enum. No, it's not a mistake. BOOLEAN is defined as enum.
![]()
Ghost
commented
May we define May we define `BOOLEAN as enum : BYTE`?
https://learn.microsoft.com/en-us/windows/win32/winprog/windows-data-types
![]()
Ghost
commented
Swapped all BOOLEAN on the structure with BYTE Swapped all BOOLEAN on the structure with BYTE
|
||||
{
|
||||
UINT32 Ecx;
|
||||
UINT32 ExtendedFeatures;
|
||||
struct
|
||||
{
|
||||
BYTE SSE3 : 1;
|
||||
BYTE PCLMUL : 1;
|
||||
BYTE DTES64 : 1;
|
||||
BYTE MONITOR : 1;
|
||||
BYTE DS_CPL : 1;
|
||||
BYTE VMX : 1;
|
||||
BYTE SMX : 1;
|
||||
BYTE EST : 1;
|
||||
BYTE TM2 : 1;
|
||||
BYTE SSSE3 : 1;
|
||||
BYTE CID : 1;
|
||||
BYTE SDBG : 1;
|
||||
BYTE FMA : 1;
|
||||
BYTE CX16 : 1;
|
||||
BYTE XTPR : 1;
|
||||
BYTE PDCM : 1;
|
||||
BYTE Reserved4 : 1; // Bit 16 is reserved
|
||||
BYTE PCID : 1;
|
||||
BYTE DCA : 1;
|
||||
BYTE SSE4_1 : 1;
|
||||
BYTE SSE4_2 : 1;
|
||||
BYTE X2APIC : 1;
|
||||
BYTE MOVBE : 1;
|
||||
BYTE POPCNT : 1;
|
||||
BYTE TSC_DEADLINE : 1;
|
||||
BYTE AES : 1;
|
||||
BYTE XSAVE : 1;
|
||||
BYTE OSXSAVE : 1;
|
||||
BYTE AVX : 1;
|
||||
BYTE F16C : 1;
|
||||
BYTE RDRAND : 1;
|
||||
BYTE HYPERVISOR : 1;
|
||||
};
|
||||
};
|
||||
union
|
||||
{
|
||||
UINT32 Edx;
|
||||
UINT32 Features;
|
||||
struct
|
||||
{
|
||||
BYTE FPU : 1;
|
||||
BYTE VME : 1;
|
||||
BYTE DE : 1;
|
||||
BYTE PSE : 1;
|
||||
BYTE TSC : 1;
|
||||
BYTE MSR : 1;
|
||||
BYTE PAE : 1;
|
||||
BYTE MCE : 1;
|
||||
BYTE CX8 : 1;
|
||||
BYTE APIC : 1;
|
||||
BYTE Reserved1 : 1; // Bit 10 is reserved
|
||||
BYTE SEP : 1;
|
||||
BYTE MTRR : 1;
|
||||
BYTE PGE : 1;
|
||||
BYTE MCA : 1;
|
||||
BYTE CMOV : 1;
|
||||
BYTE PAT : 1;
|
||||
BYTE PSE36 : 1;
|
||||
BYTE PSN : 1;
|
||||
BYTE CLFLUSH : 1;
|
||||
BYTE Reserved2 : 1; // Bit 20 is reserved
|
||||
BYTE DS : 1;
|
||||
BYTE ACPI : 1;
|
||||
BYTE MMX : 1;
|
||||
BYTE FXSR : 1;
|
||||
belliash
commented
Any reason for using UINT64 here? Any reason for using UINT64 here?
![]()
Ghost
commented
I've specifically added that to access the whole value AsUINT64, maybe it's unnecesary right now, but added it as a plus. I took some inspiration from this code, that does the same: I've specifically added that to access the whole value AsUINT64, maybe it's unnecesary right now, but added it as a plus. I took some inspiration from this code, that does the same:
xtchain/binaries/x86_64-w64-mingw32/include/winhvplatformdefs.h:29
(look for typedef union WHV_CAPABILITY_FEATURES)
![]()
Ghost
commented
Swapped all BOOLEAN on the structure with BYTE Swapped all BOOLEAN on the structure with BYTE
|
||||
BYTE SSE : 1;
|
||||
BYTE SSE2 : 1;
|
||||
BYTE SS : 1;
|
||||
BYTE HTT : 1;
|
||||
BYTE TM : 1;
|
||||
BYTE Reserved3 : 1; // Bit 30 is reserved
|
||||
BYTE PBE : 1;
|
||||
};
|
||||
};
|
||||
} CPU_FEATURES, *PCPU_FEATURES;
|
||||
|
||||
/* CPUID requests */
|
||||
typedef enum _CPUID_REQUESTS
|
||||
{
|
||||
|
@ -522,6 +522,7 @@ typedef struct _KPROCESSOR_CONTROL_BLOCK
|
||||
ULONG64 RspBase;
|
||||
ULONG_PTR SetMember;
|
||||
CPU_IDENTIFICATION CpuId;
|
||||
CPU_FEATURES CpuFeatures;
|
||||
KPROCESSOR_STATE ProcessorState;
|
||||
KDPC_DATA DpcData[2];
|
||||
PVOID DpcStack;
|
||||
|
@ -61,11 +61,16 @@
|
||||
#define SEGMENT_FS 0x64
|
||||
#define SEGMENT_GS 0x65
|
||||
|
||||
/* CPUID vendor names */
|
||||
#define CPUID_VENDOR_NAME_AMD "AuthenticAMD"
|
||||
#define CPUID_VENDOR_NAME_INTEL "GenuineIntel"
|
||||
|
||||
/* CPU vendor enumeration list */
|
||||
typedef enum _CPU_VENDOR
|
||||
{
|
||||
CPU_VENDOR_AMD = 0x68747541,
|
||||
belliash
commented
Why do we change the values in CPU_VENDOR enum? Why do we change the values in CPU_VENDOR enum?
![]()
Ghost
commented
The modifications were made to enhance code readability and maintainability, I specifically targeted removing magic numbers, and, to introduce a specific value (CPU_VENDOR_INVALID) to handle cases where the CPU vendor is in an invalid state. The modifications were made to enhance code readability and maintainability, I specifically targeted removing magic numbers, and, to introduce a specific value (CPU_VENDOR_INVALID) to handle cases where the CPU vendor is in an invalid state.
belliash
commented
That magic numbers have some meaning, eg: HEX(68747541) = STR(Auth) That magic numbers have some meaning, eg: HEX(68747541) = STR(Auth)
![]()
Ghost
commented
I know. Why shouldn't we change the values in CPU_VENDOR enum? I know. Why shouldn't we change the values in CPU_VENDOR enum?
|
||||
CPU_VENDOR_INTEL = 0x756e6547,
|
||||
CPU_VENDOR_INVALID,
|
||||
CPU_VENDOR_AMD,
|
||||
CPU_VENDOR_INTEL,
|
||||
CPU_VENDOR_UNKNOWN = 0xFFFFFFFF
|
||||
} CPU_VENDOR, *PCPU_VENDOR;
|
||||
|
||||
@ -95,7 +100,7 @@ typedef enum _CPUID_FEATURES
|
||||
CPUID_FEATURES_ECX_X2APIC = 1 << 21,
|
||||
CPUID_FEATURES_ECX_MOVBE = 1 << 22,
|
||||
CPUID_FEATURES_ECX_POPCNT = 1 << 23,
|
||||
CPUID_FEATURES_ECX_TSC = 1 << 24,
|
||||
CPUID_FEATURES_ECX_TSC_DEADLINE = 1 << 24,
|
||||
CPUID_FEATURES_ECX_AES = 1 << 25,
|
||||
CPUID_FEATURES_ECX_XSAVE = 1 << 26,
|
||||
CPUID_FEATURES_ECX_OSXSAVE = 1 << 27,
|
||||
@ -135,6 +140,92 @@ typedef enum _CPUID_FEATURES
|
||||
CPUID_FEATURES_EDX_PBE = 1 << 31
|
||||
} CPUID_FEATURES, *PCPUID_FEATURES;
|
||||
|
||||
Ghost marked this conversation as resolved
Outdated
belliash
commented
UINT, or maybe something smaller if just one bit used. UINT, or maybe something smaller if just one bit used.
![]()
Ghost
commented
Using BYTE instead of unsigned int for the bitfield Using BYTE instead of unsigned int for the bitfield
|
||||
/* CPU features, as reported by CPUID instruction */
|
||||
typedef union _CPU_FEATURES
|
||||
{
|
||||
UINT64 Value;
|
||||
union
|
||||
belliash
commented
It is not guaranteed that enum has unsigned underlying type. It is not guaranteed that enum has unsigned underlying type.
![]()
Ghost
commented
You pointed the struct in this case, was this a mis-comment? You pointed the struct in this case, was this a mis-comment?
|
||||
{
|
||||
UINT32 Ecx;
|
||||
UINT32 ExtendedFeatures;
|
||||
struct
|
||||
{
|
||||
BYTE SSE3 : 1;
|
||||
BYTE PCLMUL : 1;
|
||||
BYTE DTES64 : 1;
|
||||
BYTE MONITOR : 1;
|
||||
BYTE DS_CPL : 1;
|
||||
BYTE VMX : 1;
|
||||
BYTE SMX : 1;
|
||||
BYTE EST : 1;
|
||||
BYTE TM2 : 1;
|
||||
BYTE SSSE3 : 1;
|
||||
BYTE CID : 1;
|
||||
BYTE SDBG : 1;
|
||||
BYTE FMA : 1;
|
||||
BYTE CX16 : 1;
|
||||
BYTE XTPR : 1;
|
||||
BYTE PDCM : 1;
|
||||
BYTE Reserved1 : 1; // Bit 16 is reserved
|
||||
BYTE PCID : 1;
|
||||
BYTE DCA : 1;
|
||||
BYTE SSE4_1 : 1;
|
||||
BYTE SSE4_2 : 1;
|
||||
BYTE X2APIC : 1;
|
||||
BYTE MOVBE : 1;
|
||||
BYTE POPCNT : 1;
|
||||
BYTE TSC_DEADLINE : 1;
|
||||
BYTE AES : 1;
|
||||
BYTE XSAVE : 1;
|
||||
BYTE OSXSAVE : 1;
|
||||
BYTE AVX : 1;
|
||||
BYTE F16C : 1;
|
||||
BYTE RDRAND : 1;
|
||||
BYTE HYPERVISOR : 1;
|
||||
};
|
||||
};
|
||||
union
|
||||
{
|
||||
UINT32 Edx;
|
||||
UINT32 Features;
|
||||
struct
|
||||
{
|
||||
BYTE FPU : 1;
|
||||
BYTE VME : 1;
|
||||
BYTE DE : 1;
|
||||
BYTE PSE : 1;
|
||||
BYTE TSC : 1;
|
||||
BYTE MSR : 1;
|
||||
BYTE PAE : 1;
|
||||
BYTE MCE : 1;
|
||||
BYTE CX8 : 1;
|
||||
BYTE APIC : 1;
|
||||
BYTE Reserved2 : 1; // Bit 10 is reserved
|
||||
BYTE SEP : 1;
|
||||
BYTE MTRR : 1;
|
||||
BYTE PGE : 1;
|
||||
BYTE MCA : 1;
|
||||
BYTE CMOV : 1;
|
||||
BYTE PAT : 1;
|
||||
BYTE PSE36 : 1;
|
||||
BYTE PSN : 1;
|
||||
BYTE CLFLUSH : 1;
|
||||
belliash
commented
Any reason for using UINT64 here? Any reason for using UINT64 here?
![]()
Ghost
commented
I've specifically added that to access the whole value I've specifically added that to access the whole value `AsUINT64`, maybe it's unnecesary right now, but added it as a plus. I took some inspiration from this code, that does the same:
xtchain/binaries/x86_64-w64-mingw32/include/winhvplatformdefs.h:29
(look for `typedef union WHV_CAPABILITY_FEATURES`)
belliash
commented
I didn't notice the difference between i686 and amd64. In one case it is a structure containing 2 unions. Here we got 1 bit structure. Does it mean, that CPUID returns different data across archs? I didn't notice the difference between i686 and amd64. In one case it is a structure containing 2 unions. Here we got 1 bit structure. Does it mean, that CPUID returns different data across archs?
![]()
Ghost
commented
This is a small regression on local commits, I'll push the final structure. This is a small regression on local commits, I'll push the final structure.
![]()
Ghost
commented
Fixed on last commit. Fixed on last commit.
|
||||
BYTE Reserved3 : 1; // Bit 20 is reserved
|
||||
BYTE DS : 1;
|
||||
BYTE ACPI : 1;
|
||||
BYTE MMX : 1;
|
||||
BYTE FXSR : 1;
|
||||
BYTE SSE : 1;
|
||||
BYTE SSE2 : 1;
|
||||
BYTE SS : 1;
|
||||
BYTE HTT : 1;
|
||||
BYTE TM : 1;
|
||||
BYTE Reserved4 : 1; // Bit 30 is reserved
|
||||
BYTE PBE : 1;
|
||||
};
|
||||
};
|
||||
} CPU_FEATURES, *PCPU_FEATURES;
|
||||
|
||||
Ghost marked this conversation as resolved
Outdated
belliash
commented
IMHO this should go to PROCESSOR_CONTROL_BLOCK. IMHO this should go to PROCESSOR_CONTROL_BLOCK.
![]()
Ghost
commented
Moved to PROCESSOR_CONTROL_BLOCK Moved to PROCESSOR_CONTROL_BLOCK
|
||||
/* CPUID requests */
|
||||
typedef enum _CPUID_REQUESTS
|
||||
{
|
||||
|
@ -461,6 +461,7 @@ typedef struct _KPROCESSOR_CONTROL_BLOCK
|
||||
UCHAR Number;
|
||||
ULONG_PTR SetMember;
|
||||
CPU_IDENTIFICATION CpuId;
|
||||
CPU_FEATURES CpuFeatures;
|
||||
KPROCESSOR_STATE ProcessorState;
|
||||
ULONG_PTR MultiThreadProcessorSet;
|
||||
KDPC_DATA DpcData[2];
|
||||
|
@ -108,8 +108,7 @@ ArSetGdtEntryBase(IN PKGDTENTRY Gdt,
|
||||
}
|
||||
|
||||
/**
|
||||
* Identifies processor type (vendor, model, stepping) as well as looks for available CPU features and stores them
|
||||
* in Processor Control Block (PRCB).
|
||||
* Sets the CPU vendor information in the processor control block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
@ -117,29 +116,62 @@ ArSetGdtEntryBase(IN PKGDTENTRY Gdt,
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpIdentifyProcessor(VOID)
|
||||
ArpSetCpuVendor()
|
||||
{
|
||||
PKPROCESSOR_CONTROL_BLOCK Prcb;
|
||||
CPUID_REGISTERS CpuRegisters;
|
||||
CPUID_SIGNATURE CpuSignature;
|
||||
|
||||
/* Not fully implemented yet */
|
||||
UNIMPLEMENTED;
|
||||
UINT32 VendorNameBytes[3];
|
||||
|
||||
/* Get current processor control block */
|
||||
Prcb = KeGetCurrentProcessorControlBlock();
|
||||
|
||||
/* Get CPU vendor by issueing CPUID instruction */
|
||||
/* Get CPU vendor by issuing CPUID instruction */
|
||||
RtlZeroMemory(&CpuRegisters, sizeof(CPUID_REGISTERS));
|
||||
CpuRegisters.Leaf = CPUID_GET_VENDOR_STRING;
|
||||
Ghost marked this conversation as resolved
Outdated
belliash
commented
Why this needs to be so over-complicated? It should be possible to simply store data read from EBX, EDX and ECX registers. Why this needs to be so over-complicated? It should be possible to simply store data read from EBX, EDX and ECX registers.
![]()
Ghost
commented
Simplified
Simplified
```c
XTAPI
VOID ArpSetCpuVendor(IN PKPROCESSOR_CONTROL_BLOCK Prcb,
IN CPUID_REGISTERS CpuRegisters)
{
*((UINT32 *)(Prcb->CpuId.VendorName)) = CpuRegisters.Ebx;
*((UINT32 *)(Prcb->CpuId.VendorName + 4)) = CpuRegisters.Edx;
*((UINT32 *)(Prcb->CpuId.VendorName + 8)) = CpuRegisters.Ecx;
Prcb->CpuId.VendorName[12] = '\0';
}
```
|
||||
ArCpuId(&CpuRegisters);
|
||||
|
||||
/* Store CPU vendor in processor control block */
|
||||
Prcb->CpuId.Vendor = CpuRegisters.Ebx;
|
||||
Prcb->CpuId.VendorName[0] = CpuRegisters.Ebx;
|
||||
Prcb->CpuId.VendorName[4] = CpuRegisters.Edx;
|
||||
Prcb->CpuId.VendorName[8] = CpuRegisters.Ecx;
|
||||
Prcb->CpuId.VendorName[12] = '\0';
|
||||
/* Order CPU vendor name bytes */
|
||||
VendorNameBytes[0] = CpuRegisters.Ebx;
|
||||
VendorNameBytes[1] = CpuRegisters.Edx;
|
||||
VendorNameBytes[2] = CpuRegisters.Ecx;
|
||||
|
||||
/* Copy CPU vendor name to processor control block */
|
||||
RtlZeroMemory(&Prcb->CpuId.VendorName[0], 13);
|
||||
RtlCopyMemory(&Prcb->CpuId.VendorName[0], &VendorNameBytes[0], 12);
|
||||
|
||||
/* Set CPU vendor on processor control block by comparing known vendor names */
|
||||
if (RtlCompareMemory(&Prcb->CpuId.VendorName[0], CPUID_VENDOR_NAME_AMD, 12) == 12)
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_AMD;
|
||||
}
|
||||
else if (RtlCompareMemory(&Prcb->CpuId.VendorName[0], CPUID_VENDOR_NAME_INTEL, 12) == 12)
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_INTEL;
|
||||
}
|
||||
else
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_UNKNOWN;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the CPU features information in the processor control block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
* @since XT 1.0
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuFeatures(VOID)
|
||||
{
|
||||
PKPROCESSOR_CONTROL_BLOCK Prcb;
|
||||
CPUID_REGISTERS CpuRegisters;
|
||||
CPUID_SIGNATURE CpuSignature;
|
||||
UINT32 FeatureArray[2];
|
||||
Ghost marked this conversation as resolved
Outdated
belliash
commented
All these All these `ifdef ` statements are completely useless, as DebugPrint on non-debug build is just a NOP.
|
||||
|
||||
/* Get current processor control block */
|
||||
Prcb = KeGetCurrentProcessorControlBlock();
|
||||
|
||||
/* Get CPU features */
|
||||
RtlZeroMemory(&CpuRegisters, sizeof(CPUID_REGISTERS));
|
||||
@ -153,24 +185,25 @@ ArpIdentifyProcessor(VOID)
|
||||
Prcb->CpuId.Stepping = CpuSignature.Stepping;
|
||||
|
||||
/* CPU vendor specific quirks */
|
||||
if(Prcb->CpuId.Vendor == CPU_VENDOR_AMD)
|
||||
if (Prcb->CpuId.Vendor == CPU_VENDOR_AMD)
|
||||
belliash
commented
Please don't mix code changes with formatting. Please don't mix code changes with formatting.
![]()
Ghost
commented
Do you expect to just copy and paste the code from one side to another without giving it proper format? Notice it's placed on another function (Diff does not help here) Do you expect to just copy and paste the code from one side to another without giving it proper format? Notice it's placed on another function (Diff does not help here)
belliash
commented
It does not look like copied from one place into another. There is additional space between if and parenthesis. If it gets removed, git won't see any change in this place. It is placed in another function, because ArpIdentifyProcessor() got divided. It does not look like copied from one place into another. There is additional space between if and parenthesis. If it gets removed, git won't see any change in this place.
It is placed in another function, because ArpIdentifyProcessor() got divided.
![]()
Ghost
commented
In the process of moving the code from In the process of moving the code from `ArpIdentifyProcessor` to `ArpSetCpuVendor` I formatted the whole function, as it makes sense to tidy up and reformat the function I've touched.
|
||||
{
|
||||
/* AMD CPU */
|
||||
Prcb->CpuId.Family = Prcb->CpuId.Family + CpuSignature.ExtendedFamily;
|
||||
if(Prcb->CpuId.Model == 0xF)
|
||||
|
||||
if (Prcb->CpuId.Model == 0xF)
|
||||
{
|
||||
Prcb->CpuId.Model = Prcb->CpuId.Model + (CpuSignature.ExtendedModel << 4);
|
||||
}
|
||||
}
|
||||
else if(Prcb->CpuId.Vendor == CPU_VENDOR_INTEL)
|
||||
else if (Prcb->CpuId.Vendor == CPU_VENDOR_INTEL)
|
||||
{
|
||||
/* Intel CPU */
|
||||
if(Prcb->CpuId.Family == 0xF)
|
||||
if (Prcb->CpuId.Family == 0xF)
|
||||
{
|
||||
Prcb->CpuId.Family = Prcb->CpuId.Family + CpuSignature.ExtendedFamily;
|
||||
}
|
||||
|
||||
if((Prcb->CpuId.Family == 0xF) || (Prcb->CpuId.Family == 0x6))
|
||||
if ((Prcb->CpuId.Family == 0xF) || (Prcb->CpuId.Family == 0x6))
|
||||
{
|
||||
Prcb->CpuId.Model = Prcb->CpuId.Model + (CpuSignature.ExtendedModel << 4);
|
||||
}
|
||||
@ -181,7 +214,29 @@ ArpIdentifyProcessor(VOID)
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_UNKNOWN;
|
||||
}
|
||||
|
||||
/* TODO: Store a list of CPU features in processor control block */
|
||||
/* Store CPU features in processor control block */
|
||||
FeatureArray[0] = CpuRegisters.Edx;
|
||||
FeatureArray[1] = CpuRegisters.Ecx;
|
||||
RtlCopyMemory(&Prcb->CpuFeatures, &FeatureArray[0], sizeof(UINT64));
|
||||
}
|
||||
|
||||
/**
|
||||
* Identifies processor type (vendor, model, stepping) as well as looks for available CPU features and stores them
|
||||
* in Processor Control Block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
* @since XT 1.0
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpIdentifyProcessor(VOID)
|
||||
{
|
||||
/* Set CPU vendor */
|
||||
ArpSetCpuVendor();
|
||||
|
||||
/* Set CPU features */
|
||||
ArpSetCpuFeatures();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -105,8 +105,7 @@ ArSetGdtEntryBase(IN PKGDTENTRY Gdt,
|
||||
}
|
||||
|
||||
/**
|
||||
* Identifies processor type (vendor, model, stepping) as well as looks for available CPU features and stores them
|
||||
* in Processor Control Block (PRCB).
|
||||
* Sets the CPU vendor information in the processor control block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
@ -114,14 +113,11 @@ ArSetGdtEntryBase(IN PKGDTENTRY Gdt,
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpIdentifyProcessor(VOID)
|
||||
ArpSetCpuVendor()
|
||||
{
|
||||
PKPROCESSOR_CONTROL_BLOCK Prcb;
|
||||
CPUID_REGISTERS CpuRegisters;
|
||||
CPUID_SIGNATURE CpuSignature;
|
||||
|
||||
/* Not fully implemented yet */
|
||||
UNIMPLEMENTED;
|
||||
UINT32 VendorNameBytes[3];
|
||||
|
||||
/* Get current processor control block */
|
||||
Prcb = KeGetCurrentProcessorControlBlock();
|
||||
@ -131,12 +127,48 @@ ArpIdentifyProcessor(VOID)
|
||||
CpuRegisters.Leaf = CPUID_GET_VENDOR_STRING;
|
||||
ArCpuId(&CpuRegisters);
|
||||
|
||||
/* Store CPU vendor in processor control block */
|
||||
Prcb->CpuId.Vendor = CpuRegisters.Ebx;
|
||||
Prcb->CpuId.VendorName[0] = CpuRegisters.Ebx;
|
||||
Prcb->CpuId.VendorName[4] = CpuRegisters.Edx;
|
||||
Prcb->CpuId.VendorName[8] = CpuRegisters.Ecx;
|
||||
Prcb->CpuId.VendorName[12] = '\0';
|
||||
/* Fix CPU vendor name bytes order */
|
||||
VendorNameBytes[0] = CpuRegisters.Ebx;
|
||||
VendorNameBytes[1] = CpuRegisters.Edx;
|
||||
VendorNameBytes[2] = CpuRegisters.Ecx;
|
||||
|
||||
/* Copy CPU vendor name to processor control block */
|
||||
RtlZeroMemory(&Prcb->CpuId.VendorName[0], 13);
|
||||
RtlCopyMemory(&Prcb->CpuId.VendorName[0], &VendorNameBytes[0], 12);
|
||||
|
||||
/* Set CPU vendor on processor control block by comparing known vendor names */
|
||||
if (RtlCompareMemory(&Prcb->CpuId.VendorName[0], CPUID_VENDOR_NAME_AMD, 12) == 12)
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_AMD;
|
||||
}
|
||||
else if (RtlCompareMemory(&Prcb->CpuId.VendorName[0], CPUID_VENDOR_NAME_INTEL, 12) == 12)
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_INTEL;
|
||||
}
|
||||
else
|
||||
{
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_UNKNOWN;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the CPU features information in the processor control block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
* @since XT 1.0
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuFeatures(VOID)
|
||||
{
|
||||
PKPROCESSOR_CONTROL_BLOCK Prcb;
|
||||
CPUID_REGISTERS CpuRegisters;
|
||||
CPUID_SIGNATURE CpuSignature;
|
||||
UINT32 FeatureArray[2];
|
||||
|
||||
/* Get current processor control block */
|
||||
Prcb = KeGetCurrentProcessorControlBlock();
|
||||
|
||||
/* Get CPU features */
|
||||
RtlZeroMemory(&CpuRegisters, sizeof(CPUID_REGISTERS));
|
||||
@ -149,25 +181,29 @@ ArpIdentifyProcessor(VOID)
|
||||
Prcb->CpuId.Model = CpuSignature.Model;
|
||||
Prcb->CpuId.Stepping = CpuSignature.Stepping;
|
||||
|
||||
/* Prcb->CpuId.Vendor print*/
|
||||
DebugPrint(L"CPU Signature: %s\n", Prcb->CpuId.Vendor);
|
||||
|
||||
/* CPU vendor specific quirks */
|
||||
if(Prcb->CpuId.Vendor == CPU_VENDOR_AMD)
|
||||
if (Prcb->CpuId.Vendor == CPU_VENDOR_AMD)
|
||||
{
|
||||
/* AMD CPU */
|
||||
Prcb->CpuId.Family = Prcb->CpuId.Family + CpuSignature.ExtendedFamily;
|
||||
if(Prcb->CpuId.Model == 0xF)
|
||||
|
||||
if (Prcb->CpuId.Model == 0xF)
|
||||
{
|
||||
Prcb->CpuId.Model = Prcb->CpuId.Model + (CpuSignature.ExtendedModel << 4);
|
||||
}
|
||||
}
|
||||
else if(Prcb->CpuId.Vendor == CPU_VENDOR_INTEL)
|
||||
else if (Prcb->CpuId.Vendor == CPU_VENDOR_INTEL)
|
||||
{
|
||||
/* Intel CPU */
|
||||
if(Prcb->CpuId.Family == 0xF)
|
||||
if (Prcb->CpuId.Family == 0xF)
|
||||
belliash
commented
Please don't mix code changes with formatting. Please don't mix code changes with formatting.
![]()
Ghost
commented
Do you expect to just copy and paste the code from one side to another without giving it proper format? Notice it's placed on another function (Diff does not help here) Do you expect to just copy and paste the code from one side to another without giving it proper format? Notice it's placed on another function (Diff does not help here)
belliash
commented
It does not look like copied from one place into another. There is additional space between if and parenthesis. If it gets removed, git won't see any change in this place. It is placed in another function, because ArpIdentifyProcessor() got divided. It does not look like copied from one place into another. There is additional space between if and parenthesis. If it gets removed, git won't see any change in this place.
It is placed in another function, because ArpIdentifyProcessor() got divided.
![]()
Ghost
commented
In the process of moving the code from In the process of moving the code from `ArpIdentifyProcessor` to `ArpSetCpuVendor` I formatted the whole function, as it makes sense to tidy up and reformat the function I've touched.
|
||||
{
|
||||
Prcb->CpuId.Family = Prcb->CpuId.Family + CpuSignature.ExtendedFamily;
|
||||
}
|
||||
|
||||
if((Prcb->CpuId.Family == 0xF) || (Prcb->CpuId.Family == 0x6))
|
||||
if ((Prcb->CpuId.Family == 0xF) || (Prcb->CpuId.Family == 0x6))
|
||||
{
|
||||
Prcb->CpuId.Model = Prcb->CpuId.Model + (CpuSignature.ExtendedModel << 4);
|
||||
}
|
||||
@ -178,7 +214,29 @@ ArpIdentifyProcessor(VOID)
|
||||
Prcb->CpuId.Vendor = CPU_VENDOR_UNKNOWN;
|
||||
}
|
||||
|
||||
/* TODO: Store a list of CPU features in processor control block */
|
||||
/* Store CPU features in processor control block */
|
||||
FeatureArray[0] = CpuRegisters.Edx;
|
||||
FeatureArray[1] = CpuRegisters.Ecx;
|
||||
RtlCopyMemory(&Prcb->CpuFeatures, &FeatureArray[0], sizeof(UINT64));
|
||||
}
|
||||
|
||||
/**
|
||||
* Identifies processor type (vendor, model, stepping) as well as looks for available CPU features and stores them
|
||||
* in Processor Control Block (PRCB).
|
||||
*
|
||||
* @return This routine does not return any value.
|
||||
*
|
||||
* @since XT 1.0
|
||||
*/
|
||||
XTAPI
|
||||
VOID
|
||||
ArpIdentifyProcessor(VOID)
|
||||
{
|
||||
/* Set CPU vendor */
|
||||
ArpSetCpuVendor();
|
||||
|
||||
/* Set CPU features */
|
||||
ArpSetCpuFeatures();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -288,4 +288,12 @@ ArpSetIdtGate(IN PKIDTENTRY Idt,
|
||||
IN USHORT Ist,
|
||||
IN USHORT Access);
|
||||
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuVendor(VOID);
|
||||
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuFeatures(VOID);
|
||||
|
||||
#endif /* __XTOSKRNL_AMD64_AR_H */
|
||||
|
@ -284,4 +284,12 @@ XTAPI
|
||||
VOID
|
||||
ArpSetNonMaskableInterruptTssEntry(IN PKPROCESSOR_BLOCK ProcessorBlock);
|
||||
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuVendor(VOID);
|
||||
|
||||
XTAPI
|
||||
VOID
|
||||
ArpSetCpuFeatures(VOID);
|
||||
|
||||
#endif /* __XTOSKRNL_I686_AR_H */
|
||||
|
Why do we change the values in CPU_VENDOR enum?
The modifications were made to enhance code readability and maintainability, I specifically targeted removing magic numbers, and, to introduce a specific value (CPU_VENDOR_INVALID) to handle cases where the CPU vendor is in an invalid state.
That magic numbers have some meaning: HEX(68747541) = STR(Auth)
I know. Why shouldn't we change the values in CPU_VENDOR enum?