build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38985 )
Change subject: WIP: Add VTD ......................................................................
Patch Set 4:
(23 comments)
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... File src/cpu/intel/xeonsp/cpu/skylake-sp/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 36: #define SKXSP_MMAP_VTD_CFG_REG_DEVID 0x2024 please, no space before tabs
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... File src/cpu/intel/xeonsp/cpu/skylake-sp/include/soc/skx_log_utils.h:
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 20: printk(BIOS_SPEW, "%s:%d res: %s, dev: %s, index: 0x%x, base: 0x%llx, end: 0x%llx, size_kb: 0x%llx\n", \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 21: __func__, __LINE__, type, dev_path(dev), index, (base_kb << 10), (base_kb << 10) + (size_kb << 10) - 1, size_kb) line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 24: printk(BIOS_SPEW, "%s:%d res: %s, dev: %s, index: 0x%x, base: 0x%llx, end: 0x%llx, size: 0x%llx\n", \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 27: #define ADD_MMIO_RESOURCE(dev, index, base, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 28: mmio_resource(dev, index, (uint64_t) ((uint64_t)base >> 10), (uint64_t) ((uint64_t)size >> 10)); \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 29: LOG_MEM_RESOURCE("mmio", dev, index, (uint64_t) ((uint64_t)base >> 10), (uint64_t) ((uint64_t)size >> 10)) line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... File src/cpu/intel/xeonsp/cpu/skylake-sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 69: [MMCFG_LIMIT_REG] = MAP_ENTRY_LIMIT_64(SKXSP_VTD_MMCFG_LIMIT_CSR, 26, "MMCFG_LIMIT"), line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 74: [TSEG_LIMIT_REG] = MAP_ENTRY_LIMIT_32(SKXSP_VTD_TSEG_LIMIT_CSR, 20, "TSEGMB_LIMIT"), line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 166: * |PCIe MMCFG (relocatable) | CONFIG_MMCONF_BASE_ADDRESS 64 or 256MB (0x80000000 - 0x8fffffff, 0x40000) line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 206: printk(BIOS_SPEW, "cbmem_top: 0x%lx, fsp range: [0x%llx - 0x%llx], top_of_ram: 0x%llx\n", (uintptr_t) cbmem_top(), line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 207: range_entry_base(&fsp_mem), range_entry_end(&fsp_mem), top_of_ram); line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 224: * arch_upd->BootLoaderTolumSize = cbmem_overhead_size(); == 2 * CBMEM_ROOT_MIN_SIZE line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 224: * arch_upd->BootLoaderTolumSize = cbmem_overhead_size(); == 2 * CBMEM_ROOT_MIN_SIZE please, no space before tabs
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 242: base_kb = (range_entry_base(&fsp_mem) + (range_entry_end(&fsp_mem) - range_entry_base(&fsp_mem) + 1)) >> 10; line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 272: resource->size = (resource_t) (mc_values[MMCFG_LIMIT_REG] - mc_values[MMCFG_BASE_REG] + 1); line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 274: IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED; line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 275: LOG_MEM_RESOURCE("mmiocfg_res", dev, index-1, (resource->base >> 10), (resource->size >> 10)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 282: IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED; line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 283: LOG_MEM_RESOURCE("apic_res", dev, index-1, (resource->base >> 10), (resource->size >> 10)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 306: IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED; line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 307: LOG_MEM_RESOURCE("APEI_ERST", dev, index-1, (resource->base >> 10), (resource->size >> 10)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/4/src/cpu/intel/xeonsp/cpu/sk... PS4, Line 325: printk(BIOS_DEBUG, "mmapvtd_init\n"); Prefer using '"%s...", __func__' to using 'mmapvtd_init', this function's name, in a string