Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, EricR Lai.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 )
Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/51374/comment/860239ec_96049de5
PS8, Line 532: mov %al, %cl
: mov $0x01, %eax
: shl %cl, %eax
: subl $0x01, %eax /* contains ((1 << SFWayCnt) - 1) */
: mov %bl, %cl
: mov $0x01, %ebx
: shl %cl, %ebx
: subl $0x01, %ebx /* contains ((1 << data_ways) - 1) */
: sub %ebx, %eax /* contains SF mask */
> `edx` is also free at this point, so I think that could be used instead of backing of `ebx` and usin […]
subl %ecx, %edi /* ecx == SFWayCnt - data_ways */
won't edi will get impacted here ? and then line 548 would have wrong restoration ?
Can you please take a look into below code, we don't need edx as well looks like and in this process edi also not getting tampered. Also like 523 and 549 also not needed may be.
ebx == non-eviction mask
ecx == number of ways
mov %ecx, %edi /* edi == data_ways */
mov $IA32_SF_QOS_INFO, %ecx /* ecx == IA32_SF_QOS_INFO */
rdmsr
and $0x3f, %eax /* eax == SFWayCnt */
mov %eax, %ecx /* ecx == eax */
sub %edi, %ecx /* ecx == SFWayCnt - data_ways */
movl $0x01, %eax /* eax == 1 */
shl $cl, %eax /* eax == 1 << (SFWayCnt - data_ways) */
subl $0x01, %eax /* eax == (1 << (SFWayCnt - data_ways)) - 1) */
mov %edi, %ecx /* ecx == data_ways */
shl %cl, %eax /* eax == ((1 << (SFWayCnt - data_ways)) - 1)) << data_ways */
--
To view, visit https://review.coreboot.org/c/coreboot/+/51374
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941
Gerrit-Change-Number: 51374
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 18:46:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sathyanarayana Nujella, Anil Kumar K, Selma Bensaid.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55368 )
Change subject: mb/intel/adlrvp: Add probed fw_configs to SMBIOS OEM strings
......................................................................
Patch Set 4:
(2 comments)
File src/mainboard/intel/adlrvp/mainboard.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-123593):
https://review.coreboot.org/c/coreboot/+/55368/comment/41f9a7b7_0c2b8272
PS4, Line 56: }
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-123593):
https://review.coreboot.org/c/coreboot/+/55368/comment/56cca60b_9c4ef5b3
PS4, Line 56: }
please, no spaces at the start of a line
--
To view, visit https://review.coreboot.org/c/coreboot/+/55368
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I05887d9d654eae6d9d2da731d8ab4cf4a05c287f
Gerrit-Change-Number: 55368
Gerrit-PatchSet: 4
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Jairaj Arava <jairaj.arava(a)intel.com>
Gerrit-Reviewer: Sathyanarayana Nujella <sathyanarayana.nujella(a)intel.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sathyanarayana Nujella <sathyanarayana.nujella(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 18:38:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Arthur Heymans.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56151 )
Change subject: cpu/x86/mtrr: Add check_var_mtrr_range_enabled() helper function
......................................................................
Patch Set 1:
(1 comment)
File src/cpu/x86/mtrr/earlymtrr.c:
https://review.coreboot.org/c/coreboot/+/56151/comment/52c7a78f_0bc1b7f8
PS1, Line 168: bool check_var_mtrr_range_enabled(uintptr_t base, size_t size)
> > This function relies on the caller to align base and size of the address range you want to check. […]
>> It can also be the case that your input range spans multiple MTRRs.
Yes, you are right, as it can split into multiple MTRRs, hence searching could be difficult, i mean i can put something like this.
caller passing base <= MTRR base address and (base + size) <= (MTRR base + MTRR size) => this would help me to get if address range belongs to a single MTRR or nor.
but if (base + size) > (MTRR base + MTRR size) then finding the extra size and calculating a new base and recursively calling into next MTRR, won't that be little complicated?
>> You also don't want to make assumptions on the MTRR default caching type: read it out first
yes, i will add a check, i missed that and i realized it later, if this range itself is UC then no point running this loop.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56151
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0df698c1501ad678ec0d58e133d7e437b658f4ca
Gerrit-Change-Number: 56151
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 07 Jul 2021 18:21:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Arthur Heymans, Patrick Rudolph, EricR Lai.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55653 )
Change subject: soc/intel/common: Add cpu_fill_code_cache() to test eNEM
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> How exactly does eNEM work? […]
You can find details here https://software.intel.com/content/www/us/en/develop/articles/introduction-…
--
To view, visit https://review.coreboot.org/c/coreboot/+/55653
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieb1e644f154b8f8d8da41ba8084e99827e7eff23
Gerrit-Change-Number: 55653
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 18:15:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment