* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment [not found] <16E1B70DABEEDF46.30116@groups.io> @ 2022-05-16 5:41 ` Sean Rhodes 2022-05-16 6:54 ` Wu, Hao A 0 siblings, 1 reply; 10+ messages in thread From: Sean Rhodes @ 2022-05-16 5:41 UTC (permalink / raw) To: devel, Sean Rhodes; +Cc: Jian J Wang, Hao A Wu, Liming Gao [-- Attachment #1: Type: text/plain, Size: 3560 bytes --] Hi Would any one be able to review please? Thank you On Fri, 1 Apr 2022, 09:03 Sean Rhodes via groups.io, <sean= starlabs.systems@groups.io> wrote: > WorkSpaceAddress and SpareAreaAddress point into MMIO, which isn't > always aligned. Remove the check for block alignment to avoid > false assertions. > > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao A Wu <hao.a.wu@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Signed-off-by: Sean Rhodes <sean@starlabs.systems> > Change-Id: Ia1c1f44b6a0e7f32cac0d7806e74d729e5d83a6d > --- > .../Universal/FaultTolerantWriteDxe/FtwMisc.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c > b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c > index 661e148767..3b9ff1c828 100644 > --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c > +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c > @@ -1121,12 +1121,10 @@ FindFvbForFtw ( > FtwDevice->NumberOfWorkSpaceBlock = FTW_BLOCKS > (FtwDevice->FtwWorkSpaceBase + FtwDevice->FtwWorkSpaceSize, > FtwDevice->WorkBlockSize); > if (FtwDevice->FtwWorkSpaceSize >= FtwDevice->WorkBlockSize) { > // > - // Check the alignment of work space address and length, they > should be block size aligned when work space size is larger than one block > size. > + // Check the alignment of work space length, it should be > block size aligned when work space size is larger than one block size. > // > - if (((FtwDevice->WorkSpaceAddress & (FtwDevice->WorkBlockSize > - 1)) != 0) || > - ((FtwDevice->WorkSpaceLength & (FtwDevice->WorkBlockSize > - 1)) != 0)) > - { > - DEBUG ((DEBUG_ERROR, "Ftw: Work space address or length is > not block size aligned when work space size is larger than one block > size\n")); > + if ((FtwDevice->WorkSpaceLength & (FtwDevice->WorkBlockSize - > 1)) != 0) { > + DEBUG ((EFI_D_ERROR, "Ftw: Work space length is not block > size aligned when work space size is larger than one block size\n")); > FreePool (HandleBuffer); > ASSERT (FALSE); > return EFI_ABORTED; > @@ -1171,12 +1169,10 @@ FindFvbForFtw ( > } > > // > - // Check the alignment of spare area address and length, they > should be block size aligned > + // Check the alignment of spare area length, it should be block > size aligned > // > - if (((FtwDevice->SpareAreaAddress & (FtwDevice->SpareBlockSize > - 1)) != 0) || > - ((FtwDevice->SpareAreaLength & (FtwDevice->SpareBlockSize - > 1)) != 0)) > - { > - DEBUG ((DEBUG_ERROR, "Ftw: Spare area address or length is > not block size aligned\n")); > + if ((FtwDevice->SpareAreaLength & (FtwDevice->SpareBlockSize - > 1)) != 0) { > + DEBUG ((EFI_D_ERROR, "Ftw: Spare area address or length is > not block size aligned\n")); > FreePool (HandleBuffer); > // > // Report Status Code EFI_SW_EC_ABORTED. > -- > 2.32.0 > > > > ------------ > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#88320): https://edk2.groups.io/g/devel/message/88320 > Mute This Topic: https://groups.io/mt/90173290/6718866 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [sean@starlabs.systems] > ------------ > > > [-- Attachment #2: Type: text/html, Size: 5051 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 5:41 ` [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Sean Rhodes @ 2022-05-16 6:54 ` Wu, Hao A 2022-05-16 6:59 ` Sean Rhodes 0 siblings, 1 reply; 10+ messages in thread From: Wu, Hao A @ 2022-05-16 6:54 UTC (permalink / raw) To: Rhodes, Sean, devel@edk2.groups.io, Rhodes, Sean Cc: Wang, Jian J, Gao, Liming [-- Attachment #1: Type: text/plain, Size: 5302 bytes --] Sorry for a question. I referred the code in InitFtwDevice(): FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFlashNvStorageFtwWorkingBase64); if (FtwDevice->WorkSpaceAddress == 0) { FtwDevice->WorkSpaceAddress = (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFlashNvStorageFtwWorkingBase); } and the PCD definition in MdeModulePkg.dec: ## Base address of the FTW working block range in flash device. # If PcdFlashNvStorageFtwWorkingSize is larger than one block size, this value should be block size aligned. # @Prompt Base address of flash FTW working block range. gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0|UINT32|0x30000010 ## 64-bit Base address of the FTW working block range in flash device. # If PcdFlashNvStorageFtwWorkingSize is larger than one block size, this value should be block size aligned. # @Prompt 64-bit Base address of flash FTW working block range. gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0|UINT64|0x80000010 The description of both PCDs mentioned a block size alignment requirement. Does the change in this patch conflict with the above PCD description? (SpareAreaAddress is having a similar case.) Best Regards, Hao Wu From: Sean Rhodes <sean@starlabs.systems> Sent: Monday, May 16, 2022 1:41 PM To: devel@edk2.groups.io; Rhodes, Sean <sean@starlabs.systems> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Hi Would any one be able to review please? Thank you On Fri, 1 Apr 2022, 09:03 Sean Rhodes via groups.io<http://groups.io>, <sean=starlabs.systems@groups.io<mailto:starlabs.systems@groups.io>> wrote: WorkSpaceAddress and SpareAreaAddress point into MMIO, which isn't always aligned. Remove the check for block alignment to avoid false assertions. Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>> Signed-off-by: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>> Change-Id: Ia1c1f44b6a0e7f32cac0d7806e74d729e5d83a6d --- .../Universal/FaultTolerantWriteDxe/FtwMisc.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c index 661e148767..3b9ff1c828 100644 --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c @@ -1121,12 +1121,10 @@ FindFvbForFtw ( FtwDevice->NumberOfWorkSpaceBlock = FTW_BLOCKS (FtwDevice->FtwWorkSpaceBase + FtwDevice->FtwWorkSpaceSize, FtwDevice->WorkBlockSize); if (FtwDevice->FtwWorkSpaceSize >= FtwDevice->WorkBlockSize) { // - // Check the alignment of work space address and length, they should be block size aligned when work space size is larger than one block size. + // Check the alignment of work space length, it should be block size aligned when work space size is larger than one block size. // - if (((FtwDevice->WorkSpaceAddress & (FtwDevice->WorkBlockSize - 1)) != 0) || - ((FtwDevice->WorkSpaceLength & (FtwDevice->WorkBlockSize - 1)) != 0)) - { - DEBUG ((DEBUG_ERROR, "Ftw: Work space address or length is not block size aligned when work space size is larger than one block size\n")); + if ((FtwDevice->WorkSpaceLength & (FtwDevice->WorkBlockSize - 1)) != 0) { + DEBUG ((EFI_D_ERROR, "Ftw: Work space length is not block size aligned when work space size is larger than one block size\n")); FreePool (HandleBuffer); ASSERT (FALSE); return EFI_ABORTED; @@ -1171,12 +1169,10 @@ FindFvbForFtw ( } // - // Check the alignment of spare area address and length, they should be block size aligned + // Check the alignment of spare area length, it should be block size aligned // - if (((FtwDevice->SpareAreaAddress & (FtwDevice->SpareBlockSize - 1)) != 0) || - ((FtwDevice->SpareAreaLength & (FtwDevice->SpareBlockSize - 1)) != 0)) - { - DEBUG ((DEBUG_ERROR, "Ftw: Spare area address or length is not block size aligned\n")); + if ((FtwDevice->SpareAreaLength & (FtwDevice->SpareBlockSize - 1)) != 0) { + DEBUG ((EFI_D_ERROR, "Ftw: Spare area address or length is not block size aligned\n")); FreePool (HandleBuffer); // // Report Status Code EFI_SW_EC_ABORTED. -- 2.32.0 ------------ Groups.io Links: You receive all messages sent to this group. View/Reply Online (#88320): https://edk2.groups.io/g/devel/message/88320 Mute This Topic: https://groups.io/mt/90173290/6718866 Group Owner: devel+owner@edk2.groups.io<mailto:devel%2Bowner@edk2.groups.io> Unsubscribe: https://edk2.groups.io/g/devel/unsub [sean@starlabs.systems] ------------ [-- Attachment #2: Type: text/html, Size: 10696 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 6:54 ` Wu, Hao A @ 2022-05-16 6:59 ` Sean Rhodes 2022-05-16 7:36 ` Wu, Hao A 0 siblings, 1 reply; 10+ messages in thread From: Sean Rhodes @ 2022-05-16 6:59 UTC (permalink / raw) To: Wu, Hao A, devel [-- Attachment #1: Type: text/plain, Size: 97 bytes --] Hi Hao Yes, it does conflict - I will update the patch to fix these comments :) Thank you [-- Attachment #2: Type: text/html, Size: 113 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 6:59 ` Sean Rhodes @ 2022-05-16 7:36 ` Wu, Hao A 2022-05-16 7:53 ` Sean Rhodes 0 siblings, 1 reply; 10+ messages in thread From: Wu, Hao A @ 2022-05-16 7:36 UTC (permalink / raw) To: devel@edk2.groups.io, Rhodes, Sean [-- Attachment #1: Type: text/plain, Size: 550 bytes --] Sorry for not being clear on what I mean. Is it possible to change the platform PCD values and keep these block size alignment requirements. Best Regards, Hao Wu From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes Sent: Monday, May 16, 2022 3:00 PM To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Hi Hao Yes, it does conflict - I will update the patch to fix these comments :) Thank you [-- Attachment #2: Type: text/html, Size: 3051 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 7:36 ` Wu, Hao A @ 2022-05-16 7:53 ` Sean Rhodes 2022-05-16 9:03 ` Wu, Hao A 0 siblings, 1 reply; 10+ messages in thread From: Sean Rhodes @ 2022-05-16 7:53 UTC (permalink / raw) To: Wu, Hao A; +Cc: devel@edk2.groups.io [-- Attachment #1: Type: text/plain, Size: 979 bytes --] The bug discovered was with coreboot, and the PCD values are derived from the block size of its SMMStore (NvStorage) region. The discussion on the patch can be found here: https://review.coreboot.org/c/coreboot/+/62990 Hacking the PCDs could work,, but why would we want to keep an incorrect check? Thanks! On Mon, 16 May 2022 at 08:36, Wu, Hao A <hao.a.wu@intel.com> wrote: > Sorry for not being clear on what I mean. > > Is it possible to change the platform PCD values and keep these block size > alignment requirements. > > > > Best Regards, > > Hao Wu > > > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Sean > Rhodes > *Sent:* Monday, May 16, 2022 3:00 PM > *To:* Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > Hi Hao > > Yes, it does conflict - I will update the patch to fix these comments :) > > Thank you > > > [-- Attachment #2: Type: text/html, Size: 2920 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 7:53 ` Sean Rhodes @ 2022-05-16 9:03 ` Wu, Hao A 2022-05-17 11:58 ` Sheng Lean Tan 0 siblings, 1 reply; 10+ messages in thread From: Wu, Hao A @ 2022-05-16 9:03 UTC (permalink / raw) To: Zeng, Star, Gao, Liming, devel@edk2.groups.io, Rhodes, Sean [-- Attachment #1: Type: text/plain, Size: 1620 bytes --] Sorry Star and Liming, For the below patch (removing the alignment check for WorkSpace & SpareArea): https://edk2.groups.io/g/devel/message/89742 Do you think it will impact the FTW service on flash device? Thanks in advance. Best Regards, Hao Wu From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes Sent: Monday, May 16, 2022 3:54 PM To: Wu, Hao A <hao.a.wu@intel.com> Cc: devel@edk2.groups.io Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment The bug discovered was with coreboot, and the PCD values are derived from the block size of its SMMStore (NvStorage) region. The discussion on the patch can be found here: https://review.coreboot.org/c/coreboot/+/62990 Hacking the PCDs could work,, but why would we want to keep an incorrect check? Thanks! On Mon, 16 May 2022 at 08:36, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: Sorry for not being clear on what I mean. Is it possible to change the platform PCD values and keep these block size alignment requirements. Best Regards, Hao Wu From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes Sent: Monday, May 16, 2022 3:00 PM To: Wu; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Hi Hao Yes, it does conflict - I will update the patch to fix these comments :) Thank you [-- Attachment #2: Type: text/html, Size: 6601 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-16 9:03 ` Wu, Hao A @ 2022-05-17 11:58 ` Sheng Lean Tan 2022-05-17 15:05 ` Zeng, Star 0 siblings, 1 reply; 10+ messages in thread From: Sheng Lean Tan @ 2022-05-17 11:58 UTC (permalink / raw) To: devel, hao.a.wu; +Cc: Zeng, Star, Gao, Liming, Rhodes, Sean [-- Attachment #1: Type: text/plain, Size: 2264 bytes --] Hi Star & Liming, Any update on this? Much appreciated. Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Mon, 16 May 2022 at 11:03, Wu, Hao A <hao.a.wu@intel.com> wrote: > Sorry Star and Liming, > > > > For the below patch (removing the alignment check for WorkSpace & > SpareArea): > > https://edk2.groups.io/g/devel/message/89742 > > > > Do you think it will impact the FTW service on flash device? Thanks in > advance. > > > > Best Regards, > > Hao Wu > > > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Sean > Rhodes > *Sent:* Monday, May 16, 2022 3:54 PM > *To:* Wu, Hao A <hao.a.wu@intel.com> > *Cc:* devel@edk2.groups.io > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > The bug discovered was with coreboot, and the PCD values are derived from > the block size of its SMMStore (NvStorage) region. The discussion on the > patch can be found here: https://review.coreboot.org/c/coreboot/+/62990 > > > > Hacking the PCDs could work,, but why would we want to keep an incorrect > check? > > > > Thanks! > > > > > > On Mon, 16 May 2022 at 08:36, Wu, Hao A <hao.a.wu@intel.com> wrote: > > Sorry for not being clear on what I mean. > > Is it possible to change the platform PCD values and keep these block size > alignment requirements. > > > > Best Regards, > > Hao Wu > > > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Sean > Rhodes > *Sent:* Monday, May 16, 2022 3:00 PM > *To:* Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > Hi Hao > > Yes, it does conflict - I will update the patch to fix these comments :) > > Thank you > > > > [-- Attachment #2: Type: text/html, Size: 7702 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-17 11:58 ` Sheng Lean Tan @ 2022-05-17 15:05 ` Zeng, Star 2022-05-17 15:16 ` Sean Rhodes 0 siblings, 1 reply; 10+ messages in thread From: Zeng, Star @ 2022-05-17 15:05 UTC (permalink / raw) To: Tan, Lean Sheng, devel@edk2.groups.io, Wu, Hao A Cc: Gao, Liming, Rhodes, Sean, Zeng, Star [-- Attachment #1: Type: text/plain, Size: 3154 bytes --] When length is larger than block size and block size aligned, if the address is not block size aligned, that means the range will mix with other range, but erase operation will be done per block, that will be risky and may break the fault tolerant mechanism. I could not remember all the details. Personally, I do not think it is right way to remove the check. Thanks, Star From: Lean Sheng Tan <sheng.tan@9elements.com> Sent: Tuesday, May 17, 2022 7:58 PM To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com> Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Rhodes, Sean <sean@starlabs.systems> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Hi Star & Liming, Any update on this? Much appreciated. Best Regards, Lean Sheng Tan [http://static.9elements.com/logo-signature.png] 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com<mailto:sheng.tan@9elements.com> Phone: +49 234 68 94 188<tel:+492346894188> Mobile: +49 176 76 113842<tel:+4917676113842> Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR<https://9elements.com/privacy> On Mon, 16 May 2022 at 11:03, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: Sorry Star and Liming, For the below patch (removing the alignment check for WorkSpace & SpareArea): https://edk2.groups.io/g/devel/message/89742 Do you think it will impact the FTW service on flash device? Thanks in advance. Best Regards, Hao Wu From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes Sent: Monday, May 16, 2022 3:54 PM To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment The bug discovered was with coreboot, and the PCD values are derived from the block size of its SMMStore (NvStorage) region. The discussion on the patch can be found here: https://review.coreboot.org/c/coreboot/+/62990 Hacking the PCDs could work,, but why would we want to keep an incorrect check? Thanks! On Mon, 16 May 2022 at 08:36, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: Sorry for not being clear on what I mean. Is it possible to change the platform PCD values and keep these block size alignment requirements. Best Regards, Hao Wu From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes Sent: Monday, May 16, 2022 3:00 PM To: Wu; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Hi Hao Yes, it does conflict - I will update the patch to fix these comments :) Thank you [-- Attachment #2: Type: text/html, Size: 14065 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-17 15:05 ` Zeng, Star @ 2022-05-17 15:16 ` Sean Rhodes 2022-10-03 9:53 ` Sheng Lean Tan 0 siblings, 1 reply; 10+ messages in thread From: Sean Rhodes @ 2022-05-17 15:16 UTC (permalink / raw) To: Zeng, Star; +Cc: Tan, Lean Sheng, devel@edk2.groups.io, Wu, Hao A, Gao, Liming [-- Attachment #1: Type: text/plain, Size: 4177 bytes --] Hi Star I think the point is shown in a comment from coreboot: "As mentioned above, only the offsets need to be aligned, not the absolute bases. Please, have a look for instance at `MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c:1111`: (FtwDevice->WorkSpaceAddress >= (FvbBaseAddress + BlockSize * (LbaIndex - 1))) Things become more obvious if we remove the unnecessary parentheses: FtwDevice->WorkSpaceAddress >= FvbBaseAddress + BlockSize * (LbaIndex - 1) It's the same as: FtwDevice->WorkSpaceAddress - FvbBaseAddress >= BlockSize * (LbaIndex - 1) And _if_ aligned, the same as: (FtwDevice->WorkSpaceAddress - FvbBaseAddress) / BlockSize >= LbaIndex - 1 Now it's easy to see: neither `FtwDevice->WorkSpaceAddress` nor `FvbBaseAddress` have to be aligned, but their relative distance has to be." So if this solution isn't acceptable, could you suggest one that would be? Many thanks On Tue, 17 May 2022 at 16:05, Zeng, Star <star.zeng@intel.com> wrote: > When length is larger than block size and block size aligned, if the > address is not block size aligned, that means the range will mix with other > range, but erase operation will be done per block, that will be risky and > may break the fault tolerant mechanism. > > I could not remember all the details. Personally, I do not think it is > right way to remove the check. > > > > > > Thanks, > > Star > > *From:* Lean Sheng Tan <sheng.tan@9elements.com> > *Sent:* Tuesday, May 17, 2022 7:58 PM > *To:* devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com> > *Cc:* Zeng, Star <star.zeng@intel.com>; Gao, Liming < > gaoliming@byosoft.com.cn>; Rhodes, Sean <sean@starlabs.systems> > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > Hi Star & Liming, > > Any update on this? > > Much appreciated. > > > Best Regards, > > *Lean Sheng Tan* > > > 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany > > Email: sheng.tan@9elements.com > > Phone: *+49 234 68 94 188 <+492346894188>* > > Mobile: *+49 176 76 113842 <+4917676113842>* > > > > Registered office: Bochum > > Commercial register: Amtsgericht Bochum, HRB 17519 > > Management: Sebastian German, Eray Bazaar > > > Data protection information according to Art. 13 GDPR > <https://9elements.com/privacy> > > > > > > On Mon, 16 May 2022 at 11:03, Wu, Hao A <hao.a.wu@intel.com> wrote: > > Sorry Star and Liming, > > > > For the below patch (removing the alignment check for WorkSpace & > SpareArea): > > https://edk2.groups.io/g/devel/message/89742 > > > > Do you think it will impact the FTW service on flash device? Thanks in > advance. > > > > Best Regards, > > Hao Wu > > > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Sean > Rhodes > *Sent:* Monday, May 16, 2022 3:54 PM > *To:* Wu, Hao A <hao.a.wu@intel.com> > *Cc:* devel@edk2.groups.io > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > The bug discovered was with coreboot, and the PCD values are derived from > the block size of its SMMStore (NvStorage) region. The discussion on the > patch can be found here: https://review.coreboot.org/c/coreboot/+/62990 > > > > Hacking the PCDs could work,, but why would we want to keep an incorrect > check? > > > > Thanks! > > > > > > On Mon, 16 May 2022 at 08:36, Wu, Hao A <hao.a.wu@intel.com> wrote: > > Sorry for not being clear on what I mean. > > Is it possible to change the platform PCD values and keep these block size > alignment requirements. > > > > Best Regards, > > Hao Wu > > > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Sean > Rhodes > *Sent:* Monday, May 16, 2022 3:00 PM > *To:* Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io > *Subject:* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: > Don't check for address alignment > > > > Hi Hao > > Yes, it does conflict - I will update the patch to fix these comments :) > > Thank you > > > > [-- Attachment #2: Type: text/html, Size: 13082 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment 2022-05-17 15:16 ` Sean Rhodes @ 2022-10-03 9:53 ` Sheng Lean Tan 0 siblings, 0 replies; 10+ messages in thread From: Sheng Lean Tan @ 2022-10-03 9:53 UTC (permalink / raw) To: Sean Rhodes, devel [-- Attachment #1: Type: text/plain, Size: 47 bytes --] Hi Wu Hao/ Zeng Star, Any update on this? :) [-- Attachment #2: Type: text/html, Size: 51 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-10-03 9:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <16E1B70DABEEDF46.30116@groups.io> 2022-05-16 5:41 ` [edk2-devel] [PATCH] MdeModulePkg/FaultTolerantWriteDxe: Don't check for address alignment Sean Rhodes 2022-05-16 6:54 ` Wu, Hao A 2022-05-16 6:59 ` Sean Rhodes 2022-05-16 7:36 ` Wu, Hao A 2022-05-16 7:53 ` Sean Rhodes 2022-05-16 9:03 ` Wu, Hao A 2022-05-17 11:58 ` Sheng Lean Tan 2022-05-17 15:05 ` Zeng, Star 2022-05-17 15:16 ` Sean Rhodes 2022-10-03 9:53 ` Sheng Lean Tan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox