[coreboot-gerrit] Change in coreboot[master]: soc/intel/skylake: Create API to get cache size

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/18922 )

Change subject: soc/intel/skylake: Create API to get cache size
......................................................................


Patch Set 4:

(4 comments)

> (4 comments)
 > 
 > You could save a bunch of trouble and just return CONFIG_ROM_SIZE
 > since any spi based boot medium on skl could cache the entire rom
 > just below 4GiB.

Yes, Earlier i thought the same just to return CAR_ROM_SIZE but after reading the logical involved in small core to get just BIOS region and don't bother to cache entire Flash Region (ME mutiple copy, descriptor and other range), also in order to get common code funda rolling, we may use this concept in both Intel cores

https://review.coreboot.org/#/c/18922/4/src/soc/intel/skylake/mmap_boot.c
File src/soc/intel/skylake/mmap_boot.c:

Line 39:  * bios_start +---> +------------+--------------------------> +----------------+ 4GiB - bios_size
> > 80 cols
will fix


PS4, Line 48: *                  | Device ext |
> This does not exist on skylake.
will fix


Line 53: static size_t bios_start CAR_GLOBAL;
> There's no reason for this to be a global.
in that case, we may also need to touch APL code to remove CAR global. Also i suggest to use SPI read and return the bios region size. make sense?


Line 84: size_t get_bios_size(void)
> What's the plan with this file? Are you going to merge this into a common o
we will put this into common as both big and small core align with same fundamental idea of allocating MTRRs and i really like the way this been done for small core. only think we may need to pull this little early to get maximum performance


-- 
To view, visit https://review.coreboot.org/18922
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I024e0a9e4ee33794a7ea91bc57c5ee6673825e47
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan at intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Pratikkumar Prajapati <pratikkumar.v.prajapati at intel.corp-partner.google.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi at intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma at intel.com>
Gerrit-HasComments: Yes