Messages in this thread | | | Subject | Re: [PATCH] ACPI/HMAT: Fix the parsing of Cache Associativity and Write Policy | From | Tao Xu <> | Date | Tue, 10 Dec 2019 16:19:55 +0800 |
| |
On 12/10/2019 4:06 PM, Rafael J. Wysocki wrote: > On Tue, Dec 10, 2019 at 2:04 AM Tao Xu <tao3.xu@intel.com> wrote: >> >> On 12/9/2019 6:01 PM, Rafael J. Wysocki wrote: >>> On Mon, Dec 2, 2019 at 8:03 AM Tao Xu <tao3.xu@intel.com> wrote: >>>> >>>> In chapter 5.2.27.5, Table 5-147: Field "Cache Attributes" of >>>> ACPI 6.3 spec: 0 is "None", 1 is "Direct Mapped", 2 is "Complex Cache >>>> Indexing" for Cache Associativity; 0 is "None", 1 is "Write Back", >>>> 2 is "Write Through" for Write Policy. >>> >>> Well, I'm not sure what the connection between the above statement, >>> which is correct AFAICS, and the changes made by the patch is. >>> >>> Is that the *_OTHER symbol names are confusing or something deeper? >>> >> >> Because in include/acpi/actbl1.h: >> >> #define ACPI_HMAT_CA_NONE (0) >> >> ACPI_HMAT_CA_NONE is 0, but in include/linux/node.h: >> >> enum cache_indexing { >> NODE_CACHE_DIRECT_MAP, >> NODE_CACHE_INDEXED, >> NODE_CACHE_OTHER, >> }; >> NODE_CACHE_OTHER is 2, and for otner enum: >> >> case ACPI_HMAT_CA_DIRECT_MAPPED: >> tcache->cache_attrs.indexing = NODE_CACHE_DIRECT_MAP; >> break; >> case ACPI_HMAT_CA_COMPLEX_CACHE_INDEXING: >> tcache->cache_attrs.indexing = NODE_CACHE_INDEXED; >> break; >> in include/acpi/actbl1.h: >> >> #define ACPI_HMAT_CA_DIRECT_MAPPED (1) >> #define ACPI_HMAT_CA_COMPLEX_CACHE_INDEXING (2) >> >> but in include/linux/node.h: >> >> NODE_CACHE_DIRECT_MAP is 0, NODE_CACHE_INDEXED is 1. This is incorrect. > > Why is it incorrect?
Sorry I paste the wrong pre-define.
This is the incorrect line:
case ACPI_HMAT_CA_DIRECT_MAPPED: tcache->cache_attrs.indexing = NODE_CACHE_DIRECT_MAP;
ACPI_HMAT_CA_DIRECT_MAPPED is 1, NODE_CACHE_DIRECT_MAP is 0. That means if HMAT table input 1 for cache_attrs.indexing, kernel store 0 in cache_attrs.indexing. But in ACPI 6.3, 0 means "None". So for the whole switch codes:
switch ((attrs & ACPI_HMAT_CACHE_ASSOCIATIVITY) >> 8) { case ACPI_HMAT_CA_DIRECT_MAPPED(1): tcache->cache_attrs.indexing = NODE_CACHE_DIRECT_MAP(0); break; case ACPI_HMAT_CA_COMPLEX_CACHE_INDEXING(2): tcache->cache_attrs.indexing = NODE_CACHE_INDEXED(1); break; case ACPI_HMAT_CA_NONE(0): default: tcache->cache_attrs.indexing = NODE_CACHE_OTHER(2); break; }
| |