We have been seeing some issues with this specific value for some platforms. It also doesn't seem to handle the alignment properly, and I suspect the veneer region either ends up open to a lot more addresses than intended, or in other cases, doesn't work at all.

I can't find good documentation on how the SAU regions work, but if it is anything like the MPU regions, the validity would use something like:

if ((address & ~align) == base)

I'm concerned that a lot of testing is happening on qemu, which does the range testing with < and >, which is not how it is implemented in the hardware (I think, as I said, I haven't found any documentation on this).

But, yes, the limit needs to be the last valid address, regardless of alignment. Generally, the base needs to be a multiple of 1 greater than this value for the check to be meaningful.

David


On Thu, Jan 26, 2023 at 11:58 AM Chris.Brand--- via TF-M <tf-m@lists.trustedfirmware.org> wrote:

Hi,

 

Looking at platform/ext/common/tfm_hal_platform_v8m.c, this line stands out as quite odd:

    .veneer_limit =

        (uint32_t)&REGION_NAME(Image$$, VENEER_ALIGN, $$Limit) - 1,

 

It seems that the various “$$Limit” values that exist within TF-M aren’t actually “limits” in the usual meaning of the word (the last value within the range) but are instead the first value *outside* the range. Elsewhere in TF-M we do seem to use the word “limit” in the conventional way.

 

I worry that this could lead to errors around the boundaries, particularly as some functions we use to check ranges take a “limit” parameter – we could easily see code that looks correct but that actual does the wrong thing, like

ARM_MPC_ConfigRegion(REGION_NAME(Image$$, ER_VENEER, $$Base),

                                              REGION_NAME(Image$$, VENEER_ALIGN, $$Limit, attr);

Or a similar call to check_address_range().

 

For example, I suspect that this is a bug in the Nuvoton M2354 platform caused by this issue:

platform/ext/target/nuvoton/m2354/target_cfg.c:

    .veneer_limit = (uint32_t)&REGION_NAME(Image$$, VENEER_ALIGN, $$Limit),

 

It’s obviously a big thing to fix, but it does look to me to be worthwhile…

 

Chris Brand

 

Cypress Semiconductor (Canada), Inc.

An Infineon Technologies Company

Sr Prin Software Engr

CSCA CSS ICW SW PSW 1

Office: +1 778 234 0515

Chris.Brand@infineon.com

 

International Place 13700

V6V 2X8 Richmond

Canada

 

www.infineon.com  www.cypress.com  Discoveries  Facebook  Twitter  LinkedIn

 

Part of your life. Part of tomorrow.

 

NOTICE: The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material of Infineon Technologies AG and its affiliated entities which is for the exclusive use of the individual designated above as the recipient. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited. If you received this in error, please contact immediately the sender by returning e-mail and delete the material from any computer. If you are not the specified recipient, you are hereby notified that all disclosure, reproduction, distribution or action taken on the basis of this message is prohibited.

--
TF-M mailing list -- tf-m@lists.trustedfirmware.org
To unsubscribe send an email to tf-m-leave@lists.trustedfirmware.org