Add CPU features and fix CPU vendor #2

Kapalı
Ghost (silindi): prcb-cpu-features içindeki 5 işlemeyi master ile birleştirmek istiyor
8 değiştirilmiş dosya ile 337 ekleme ve 45 silme
Sadece a270c08dcf işlemesindeki değişiklikler gösteriliyor - Tüm işlemeleri göster

Dosyayı Görüntüle

@@ -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,
Eskimiş
Gözden Geçir

Why do we change the values in CPU_VENDOR enum?

Why do we change the values in CPU_VENDOR enum?
Eskimiş
Gözden Geçir

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.
Eskimiş
Gözden Geçir

That magic numbers have some meaning: HEX(68747541) = STR(Auth)

That magic numbers have some meaning: HEX(68747541) = STR(Auth)
Eskimiş
Gözden Geçir

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;
@@ -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,84 @@ typedef enum _CPUID_FEATURES
CPUID_FEATURES_EDX_PBE = 1 << 31
} CPUID_FEATURES, *PCPUID_FEATURES;
Ghost bu konuşmayı çözümlenmiş olarak işaretledi Eskimiş
Eskimiş
Gözden Geçir

UINT, or maybe something smaller if just one bit used.

UINT, or maybe something smaller if just one bit used.
Eskimiş
Gözden Geçir

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 struct _CPU_FEATURES {
union {
struct {
BOOLEAN SSE3 : 1;
Eskimiş
Gözden Geçir

It is not guaranteed that enum has unsigned underlying type.

It is not guaranteed that enum has unsigned underlying type.
Eskimiş
Gözden Geçir

You pointed the struct in this case, was this a mis-comment?

You pointed the struct in this case, was this a mis-comment?
Eskimiş
Gözden Geçir

No, it's not a mistake. BOOLEAN is defined as enum.

No, it's not a mistake. BOOLEAN is defined as enum.
Eskimiş
Gözden Geçir
May we define `BOOLEAN as enum : BYTE`? https://learn.microsoft.com/en-us/windows/win32/winprog/windows-data-types
Eskimiş
Gözden Geçir

Swapped all BOOLEAN on the structure with BYTE

Swapped all BOOLEAN on the structure with BYTE
BOOLEAN PCLMUL : 1;
BOOLEAN DTES64 : 1;
BOOLEAN MONITOR : 1;
BOOLEAN DS_CPL : 1;
BOOLEAN VMX : 1;
BOOLEAN SMX : 1;
BOOLEAN EST : 1;
BOOLEAN TM2 : 1;
BOOLEAN SSSE3 : 1;
BOOLEAN CID : 1;
BOOLEAN SDBG : 1;
BOOLEAN FMA : 1;
BOOLEAN CX16 : 1;
BOOLEAN XTPR : 1;
BOOLEAN PDCM : 1;
BOOLEAN Reserved1 : 1; // Bit 16 is reserved
BOOLEAN PCID : 1;
BOOLEAN DCA : 1;
BOOLEAN SSE4_1 : 1;
BOOLEAN SSE4_2 : 1;
BOOLEAN X2APIC : 1;
BOOLEAN MOVBE : 1;
BOOLEAN POPCNT : 1;
BOOLEAN TSC_DEADLINE : 1;
BOOLEAN AES : 1;
BOOLEAN XSAVE : 1;
BOOLEAN OSXSAVE : 1;
BOOLEAN AVX : 1;
BOOLEAN F16C : 1;
BOOLEAN RDRAND : 1;
BOOLEAN HYPERVISOR : 1;
};
UINT32 ExtendedFeatures;
};
union {
struct {
BOOLEAN FPU : 1;
BOOLEAN VME : 1;
BOOLEAN DE : 1;
BOOLEAN PSE : 1;
BOOLEAN TSC : 1;
BOOLEAN MSR : 1;
BOOLEAN PAE : 1;
BOOLEAN MCE : 1;
BOOLEAN CX8 : 1;
BOOLEAN APIC : 1;
BOOLEAN Reserved2 : 1; // Bit 10 is reserved
BOOLEAN SEP : 1;
BOOLEAN MTRR : 1;
BOOLEAN PGE : 1;
BOOLEAN MCA : 1;
BOOLEAN CMOV : 1;
BOOLEAN PAT : 1;
BOOLEAN PSE36 : 1;
BOOLEAN PSN : 1;
BOOLEAN CLFLUSH : 1;
BOOLEAN Reserved3 : 1; // Bit 20 is reserved
BOOLEAN DS : 1;
BOOLEAN ACPI : 1;
BOOLEAN MMX : 1;
BOOLEAN FXSR : 1;
BOOLEAN SSE : 1;
BOOLEAN SSE2 : 1;
BOOLEAN SS : 1;
BOOLEAN HTT : 1;
BOOLEAN TM : 1;
BOOLEAN Reserved4 : 1; // Bit 30 is reserved
BOOLEAN PBE : 1;
};
UINT64 Features;
Eskimiş
Gözden Geçir

Any reason for using UINT64 here?

Any reason for using UINT64 here?
Eskimiş
Gözden Geçir

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)

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)
Eskimiş
Gözden Geçir

Swapped all BOOLEAN on the structure with BYTE

Swapped all BOOLEAN on the structure with BYTE
};
} CPU_FEATURES, *PCPU_FEATURES;
/* CPUID requests */
typedef enum _CPUID_REQUESTS
{

Dosyayı Görüntüle

@@ -514,6 +514,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;

Dosyayı Görüntüle

@@ -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,
Eskimiş
Gözden Geçir

Why do we change the values in CPU_VENDOR enum?

Why do we change the values in CPU_VENDOR enum?
Eskimiş
Gözden Geçir

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.
Eskimiş
Gözden Geçir

That magic numbers have some meaning, eg: HEX(68747541) = STR(Auth)

That magic numbers have some meaning, eg: HEX(68747541) = STR(Auth)
Eskimiş
Gözden Geçir

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,79 @@ typedef enum _CPUID_FEATURES
CPUID_FEATURES_EDX_PBE = 1 << 31
} CPUID_FEATURES, *PCPUID_FEATURES;
Ghost bu konuşmayı çözümlenmiş olarak işaretledi Eskimiş
Eskimiş
Gözden Geçir

UINT, or maybe something smaller if just one bit used.

UINT, or maybe something smaller if just one bit used.
Eskimiş
Gözden Geçir

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 struct _CPU_FEATURES {
union {
struct {
BOOLEAN SSE3 : 1;
Eskimiş
Gözden Geçir

It is not guaranteed that enum has unsigned underlying type.

It is not guaranteed that enum has unsigned underlying type.
Eskimiş
Gözden Geçir

You pointed the struct in this case, was this a mis-comment?

You pointed the struct in this case, was this a mis-comment?
BOOLEAN PCLMUL : 1;
BOOLEAN DTES64 : 1;
BOOLEAN MONITOR : 1;
BOOLEAN DS_CPL : 1;
BOOLEAN VMX : 1;
BOOLEAN SMX : 1;
BOOLEAN EST : 1;
BOOLEAN TM2 : 1;
BOOLEAN SSSE3 : 1;
BOOLEAN CID : 1;
BOOLEAN SDBG : 1;
BOOLEAN FMA : 1;
BOOLEAN CX16 : 1;
BOOLEAN XTPR : 1;
BOOLEAN PDCM : 1;
BOOLEAN Reserved1 : 1; // Bit 16 is reserved
BOOLEAN PCID : 1;
BOOLEAN DCA : 1;
BOOLEAN SSE4_1 : 1;
BOOLEAN SSE4_2 : 1;
BOOLEAN X2APIC : 1;
BOOLEAN MOVBE : 1;
BOOLEAN POPCNT : 1;
BOOLEAN TSC_DEADLINE : 1;
BOOLEAN AES : 1;
BOOLEAN XSAVE : 1;
BOOLEAN OSXSAVE : 1;
BOOLEAN AVX : 1;
BOOLEAN F16C : 1;
BOOLEAN RDRAND : 1;
BOOLEAN HYPERVISOR : 1;
BOOLEAN FPU : 1;
BOOLEAN VME : 1;
BOOLEAN DE : 1;
BOOLEAN PSE : 1;
BOOLEAN TSC : 1;
BOOLEAN MSR : 1;
BOOLEAN PAE : 1;
BOOLEAN MCE : 1;
BOOLEAN CX8 : 1;
BOOLEAN APIC : 1;
BOOLEAN Reserved2 : 1; // Bit 10 is reserved
BOOLEAN SEP : 1;
BOOLEAN MTRR : 1;
BOOLEAN PGE : 1;
BOOLEAN MCA : 1;
BOOLEAN CMOV : 1;
BOOLEAN PAT : 1;
BOOLEAN PSE36 : 1;
BOOLEAN PSN : 1;
BOOLEAN CLFLUSH : 1;
BOOLEAN Reserved3 : 1; // Bit 20 is reserved
BOOLEAN DS : 1;
BOOLEAN ACPI : 1;
BOOLEAN MMX : 1;
BOOLEAN FXSR : 1;
BOOLEAN SSE : 1;
BOOLEAN SSE2 : 1;
BOOLEAN SS : 1;
BOOLEAN HTT : 1;
BOOLEAN TM : 1;
BOOLEAN Reserved4 : 1; // Bit 30 is reserved
BOOLEAN PBE : 1;
};
UINT64 AsUINT64;
Eskimiş
Gözden Geçir

Any reason for using UINT64 here?

Any reason for using UINT64 here?
Eskimiş
Gözden Geçir

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)

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`)
Eskimiş
Gözden Geçir

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?
Eskimiş
Gözden Geçir

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.
Eskimiş
Gözden Geçir

Fixed on last commit.

Fixed on last commit.
};
} CPU_FEATURES, *PCPU_FEATURES;
/* CPUID requests */
typedef enum _CPUID_REQUESTS
{

Dosyayı Görüntüle

@@ -454,6 +454,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];

Dosyayı Görüntüle

@@ -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 bu konuşmayı çözümlenmiş olarak işaretledi Eskimiş
Eskimiş
Gözden Geçir

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.
Eskimiş
Gözden Geçir

Simplified

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';
}
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 bu konuşmayı çözümlenmiş olarak işaretledi Eskimiş
Eskimiş
Gözden Geçir

All these ifdef statements are completely useless, as DebugPrint on non-debug build is just a NOP.

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)
Gözden Geçir

Please don't mix code changes with formatting.

Please don't mix code changes with formatting.
Gözden Geçir

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)
Gözden Geçir

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.
Gözden Geçir

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.

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();
}
/**

Dosyayı Görüntüle

@@ -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)
Gözden Geçir

Please don't mix code changes with formatting.

Please don't mix code changes with formatting.
Gözden Geçir

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)
Gözden Geçir

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.
Gözden Geçir

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.

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();
}
/**

Dosyayı Görüntüle

@@ -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 */

Dosyayı Görüntüle

@@ -284,4 +284,12 @@ XTAPI
VOID
ArpSetNonMaskableInterruptTssEntry(IN PKPROCESSOR_BLOCK ProcessorBlock);
XTAPI
VOID
ArpSetCpuVendor(VOID);
XTAPI
VOID
ArpSetCpuFeatures(VOID);
#endif /* __XTOSKRNL_I686_AR_H */