Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36216 )
Change subject: soc/intel: common,skl,cnl,icl: drop reserved mmio memory size calculation ......................................................................
Patch Set 7:
Patch Set 6:
Patch Set 6:
Patch Set 6:
sorry for being late in review.. what is the reason of dropping reverse API call ? what we are really wish to achieve here.
I do doubt ur test coverage, i mean did u really enable any reversed range like PRMRR, ME Stolen Memory, GDXC etc...the reason we wrote this function to know if FSP has reserved something without CB knowledge. existing code should hold good.
I do doubt ur test coverage, i mean did u really enable any reversed range like PRMRR
Yes, and it broke. See discussion, please
The reserved memory size is influenced by multiple other factors like PTT, TraceHub, PRMRR etc.
i don't sure if i understood why its broken, i have enabled all those config several time on RVP for any issue debug. And don't see any issue since last 3 year. And its not new that this code is only used by chrome team, we have IOT and DCG team using the same code with those configs and never seen such issue.
existing code should hold good.
No. See discussion.
Subrata, check the whole patch series,
i believe your main agenda is to get rid of memmap design and follow FSP Hob to know the memory start. if yes, then don't bother with my concern. i'm only concern that if i do enable those reversed ranges using FSP UPD then i'm expecting do see that size in API.
please. There are multiple problems that need to be solved by a) relying on FSP or b) Intel making FSP finally open source or c) Intel doing it's homework and documenting their stuff ;-) I prefer b, but I guess this won't happen, even though Raja Koduri claimed working on making it open which I suspect to be some marketing move :/
I don't think i do get copied in such big management ladder. may be you can consider adding Vincent.
I am aware that this is not your decision, that's why I mentioned Raja.
Sorry, I'm not familiar with all those people, yet - which Vincent exactly?
But this design of Memmap.c is something that intel-google agreed upto in past and so far it holds good.
Well, as already stated we found multiple problems. For example disabling SGX and setting PRMRR=0 does not work on master. This was the reason to look deeper into memmap and systemagent and finally raised the problems there.