Hi Kevin,

 

Thanks for the fix.

 

 

Yeah, I think the easiest way is to store base and size in sau_cfg and then calculate RLAR value and do the -1 inside sau_and_idau_cfg() function. This should remove inconsistency and have a straightforward design.

 

Regards,

Bohdan Hunko

 

Cypress Semiconductor Ukraine

Engineer

CSUKR CSS ICW SW FW

Mobile: +38099 50 19 714
Bohdan.Hunko@infineon.com

 

 

From: Kevin Peng <Kevin.Peng@arm.com>
Sent: 28 December 2022 08:06
To: Hunko Bohdan (CSUKR CSS ICW SW FW 3) <Bohdan.Hunko@infineon.com>; Ken Liu <Ken.Liu@arm.com>; tf-m@lists.trustedfirmware.org
Cc: nd <nd@arm.com>
Subject: RE: minor bug in an521 protections

 

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

 

Hi Bohdan,

 

Thanks for spotting this.

I went through all the SAU configurations for Arm platforms and did find some missing “-1”.

Basically they are all veneer regions.

Other regions such as NS_DATA_START, DEV_APIS_TEST_NVMEM_REGION_START, they are already defined with “-1”.

Ideally we should keep consistencies for the region settings regarding where the “-1” should be done.

Let’s fix the bug first.

 

Best Regards,

Kevin

 

From: Bohdan.Hunko@infineon.com <Bohdan.Hunko@infineon.com>
Sent: Friday, December 23, 2022 18:34
To: Kevin Peng <Kevin.Peng@arm.com>; Ken Liu <Ken.Liu@arm.com>; tf-m@lists.trustedfirmware.org
Cc: nd <nd@arm.com>
Subject: RE: minor bug in an521 protections

 

Hi all,

 

If -1 in NS_PARTITION_START is intentional why then there is no -1 in same array for some other entries like  NS_DATA_START, DEV_APIS_TEST_NVMEM_REGION_START.

 

-1 should either is always be needed or always not needed.

 

From Kevin Peng`s explanation looks like it is always needed, so there still is a bug in cases where -1 is not subtracted.  If so I think the simpliest solution would be to do -1 when we actually assign to RLAR register and keep array entries without -1.

 

 

Regards,

Bohdan Hunko

 

Cypress Semiconductor Ukraine

Engineer

CSUKR CSS ICW SW FW

Mobile: +38099 50 19 714
Bohdan.Hunko@infineon.com

 

 

From: Kevin Peng via TF-M <tf-m@lists.trustedfirmware.org>
Sent: 23 December 2022 05:04
To: Ken Liu <Ken.Liu@arm.com>; tf-m@lists.trustedfirmware.org
Cc: nd <nd@arm.com>
Subject: [TF-M] Re: minor bug in an521 protections

 

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

 

Hi Bohdan,

 

I believe the “-1” is intentional because the SAU does a round-up on limit address.

 

From the Armv8m spec:

 

When the SAU is enabled, an address is defined as matching a region in the SAU if the following is true:

Address >= SAU_REGIONn.BADDR: '00000' && Address <= SAU_REGIONn.LADDR: '11111'.

 

In your example, 0x1000_0000 + 0x1000 – 1 = 0x10000FFF

The LADDR limit address written to SAU_REGIONn.LADDR will be bits 31:5 of 0x1000_0FE0 (only bits 31:5 stand for the address)

(Base address would be 0x1000_0000.)

Then the above match would be

Address >= 0x1000_0000 && Address <= 0x1000_0FFF (Bit 31:5 of 0x1000_0FE0:’11111’)

Which matches the expectation.

 

If you didn’t do a “-1”, then the match would be:

Bit 31:5 of 0x1000_1000 written to LADDR bits of SAU_REGIONn.LADDR.

Address >= 0x1000_0000 && Address <= 0x1000_101F (Bit 31:5 of 0x1000_1000:’11111’)

Which would include 32 bytes more than expectation.

 

Best Regards,

Kevin

 

From: Ken Liu via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Friday, December 23, 2022 10:04
To: tf-m@lists.trustedfirmware.org
Cc: nd <nd@arm.com>
Subject: [TF-M] Re: minor bug in an521 protections

 

Hi Bohdan,

 

Thanks for reporting this, will go through all SAU related settings.

 

/Ken

 

From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Friday, December 23, 2022 4:59 AM
To: tf-m@lists.trustedfirmware.org
Subject: [TF-M] minor bug in an521 protections

 

Hi,

 

I have been working on TZ stuff recently and found small problem in an521 platform protections.

The problem is in SAU config.

sau_cfg in platform/ext/target/arm/mps2/an521/target_cfg.c has entry for NSPE code:

    {

        ((uint32_t)NS_PARTITION_START),

        ((uint32_t)NS_PARTITION_START +

        NS_PARTITION_SIZE - 1),

        false,

    },

Where both NS_PARTITION_START and NS_PARTITION_SIZE are 32 bytes aligned, which means that when 1 is subtracted lower 5 bits are getting set to 1, for example:

0x1000_0000 + 0x1000 – 1 = 0x10000FFF

Then in sau_and_idau_cfg() function lower 5 bits are cleared by the mask:

sau_cfg[i].RLAR & SAU_RLAR_LADDR_Msk

 

This means that in reality highest 32 bytes of NSPE are protected as Secure in SAU.

 

Same problem is present for SECONDARY_PARTITION_SIZE SAU entry.

 

This is not huge problem, but still worth fixing.

 

I believe other arm and TZ platforms may also have this bug, but I haven’t checked in details.

 

 

Regards,

Bohdan Hunko

 

Cypress Semiconductor Ukraine

Engineer

CSUKR CSS ICW SW FW

Mobile: +38099 50 19 714
Bohdan.Hunko@infineon.com