public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sean Rhodes" <sean@starlabs.systems>
To: "Zeng, Star" <star.zeng@intel.com>
Cc: "Tan, Lean Sheng" <sheng.tan@9elements.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	 "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
Date: Tue, 17 May 2022 16:16:20 +0100	[thread overview]
Message-ID: <CABtds-3ZCKDXxKLO7B2Hjsi+ctqR6xHHb7QoogmhRUVf8SYPDg@mail.gmail.com> (raw)
In-Reply-To: <DM5PR11MB124409E49007CD22F415F7C7E3CE9@DM5PR11MB1244.namprd11.prod.outlook.com>

[-- 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 --]

  reply	other threads:[~2022-05-17 15:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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 [this message]
2022-10-03  9:53                   ` Sheng Lean Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABtds-3ZCKDXxKLO7B2Hjsi+ctqR6xHHb7QoogmhRUVf8SYPDg@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox