From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=jeremy.linton@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id AEB7F223CCEE6 for ; Fri, 2 Feb 2018 08:11:29 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 45BDF80D; Fri, 2 Feb 2018 08:17:07 -0800 (PST) Received: from [192.168.100.241] (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 828E23F24D; Fri, 2 Feb 2018 08:17:06 -0800 (PST) To: "Huangming (Mark)" , Ming Huang , leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, graeme.gregory@linaro.org Cc: ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com, wanghuiqiang@huawei.com, guoheyi@huawei.com, waip23@126.com, mengfanrong@huawei.com, huangdaode@hisilicon.com References: <1516953650-57980-1-git-send-email-huangming23@huawei.com> <1516953650-57980-3-git-send-email-huangming23@huawei.com> <2cdbdf0d-6a37-cd13-ab4a-d2c0d976a282@arm.com> <90cab141-5776-7168-4219-fae6c1e7ea1f@huawei.com> From: Jeremy Linton Message-ID: <138e348d-f7b6-d27f-8ee0-df1d787e3c99@arm.com> Date: Fri, 2 Feb 2018 10:17:05 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <90cab141-5776-7168-4219-fae6c1e7ea1f@huawei.com> Subject: Re: [PATCH edk2-platforms v2 02/15] Hisilicon/D05: Add PPTT support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Feb 2018 16:11:30 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, On 02/01/2018 09:42 PM, Huangming (Mark) wrote: > > > On 2018/2/1 9:11, Jeremy Linton wrote: >> Hi, >> >> >> On 01/26/2018 02:00 AM, Ming Huang wrote: >>> Add Processor Properties Topology Table, PPTT include (trimming) >>> +STATIC >>> +VOID >>> +InitCacheInfo ( >>> + VOID >>> + ) >>> +{ >>> + UINT8 Index; >>> + EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES Type1Attributes; >>> + CSSELR_DATA CsselrData; >>> + CCSIDR_DATA CcsidrData; >>> + >>> + for (Index = 0; Index < PPTT_CACHE_NO; Index++) { >>> + CsselrData.Data = 0; >>> + CcsidrData.Data = 0; >>> + SetMem ( >>> + &Type1Attributes, >>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES), >>> + 0 >>> + ); >>> + >>> + if (Index == 0) { //L1I >>> + CsselrData.Bits.InD = 1; >>> + CsselrData.Bits.Level = 0; >>> + Type1Attributes.CacheType = 1; >>> + } else if (Index == 1) { >>> + Type1Attributes.CacheType = 0; >>> + CsselrData.Bits.Level = Index - 1; >>> + } else { >>> + Type1Attributes.CacheType = 2; >>> + CsselrData.Bits.Level = Index - 1; >>> + } >>> + >>> + CcsidrData.Data = ReadCCSIDR (CsselrData.Data); >>> + >>> + if (CcsidrData.Bits.Wa == 1) { >>> + Type1Attributes.AllocationType = PPTT_TYPE1_ALLOCATION_WRITE; >>> + if (CcsidrData.Bits.Ra == 1) { >>> + Type1Attributes.AllocationType = PPTT_TYPE1_ALLOCATION_READ_WRITE; >>> + } >>> + } >>> + >>> + if (CcsidrData.Bits.Wt == 1) { >>> + Type1Attributes.WritePolicy = 1; Note a few cases where you have mixed PPTT #define definitions for some of the fields (AllocateType, WritePolicy, CacheType) with numeric values. >>> + } >>> + DEBUG ((DEBUG_INFO, >>> + "[Acpi PPTT] Level = %x!CcsidrData = %x!\n", >>> + CsselrData.Bits.Level, >>> + CcsidrData.Data)); >>> + >>> + mPpttCacheType1[Index].Type = EFI_ACPI_6_2_PPTT_TYPE_CACHE; >>> + mPpttCacheType1[Index].Length = sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE); >>> + mPpttCacheType1[Index].Reserved[0] = 0; >>> + mPpttCacheType1[Index].Reserved[1] = 0; >>> + mPpttCacheType1[Index].Flags.SizePropertyValid = 1; >>> + mPpttCacheType1[Index].Flags.NumberOfSetsValid = 1; >>> + mPpttCacheType1[Index].Flags.AssociativityValid = 1; >>> + mPpttCacheType1[Index].Flags.AllocationTypeValid = 1; >>> + mPpttCacheType1[Index].Flags.CacheTypeValid = 1; >>> + mPpttCacheType1[Index].Flags.WritePolicyValid = 1; >>> + mPpttCacheType1[Index].Flags.LineSizeValid = 1; >>> + mPpttCacheType1[Index].Flags.Reserved = 0; >>> + mPpttCacheType1[Index].NextLevelOfCache = 0; >>> + >>> + if (Index != PPTT_CACHE_NO - 1) { >>> + mPpttCacheType1[Index].NumberOfSets = (UINT16)CcsidrData.Bits.NumSets + 1; >>> + mPpttCacheType1[Index].Associativity = (UINT16)CcsidrData.Bits.Associativity + 1; >>> + mPpttCacheType1[Index].LineSize = (UINT16)( 1 << (CcsidrData.Bits.LineSize + 4)); >>> + mPpttCacheType1[Index].Size = mPpttCacheType1[Index].LineSize * \ >>> + mPpttCacheType1[Index].Associativity * \ >>> + mPpttCacheType1[Index].NumberOfSets; >>> + CopyMem ( >>> + &mPpttCacheType1[Index].Attributes, >>> + &Type1Attributes, >>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES) >>> + ); >>> + } else { >>> + // L3 cache >>> + mPpttCacheType1[Index].Size = 0x1000000; // 16m >>> + mPpttCacheType1[Index].NumberOfSets = 0x2000; >>> + mPpttCacheType1[Index].Associativity = 0x10; // CacheAssociativity16Way >>> + SetMem ( >>> + &mPpttCacheType1[Index].Attributes, >>> + sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES), >>> + 0x0A >>> + ); >>> + mPpttCacheType1[Index].LineSize = 0x80; // 128byte >>> + } >>> + } (trimming) >>> +#define PPTT_TYPE0_SOCKET_FLAG PPTT_TYPE0_PHYSICAL_PKG >>> +#define PPTT_TYPE0_SCCL_FLAG 0 >>> +#define PPTT_TYPE0_CLUSTER_FLAG 0 >>> +#define PPTT_TYPE0_CORE_FLAG PPTT_TYPE0_PROCESSORID_VALID >>> + >>> +#define PPTT_TYPE1_ALLOCATION_WRITE 0x1 >>> +#define PPTT_TYPE1_ALLOCATION_READ_WRITE 0x2 >> >> Its more clear for these two, they should be in the acpi header file. While your at it the write policy and cache type should also probably be defined and used in your init routing. >> >> > > I plan to move these two macro to Acpi62.h. > > "While your at it the write policy and cache type should also probably be defined and used in your init routing" > I don't really understand the mean above. I was simply suggesting that you define/use new #defines to describe acpi standardized values you place in the policy and type (and any more you think might be helpful) fields. For example, I found it a bit odd that you defined PPTT_TYPE1_ALLOCATION_* but not 'PPTT_TYPE1_WRITE_POLICY_*' Thanks,