Attention is currently required from: Furquan Shaikh, Martin Roth, Julius Werner, Kyösti Mälkki.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55581 )
Change subject: util/cbfstool: Remove unused pagesize parameter
......................................................................
Patch Set 5:
(1 comment)
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/55581/comment/f1193b14_c5cf7cce
PS3, Line 545: uint32_t pagesize = 1;
> > I think this reduces to: […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/55581
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a
Gerrit-Change-Number: 55581
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Jul 2021 16:01:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Julius Werner, Kyösti Mälkki.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/55581
to look at the new patch set (#5).
Change subject: util/cbfstool: Remove unused pagesize parameter
......................................................................
util/cbfstool: Remove unused pagesize parameter
Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M util/cbfstool/cbfstool.c
1 file changed, 5 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/55581/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/55581
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib672ba8ed418b1a76e4a48951eabda6923358e7a
Gerrit-Change-Number: 55581
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newpatchset
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 5:
(1 comment)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/51374/comment/1b0d2499_3ca8a267
PS5, Line 524: (1 << data_ways)
> I don't see this calculation anywhere... below I see […]
i guess better we accommodate this 1 << data_ways in below code as well?
--
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: 5
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: Thu, 01 Jul 2021 15:46:31 +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: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai.
Tim Wawrzynczak 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 5:
(2 comments)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/51374/comment/62e7d6bb_9fe7cbde
PS5, Line 520: mov %ecx, %edi
suggestion:
`/* save number of ways to edi because rdmsr has to use ecx */`
https://review.coreboot.org/c/coreboot/+/51374/comment/12fe02ae_4df7eb7a
PS5, Line 524: (1 << data_ways)
I don't see this calculation anywhere... below I see
`SF_MASK_1 = (1 << (IA32_SF_QOS_INFO & 0x3f)) - 1`
or
`SF_MASK_1 = (1 << SFWayCnt) - 1)`
--
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: 5
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: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 01 Jul 2021 15:38:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Varshit B Pandya, Maulik V Vaghela, Rizwan Qureshi, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55998 )
Change subject: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/55998
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I41858ea156c8258ea0e7be9e2f67fb0e24144c80
Gerrit-Change-Number: 55998
Gerrit-PatchSet: 2
Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 01 Jul 2021 15:29:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Varshit B Pandya, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55998 )
Change subject: soc/intel/alderlake: Correct Bus and Device of Touch Host Controller
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/55998
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I41858ea156c8258ea0e7be9e2f67fb0e24144c80
Gerrit-Change-Number: 55998
Gerrit-PatchSet: 2
Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 01 Jul 2021 15:27:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )
Change subject: cpu/intel/common: Create get_cache_info() function
......................................................................
Patch Set 1:
(2 comments)
File src/cpu/intel/common/common_init.c:
https://review.coreboot.org/c/coreboot/+/55965/comment/04578145_5ebc4ee3
PS1, Line 331: 0x03
> I assume 0x03 is supposed to be LLC here? Create a macro for that?
if we are taking cache_level as argument then do we still need macro ?
https://review.coreboot.org/c/coreboot/+/55965/comment/c0b57aaa_da141917
PS1, Line 332:
: const size_t assoc = CPUID_CACHE_WAYS_OF_ASSOC(res) + 1;
: const size_t partitions = CPUID_CACHE_PHYS_LINE(res) + 1;
: const size_t cache_line_size = CPUID_CACHE_COHER_LINE(res) + 1;
: const size_t number_of_sets = CPUID_CACHE_NO_OF_SETS(res) + 1;
: const size_t cache_size = assoc * partitions * cache_line_size * number_of_sets;
: printk(BIOS_INFO, "Cache Information : ");
: printk(BIOS_INFO, "assoc=%zd par=%zd line_size=%zd sets=%zd\n",
: assoc, partitions, cache_line_size, number_of_sets);
: printk(BIOS_INFO, "@@ cache_size 0x%lx bytes\n", cache_size);
> also remarkably similar to https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr… ... maybe this function should take a parameter for the ecx value passed to CPUID?
Yes, this is doable but then only problem is smbios_write_type7_cache_parameters() expects to get those individual fields like level, cache_type, assoc etc. etc. so we have two possible ways:
1. Do we want to execute cpuid instruction several time and return those bit-fields as part of dedicated function like below?
static struct cpuid_result get_cpu_cache_parameters(int cache_type)
{
/* 1. Add a check to detect CPU if it belongs to IA or AMD? */
return cpuid_ext(DETERMINISTIC_CACHE_PARAMETERS_CPUID, cache_type);
}
size_t get_cpu_cache_type(int cache_type)
{
struct cpuid_result res = get_cpu_cache_parameters();
return CPUID_CACHE_TYPE(res);
}
...
2. Return all cache parameters as part of a single function call. here cache_type is only input argument and rests are output as below?
size_t get_cache_info(int cache_type, size_t *assoc, size_t *partitions, ...)
{
struct cpuid_result res = cpuid_ext(DETERMINISTIC_CACHE_PARAMETERS_CPUID, cache_type);
*assoc = CPUID_CACHE_WAYS_OF_ASSOC(res) + 1;
*partitions = CPUID_CACHE_PHYS_LINE(res) + 1;
...
}
Then caller of get_cache_info() function like smbios_write_type7_cache_parameters() can use these output variables (assoc, partitions) to perform the regular operations.
>
> `get_cache_info(unsigned int cache_level)`
> or similar?
> then the SMBIOS code could reuse this function.
Yes, this is possible, but need your help to define one path.
--
To view, visit https://review.coreboot.org/c/coreboot/+/55965
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095
Gerrit-Change-Number: 55965
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: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.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: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 01 Jul 2021 14:45:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment