Hello Anjaneya "Reddy" Chagam,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40544
to review the following change.
Change subject: soc/intel/xeon_sp/skx: use mem32 address space for PCIe mem64 assignment ......................................................................
soc/intel/xeon_sp/skx: use mem32 address space for PCIe mem64 assignment
FSP HOB indicates whether mem32 address space is used for PCIe mem64 allocation or not. If that is the case, split PCIe mem32 address space to support both mem32 assignment and mem64 assignment.
TESTED=booted TiogaPass with different PCIe configurations; Verified PCIe devices works in target OS, and resource assignments are correct.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Signed-off-by: Reddy Chagam anjaneya.chagam@intel.com Tested-by: Morgan_Jang@wiwynn.com Tested-by: Ryback.Hung@quantatw.com Change-Id: If4ba1de7b367564da74936d32957d4d663f9f740 --- M src/soc/intel/xeon_sp/skx/chip.c M src/soc/intel/xeon_sp/skx/include/soc/soc_util.h M src/soc/intel/xeon_sp/skx/soc_util.c 3 files changed, 22 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/40544/1
diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index f970ce4..da98442 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -58,7 +58,7 @@ return RES_TYPE_PREF_MEM; else return RES_TYPE_NONPREF_MEM; - n} + } printk(BIOS_ERR, "Invalid resource type 0x%llx\n", flags); die("Invalid resource type"); } @@ -443,7 +443,23 @@ xeonsp_pci_dev_iterator(link, xeonsp_reset_pci_op, NULL, NULL);
struct iiostack_resource stack_info = {0}; - get_iiostack_info(&stack_info); + uint8_t pci64bit_alloc_flag = get_iiostack_info(&stack_info); + + /* + * pci64bit_alloc_flag is obtained from FSP HOB. It indicates whether + * 32bit address space needs to be split between mem32 and + * mem64 windows. + */ + if (!pci64bit_alloc_flag) { + for (int s = 0; s < stack_info.no_of_stacks; ++s) { + STACK_RES *res = &stack_info.res[s]; + uint64_t length = (res->PciResourceMem32Limit - + res->PciResourceMem32Base + 1) / 2; + res->PciResourceMem64Limit = res->PciResourceMem32Limit; + res->PciResourceMem32Limit = (res->PciResourceMem32Base + length - 1); + res->PciResourceMem64Base = res->PciResourceMem32Limit + 1; + } + }
/* assign resources */ assign_stack_resources(&stack_info, dev, NULL); diff --git a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h index 8ba4b29..002fb2b 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h @@ -22,7 +22,7 @@ uint32_t pci_read_mmio_reg(int bus, uint32_t dev, uint32_t func, int offset);
uint32_t get_socket_stack_busno(uint32_t socket, uint32_t stack); -void get_iiostack_info(struct iiostack_resource *info); +uint8_t get_iiostack_info(struct iiostack_resource *info);
int get_threads_per_package(void); int get_platform_thread_count(void); diff --git a/src/soc/intel/xeon_sp/skx/soc_util.c b/src/soc/intel/xeon_sp/skx/soc_util.c index edacafd..94f2a38 100644 --- a/src/soc/intel/xeon_sp/skx/soc_util.c +++ b/src/soc/intel/xeon_sp/skx/soc_util.c @@ -364,7 +364,7 @@ return get_cpu_count() * get_threads_per_package(); }
-void get_iiostack_info(struct iiostack_resource *info) +uint8_t get_iiostack_info(struct iiostack_resource *info) { size_t hob_size; const uint8_t fsp_hob_iio_universal_data_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; @@ -386,6 +386,8 @@ memcpy(&info->res[info->no_of_stacks++], ri, sizeof(STACK_RES)); } } + + return hob->PlatformData.Pci64BitResourceAllocation; }
#if ENV_RAMSTAGE
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40544 )
Change subject: soc/intel/xeon_sp/skx: use mem32 address space for PCIe mem64 assignment ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40544/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40544/1//COMMIT_MSG@14 PS1, Line 14: works work
https://review.coreboot.org/c/coreboot/+/40544/1/src/soc/intel/xeon_sp/skx/c... File src/soc/intel/xeon_sp/skx/chip.c:
https://review.coreboot.org/c/coreboot/+/40544/1/src/soc/intel/xeon_sp/skx/c... PS1, Line 61: n} Does that belong into an earlier commit?
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40544 )
Change subject: soc/intel/xeon_sp/skx: use mem32 address space for PCIe mem64 assignment ......................................................................
Patch Set 1:
(2 comments)
While working on this, I made a mistake and got a different change ID. The comments are addressed in https://review.coreboot.org/c/coreboot/+/40577 instead. I am going to abandon this one.
https://review.coreboot.org/c/coreboot/+/40544/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40544/1//COMMIT_MSG@14 PS1, Line 14: works
work
Done
https://review.coreboot.org/c/coreboot/+/40544/1/src/soc/intel/xeon_sp/skx/c... File src/soc/intel/xeon_sp/skx/chip.c:
https://review.coreboot.org/c/coreboot/+/40544/1/src/soc/intel/xeon_sp/skx/c... PS1, Line 61: n}
Does that belong into an earlier commit?
Done
Jonathan Zhang has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40544 )
Change subject: soc/intel/xeon_sp/skx: use mem32 address space for PCIe mem64 assignment ......................................................................
Abandoned
replaced by 40577