Re: [edk2-devel] [PATCH v5 1/2] MdePkg:Implement RISCV CMO

["Laszlo Ersek" <lersek@redhat.com>]

On 10/17/23 16:22, Laszlo Ersek wrote:
> On 10/17/23 14:17, Dhaval Sharma wrote:
>> Implementing code to support Cache Management Operations
>> (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs

(28) Please do not abbreviate RISC-V as "RV". It's incredibly confusing.

(29) Inconsistent spelling in the patch subject: "RISCV CMO". Should be

  RISC-V cache management operations

>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index ac54338089e8..2d06cf46b1ca 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -2399,6 +2399,13 @@ [PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
>>    # @Prompt CPU Rng algorithm's GUID.
>>    gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00000037
>>
>> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64]
>> +  #
>> +  # Configurability to override RV CPU Features

(30) ditto; should be RISC-V

>> +  # BIT 0 = CMO
>> +  #
>> +  gEfiMdePkgTokenSpaceGuid.PcdRVFeatureOverride|0x1|UINT64|0x69

(31) ditto, should be PcdRiscVFeatureOverride

>> +#define RV64_CACHE_BLOCK_SIZE       64
>> +#define RV_CPU_FEATURE_CMO_BITMASK  0x1

(32) Total inconsistency, RV64_ versus RV_.

Edk2 only supports RISCV64, so "64" in the prefix is meaningless. Both
prefixes should be RISCV_.

>> +RiscvIsCMOEnabled (

(33) Should be RiscVIsCMOEnabled (upper case V).

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109688): https://edk2.groups.io/g/devel/message/109688
Mute This Topic: https://groups.io/mt/102016148/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-