Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel Xeon Scalable Processor support ......................................................................
Patch Set 47:
(6 comments)
Thanks for the comments.
https://review.coreboot.org/c/coreboot/+/38548/46/src/soc/intel/xeon_sp/chip... File src/soc/intel/xeon_sp/chip.c:
https://review.coreboot.org/c/coreboot/+/38548/46/src/soc/intel/xeon_sp/chip... PS46, Line 432: struct iiostack_resource stack_info;
let's zero-initialize this without calling memset: […]
Done
https://review.coreboot.org/c/coreboot/+/38548/46/src/soc/intel/xeon_sp/chip... PS46, Line 446: stack_info.sres
it looks like we are malloc()'ing stack_info. […]
Done
https://review.coreboot.org/c/coreboot/+/38548/47/src/soc/intel/xeon_sp/unco... File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/38548/47/src/soc/intel/xeon_sp/unco... PS47, Line 208: /* Mark FSP region */ : base_kb = (range_entry_base(&fsp_mem) >> 10); : size_kb = (range_entry_end(&fsp_mem) - range_entry_base(&fsp_mem) + 1) >> 10; : LOG_MEM_RESOURCE("mmio_fsp", dev, index, base_kb, size_kb); : mmio_resource(dev, index++, base_kb, size_kb); :
you don't need to explicitly mark FSP's region. […]
Done
https://review.coreboot.org/c/coreboot/+/38548/47/src/soc/intel/xeon_sp/unco... PS47, Line 214: /* Mark coreboot region as reserved */ : base_kb = (range_entry_base(&fsp_mem) + (range_entry_end(&fsp_mem) - : range_entry_base(&fsp_mem) + 1)) >> 10; : size_kb = (mc_values[TSEG_BASE_REG] - (base_kb << 10)) >> 10; : LOG_MEM_RESOURCE("mmio_coreboot", dev, index, base_kb, size_kb); : mmio_resource(dev, index++, base_kb, size_kb);
I do not understand what is the "coreboot region". […]
Done
https://review.coreboot.org/c/coreboot/+/38548/47/src/soc/intel/xeon_sp/unco... PS47, Line 225: mmio_resource(dev, index++, base_kb, size_kb);
I think that we mark SMM as reserved resource, not MMIO.
Done
https://review.coreboot.org/c/coreboot/+/38548/47/src/soc/intel/xeon_sp/unco... PS47, Line 232: mmio_resource(dev, index++, base_kb, size_kb);
I believe this is not MMIO as well
Done