Change in coreboot[master]: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secbo...
Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot.
Change-Id: Iebce62f89312aa1e40555b637a9ced5d6498d14e Signed-off-by: Manideep Kurumella mkurumel@codeaurora.org --- M src/soc/qualcomm/sc7180/include/soc/symbols.h M src/soc/qualcomm/sc7180/memlayout.ld M src/soc/qualcomm/sc7180/soc.c 3 files changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/45418/1
diff --git a/src/soc/qualcomm/sc7180/include/soc/symbols.h b/src/soc/qualcomm/sc7180/include/soc/symbols.h index f15a8bb..b369d55 100644 --- a/src/soc/qualcomm/sc7180/include/soc/symbols.h +++ b/src/soc/qualcomm/sc7180/include/soc/symbols.h @@ -9,6 +9,7 @@ DECLARE_REGION(bsram) DECLARE_REGION(dram_aop) DECLARE_REGION(dram_soc) +DECLARE_REGION(dram_modem) DECLARE_REGION(dcb) DECLARE_REGION(pmic) DECLARE_REGION(limits_cfg) diff --git a/src/soc/qualcomm/sc7180/memlayout.ld b/src/soc/qualcomm/sc7180/memlayout.ld index 0142242..80df6c8 100644 --- a/src/soc/qualcomm/sc7180/memlayout.ld +++ b/src/soc/qualcomm/sc7180/memlayout.ld @@ -52,6 +52,7 @@ REGION(dram_aop, 0x80800000, 0x040000, 0x1000) REGION(dram_soc, 0x80900000, 0x200000, 0x1000) BL31(0x80B00000, 1M) + REGION(dram_modem, 0x86000000, 0x8C00000, 0x1000) POSTRAM_CBFS_CACHE(0x9F800000, 16M) RAMSTAGE(0xA0800000, 16M) } diff --git a/src/soc/qualcomm/sc7180/soc.c b/src/soc/qualcomm/sc7180/soc.c index 74f0868..30ad967 100644 --- a/src/soc/qualcomm/sc7180/soc.c +++ b/src/soc/qualcomm/sc7180/soc.c @@ -15,6 +15,8 @@ REGION_SIZE(dram_aop) / KiB); reserved_ram_resource(dev, 2, (uintptr_t)_dram_soc / KiB, REGION_SIZE(dram_soc) / KiB); + reserved_ram_resource(dev, 3, (uintptr_t)_dram_modem / KiB, + REGION_SIZE(dram_modem) / KiB); }
static void soc_init(struct device *dev)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45418/1/src/soc/qualcomm/sc7180/soc... File src/soc/qualcomm/sc7180/soc.c:
https://review.coreboot.org/c/coreboot/+/45418/1/src/soc/qualcomm/sc7180/soc... PS1, Line 18: reserved_ram_resource(dev, 3, (uintptr_t)_dram_modem / KiB, Please respond on the bug as for why this is needed. This will permanently remove 140MB from the OS' available memory, even on systems without a modem. Having the kernel handle this internally so it can decide whether it has a modem or not (and reclaim the memory otherwise) would be much preferable. If that's really not possible, could we maybe at least condition this on sku_id() so we only remove the memory when really needed? (This would require us to keep track of an exact mapping which SKU numbers of which boards have a modem in firmware, which is a hassle I'd rather avoid... but still better than wasting so much memory.)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45418/1/src/soc/qualcomm/sc7180/mem... File src/soc/qualcomm/sc7180/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/45418/1/src/soc/qualcomm/sc7180/mem... PS1, Line 55: REGION(dram_modem, 0x86000000, 0x8C00000, 0x1000) If this memory is really hard-reserved by the XPUs and any access to it risks hanging the system, we need to also carve it out from the MMU mapping. Otherwise, any random speculative access could hit this and cause an abort.
You need to change src/soc/qualcomm/common/mmu.c to make two mappings for before and behind this range (again, if possible only on SKUs that actually have a modem).
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45418/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45418/5//COMMIT_MSG@2 PS5, Line 2: mkurumel Please use a full name for the author.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45418/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45418/7//COMMIT_MSG@7 PS7, Line 7: modem cleanup in Secboot reboot What is “modem cleanup”?
https://review.coreboot.org/c/coreboot/+/45418/7//COMMIT_MSG@7 PS7, Line 7: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. Please remove the dot/period at the end of the commit message summary.
https://review.coreboot.org/c/coreboot/+/45418/7//COMMIT_MSG@8 PS7, Line 8: What is the problem? What did not work before, and what works now?
Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45418 )
Change subject: sc7180: Add Modem region in memlayout to avoid modem cleanup in Secboot reboot. ......................................................................
Abandoned
Replaced with CB:46661
-
Julius Werner (Code Review) -
Martin Roth (Code Review)
-
Paul Menzel (Code Review)
-
Ravi kumar (Code Review)