public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jeremy Linton <jeremy.linton@arm.com>
To: "Huangming (Mark)" <huangming23@huawei.com>,
	Ming Huang <heyi.guo@linaro.org>,
	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
Subject: Re: [PATCH edk2-platforms v2 02/15] Hisilicon/D05: Add PPTT support
Date: Fri, 2 Feb 2018 10:17:05 -0600	[thread overview]
Message-ID: <138e348d-f7b6-d27f-8ee0-df1d787e3c99@arm.com> (raw)
In-Reply-To: <90cab141-5776-7168-4219-fae6c1e7ea1f@huawei.com>

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,


  reply	other threads:[~2018-02-02 16:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26  8:00 [PATCH edk2-platforms v2 00/15] Improve D0x platforms and bug fix Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 01/15] Hisilicon/D05: Move Madt definition to head file Ming Huang
2018-01-30 14:27   ` graeme.gregory
2018-01-26  8:00 ` [PATCH edk2-platforms v2 02/15] Hisilicon/D05: Add PPTT support Ming Huang
2018-01-30 14:28   ` graeme.gregory
2018-02-01  1:11   ` Jeremy Linton
2018-02-02  3:42     ` Huangming (Mark)
2018-02-02 16:17       ` Jeremy Linton [this message]
2018-01-26  8:00 ` [PATCH edk2-platforms v2 03/15] Hisilicon/D0x/BDS: Switch to Generic BDS driver Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 04/15] Hisilicon/D0x: Break BMC SetBoot option out into separate library Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 05/15] Hisilicon D03/D05: Add capsule upgrade support Ming Huang
2018-01-29 19:58   ` Leif Lindholm
2018-01-30 12:48     ` Huangming (Mark)
2018-01-30 13:21       ` Leif Lindholm
2018-01-31  1:14         ` Huangming (Mark)
2018-01-30 12:47   ` Ard Biesheuvel
2018-01-26  8:00 ` [PATCH edk2-platforms v2 06/15] Hisilicon D03/D05: Open SasPlatform source code Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 07/15] Hisilicon D03/D05: Open SnpPlatform " Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 08/15] Hilisicon: Change DmaLib to CoherentDmaLib Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 09/15] Hisilicon/Smbios: Indicate use of ProcessorFamily2 in type 4 table Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 10/15] Hisilicon/PCIe: Disable PCIe ASPM Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 11/15] Hisilicon/D05: Replace SP805Watchdog by WatchdogTimer driver Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 12/15] Hisilicon/D03: " Ming Huang
2018-01-26  8:00 ` [PATCH edk2-platforms v2 13/15] Hisilicon/D05/ACPI: Add ITS PXM Ming Huang
2018-01-30 14:30   ` graeme.gregory
2018-01-26  8:00 ` [PATCH edk2-platforms v2 14/15] Hisilicon/D05/ACPI: Add Pcie, HNS and SAS PXM Ming Huang
2018-01-30 14:31   ` graeme.gregory
2018-01-26  8:00 ` [PATCH edk2-platforms v2 15/15] Hisilicon D03/D05: Update firmware version to 18.02 Ming Huang
2018-01-29 17:33 ` [PATCH edk2-platforms v2 00/15] Improve D0x platforms and bug fix Leif Lindholm
2018-02-03  7:50   ` Huangming (Mark)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=138e348d-f7b6-d27f-8ee0-df1d787e3c99@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox