The word “limit” seems to be well defined. Looking in a number of online dictionaries, they all seem to agree on something like “the maximum or minimum allowable amount, quantity, or number”. That fits with my experience with things like ATM withdrawal limits, speed limits, etc.

 

I think we’re setting the various $$Limit variables to the wrong values (or naming them wrong). If they were the actual limits, then these “-1”s wouldn’t be needed.

 

Chris

 

From: Kevin Peng <Kevin.Peng@arm.com>
Sent: January 27, 2023 10:38 PM
To: David Brown <david.brown@linaro.org>; Brand Chris (CSCA CSS ICW SW PSW 1) <Chris.Brand@infineon.com>
Cc: tf-m@lists.trustedfirmware.org
Subject: RE: [TF-M] Re: Region Limits

 

Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safe.

 

Hi,

 

There was already a discussion on the SAU config issues that covers this discussion.

https://lists.trustedfirmware.org/archives/list/tf-m@lists.trustedfirmware.org/thread/ZYU5ZDUJVVKWWXYBO7GSBJE2CKCL2XNB/

And a reminder to check SAU limit address settings:

https://lists.trustedfirmware.org/archives/list/tf-m@lists.trustedfirmware.org/thread/7ZQKWELE2IA2SDG3W5CDKFLYK7EP4NLF/

 

The “-1” could be particular to SAU because SAU does a round-up on limit address.

That’s how it works. Please refer to the Arm v8m architecture specification or the examples in the above link.

Although it’s hard to define what is a “limit” because the limit address can be either included or excluded in different use cases, I do agree putting a “-1” in any region definitions could be confusing.

 

Like Bohdan suggested, we could move this “-1” to the SAU configuration codes.

And even provide some utility Macros for SAU region operations to ensure both the alignments “limits/boundaries” .

 

Thanks,

Kevin

 

From: David Brown via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Friday, January 27, 2023 5:56 AM
To: Chris.Brand@infineon.com
Cc: tf-m@lists.trustedfirmware.org
Subject: [TF-M] Re: Region Limits

 

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