coreboot-gerrit September 2023

coreboot-gerrit@coreboot.org

1 participants
3824 discussions

[S] Change in coreboot[main]: soc/intel/cse: Add entries to eventlog on PSR backup events
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Krishna P Bhat D, Paul Menzel, Rizwan Qureshi, sridhar siricilla. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75760?usp=email ) Change subject: soc/intel/cse: Add entries to eventlog on PSR backup events ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75760?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2459a2b941d28a87b6c78f75dbe8779d73328d7a Gerrit-Change-Number: 75760 Gerrit-PatchSet: 14 Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 09:34:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/cse: Back up PSR data during CSE FW downgrade
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Anil Kumar K, Jérémy Compostella, Krishna P Bhat D, Paul Menzel, Rizwan Qureshi, sridhar siricilla. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74577?usp=email ) Change subject: soc/intel/cse: Back up PSR data during CSE FW downgrade ...................................................................... Patch Set 52: (4 comments) File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/74577/comment/684379d8_b4ccac4f : PS50, Line 1081: cse_boot_to_rw > as explained above we do not want to prevent the system to boot whenever there is a failure to PSR backup attempt. I'm missing the point. why PSR data backup failure is considered non-critical information. if it's non-critical then why do we want to backup that up even for downgrading ? can't we just ignore the PSR data on every downgrade like what you have done here and continue booting ? https://review.coreboot.org/c/coreboot/+/74577/comment/c7b99d20_e49e40ed : PS50, Line 1092: goto log_and_exit; > As Rizwan mentioned , we backup data whenever possible. what does that mean? "backup the data whenever possible?" are you saying that the PSR backup is not something mandatory ? > When there is a failure to backup (which is a rare event) we dont want to go to recovery. We log that as an event and continue to boot. I'm looking for some documentation that says there is no harm in ignoring the backup failure and don't need to go into recovery. > (which is a rare event) what is the reason behind such *rare* events? and how to handle the rear handle ? just ignore it ? https://review.coreboot.org/c/coreboot/+/74577/comment/15bb2a4a_0998ac26 : PS50, Line 1125: An attempt to send PSR back-up command has been made. > > what will happen when you just `return' after marking PSR_BACKUP_DONE (even in case the PSR backup is failing)? This information is missing in the above flow diagram? system will boot into OS in recovery mode and the device eventually lost its old telemetry data ? if there is no impact of losing those PSR data (in downgrade scenario) then why are we even care so much about implementing PSR ? > > > I thought you originally wished to ensure that before CSE clearing the data, we need to preserve the PSR and then CSE reapplying that backup data later. Now if PSR backup is failing and you don't consider that as any catastrophic failure, I'm missing the purpose of doing all the flow change. We can simply assume, PSR is not backup in all CSE downgrade scenario ? > > > That is a false logic, you are basing the above conclusion of not backing up the data for something that happens as an failure/anomaly and a rare event. In all normal scenarios if there is an opportunity to back-up the data, we should. What do you mean by false logic? `cse_boot_to_rw` is not a false logic. if the device is unable to boot to RW and you aren't able to send PSR_HECI_FW_DOWNGRADE_BACKUP cmd, hence, you finally marked things as PSR backup done ? i don't understand the motivation here. if you started ignoring all the failure cases and corner cases then I'm afraid we shall miss the original intent. What I'm asking is don't call the PSR backup done when it's not actually attempted or succeeded. Rather use something as PSR_BACKUP_ABORTED. At least it will help us to know whether the attempt is successful genuinely or not. File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/74577/comment/ea8d2ead_c2249abe : PS52, Line 1152: CONFIG(SOC_INTEL_CSE_LITE_PSR) use this as the first case ``` if (CONFIG(SOC_INTEL_CSE_LITE_PSR) && status == CSE_UPDATE_DOWNGRADE) ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/74577?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I135d197b5df0a20def823fe615860b5ead4391f8 Gerrit-Change-Number: 74577 Gerrit-PatchSet: 52 Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 09:00:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anil Kumar K <anil.kumar.k(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Comment-In-Reply-To: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8188: devapc: Update permission setting
by Yu-Ping Wu (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Hung-Te Lin, Paul Menzel, Yidi Lin, jason-ch chen. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77861?usp=email ) Change subject: soc/mediatek/mt8188: devapc: Update permission setting ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/77861/comment/dac4b43e_2e94361a : PS3, Line 7: Update permission setting > Yes, CB:77883 is also depends on this patch. […] How about `Update permission for master domain setup`? -- To view, visit https://review.coreboot.org/c/coreboot/+/77861?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6e95ddb5d84a09ff865d7615596430e25b69d3fc Gerrit-Change-Number: 77861 Gerrit-PatchSet: 5 Gerrit-Owner: jason-ch chen <Jason-ch.Chen(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nina-CM Wu <nina-cm.wu(a)mediatek.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: jason-ch chen <Jason-ch.Chen(a)mediatek.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:56:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: jason-ch chen <Jason-ch.Chen(a)mediatek.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/cse: Implement APIs to access PSR backup status in CMOS
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Anil Kumar K, Jérémy Compostella, Krishna P Bhat D, Rizwan Qureshi, sridhar siricilla. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77069?usp=email ) Change subject: soc/intel/cse: Implement APIs to access PSR backup status in CMOS ...................................................................... Patch Set 7: (4 comments) File src/soc/intel/common/block/cse/cse_lite_cmos.c: https://review.coreboot.org/c/coreboot/+/77069/comment/c3599319_c74e5550 : PS7, Line 113: int8_t int ? File src/soc/intel/common/block/include/intelblocks/cse.h: https://review.coreboot.org/c/coreboot/+/77069/comment/0001ecaa_03245714 : PS7, Line 176: int8_t int https://review.coreboot.org/c/coreboot/+/77069/comment/f9fc7f90_e106ef87 : PS7, Line 176: backup_status nit: status or value the DS name itself is `psr_backup_status` hence, this field can be renamed as value (aka psr_backup_status.value) File src/soc/intel/common/block/include/intelblocks/cse.h: https://review.coreboot.org/c/coreboot/+/77069/comment/b4070b24_c03804e5 : PS5, Line 171: 0x55 > Hi Subrata , We kept the values 0x55 and 0x22 to make sure these are not commonly occuring values like 0/1 . So we suggest to have them 0x55/0x22 to ensure it doesn't coincide with default cmos values during bootup honestly i don't know if 0x55/0x22 has any special meaning. requesting to use some meaningful value as per embedded system programming guidelines. additionally, you don't need to worry about CMOS default value because you have a signature and CRC field in ur DS which can be used to catch a nude/bare CMOS. -- To view, visit https://review.coreboot.org/c/coreboot/+/77069?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I270894e3e08dd50ca88e5402b59c211d7e693d14 Gerrit-Change-Number: 77069 Gerrit-PatchSet: 7 Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:43:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anil Kumar K <anil.kumar.k(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/mediatek: PCI: Fix translation window
by Yu-Ping Wu (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Hung-Te Lin, Jianjun Wang, Nico Huber, Paul Menzel, Yidi Lin. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78044?usp=email ) Change subject: soc/mediatek: PCI: Fix translation window ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78044?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I42b0f0bf9222d284dee0c29f1a6ed6366d6e6689 Gerrit-Change-Number: 78044 Gerrit-PatchSet: 14 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:37:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/mtl: Override SOC_INTEL_CSE_FW_PARTITION_CMOS_OFFSET
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Eran Mitrani, Jakub Czapiga, Kapil Porwal, Krishna P Bhat D, Rizwan Qureshi, Tarun. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78054?usp=email ) Change subject: soc/intel/mtl: Override SOC_INTEL_CSE_FW_PARTITION_CMOS_OFFSET ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/meteorlake/Kconfig: https://review.coreboot.org/c/coreboot/+/78054/comment/322128e4_b379827f : PS3, Line 442: # The default offset to store CSE RW FW version information is at 68. In Intel Meteor Lake based : # systems that use PSR, the additional size required to keep CSE RW FW version information and : # PSR back-up status in adjacent CMOS memory at offset 68 is not available. Offset 161 has : # enough space to keep both the CSE related information together. Hence override the default : # offset to 161. ``` # The default offset to store CSE RW FW version information is at 68. # However, in Intel Meteor Lake based systems that use PSR, the additional # size required to keep CSE RW FW version information and PSR back-up status # in adjacent CMOS memory at offset 68 is not available. Therefore, we # override the default offset to 161, which has enough space to keep both # the CSE related information together. ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/78054?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8bae5245f93b99be15b4e59cfeffbc23eec95001 Gerrit-Change-Number: 78054 Gerrit-PatchSet: 3 Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:36:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/mtl: Call cse_fill_bp_info early in romstage
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

1 0

[M] Change in coreboot[main]: soc/intel/cse: Add function to get cse_bp_info early
by Subrata Banik (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Krishna P Bhat D, Rizwan Qureshi. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78053?usp=email ) Change subject: soc/intel/cse: Add function to get cse_bp_info early ...................................................................... Patch Set 3: (4 comments) File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/78053/comment/d0d2666b_e2688ef0 : PS1, Line 468: is_cse_bp_info_valid > > anyway had introduced the boot time impact (which we don't measure today) across the global reset but user will feel the same > > the impact is across cold reset. This change will nullify the impact across warm/global resets. > > > There is no benefit of sending CSE cmd on every boot if u know the CBMEM data is persistent and CSE version can't be changed across warm reboot > > we can take it up as a follow-up patch, if you insist. lets do that later File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/78053/comment/61486147_6f827529 : PS2, Line 424: store_cse_bp_info_in_cbmem > store_cse_bp_info_in_cbmem() is being called by https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block… to store response in the case of late romstage cse_fw_sync() and also called by https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block… in CBMEM_CREATION_HOOK. > cbmem_find(CBMEM_ID_CSE_BP_INFO) is only used at https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block…, better not to move it to common function. i'm unable to follow the concern ? you are trying to fetch the cse bp info from cbmem. if the information is readily available then fetch it or else, prepare the data into the cbmem. I don't understand why it's better not to move things into an unified function ? take a look into cbmem_add implementation, it first do cbmem_find and on failure it makes a new entry. https://review.coreboot.org/c/coreboot/+/78053/comment/f8582ac9_90bb7bae : PS2, Line 470: cse_bp_info_in_cbmem > https://review.coreboot.org/c/coreboot/+/78053/comment/2f9f5241_046c81c8/ As Rizwan suggested. lets do that later as discussed https://review.coreboot.org/c/coreboot/+/78053/comment/2f9f5241_046c81c8/ https://review.coreboot.org/c/coreboot/+/78053/comment/96f90ffd_35143c41 : PS2, Line 507: if (cbmem_online()) : store_cse_bp_info_in_cbmem(); > We have to consider the case of cse_fw_sync() being called in late romstage also like in JSL, TGL. The cbmem init hooks would have been run prior to cse_get_bp_info being called (after fsp memory init) and we would have to then copy the get_bp_info_rsp to cbmem. Calling fetch_cse_bp_info_from_cbmem() from line 469 would eliminate the need to store the cse bp info later (basically do the cbmem add, like what this function is doing). -- To view, visit https://review.coreboot.org/c/coreboot/+/78053?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib1e72c950ba0f4911924805f501ec1bd54b6ba3c Gerrit-Change-Number: 78053 Gerrit-PatchSet: 3 Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:31:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Comment-In-Reply-To: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/mediatek: PCI: Fix translation window
by Jianjun Wang (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Hung-Te Lin, Nico Huber, Paul Menzel, Yidi Lin, Yu-Ping Wu. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78044?usp=email ) Change subject: soc/mediatek: PCI: Fix translation window ...................................................................... Patch Set 14: (1 comment) File src/soc/mediatek/common/pcie.c: https://review.coreboot.org/c/coreboot/+/78044/comment/1bd1102c_db04ce85 : PS13, Line 167: /* > Sorry, I meant adding it within the while loop. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78044?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I42b0f0bf9222d284dee0c29f1a6ed6366d6e6689 Gerrit-Change-Number: 78044 Gerrit-PatchSet: 14 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Thu, 28 Sep 2023 08:18:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/mediatek: PCI: Fix translation window
by Jianjun Wang (Code Review) 28 Sep '23

28 Sep '23

Attention is currently required from: Hung-Te Lin, Jianjun Wang, Nico Huber, Paul Menzel, Yidi Lin, Yu-Ping Wu. Hello Hung-Te Lin, Nico Huber, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78044?usp=email to look at the new patch set (#14). The following approvals got outdated and were removed: Code-Review+2 by Yu-Ping Wu, Verified+1 by build bot (Jenkins) Change subject: soc/mediatek: PCI: Fix translation window ...................................................................... soc/mediatek: PCI: Fix translation window Dojo fails to boot from NVMe with CONFIG_RESOURCE_ALLOCATION_TOP_DOWN enabled. The root cause is using __fls() will get a smaller value when the size is not a power of 2, for example, __fls(0x3000000) = 25. Hence the PCIe translation window size is set to 0x2000000. Accessing addresses higher than 0x2300000 will fail. Fix translation window by splitting the MMIO space to multiple tables if its size is not a power of 2. Resolves: https://ticket.coreboot.org/issues/508. TEST=Build pass and boot up to kernel successfully via SSD on Dojo board, it can boot with and without the CONFIG_RESOURCE_ALLOCATION_TOP_DOWN option. BUS=b:298255933 BRANCH=cherry Change-Id: I42b0f0bf9222d284dee0c29f1a6ed6366d6e6689 Signed-off-by: Jianjun Wang <jianjun.wang(a)mediatek.com> --- M src/soc/mediatek/common/pcie.c 1 file changed, 60 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/78044/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/78044?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I42b0f0bf9222d284dee0c29f1a6ed6366d6e6689 Gerrit-Change-Number: 78044 Gerrit-PatchSet: 14 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2023