Add warning about FEATURE_UVISOR being deprecated #7000

danny4478 · 2018-05-24T08:49:35Z

Description

Add warning about FEATURE_UVISOR being deprecated.
If FEATURE_UVISOR is being used (compiled), then a compilation warning is issued.

Pull request type

[ ] Fix
[ x ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

0xc0170 · 2018-05-24T09:02:59Z

features/FEATURE_UVISOR/includes/uvisor/api/inc/uvisor_deprecation.h

+#define __UVISOR_DEPRECATION_H__
+
+#if defined(UVISOR_PRESENT) && UVISOR_PRESENT == 1
+#warning "---- WARNING: You are using FEATURE_UVISOR which is deprecated!! ----"


You are using FEATURE_UVISOR which is deprecated since mbed-os-5.9 might be better - follow what we do for API - since what release it is deprecated.

Lets wait for @AnotherButler review

Agree with @0xc0170 Uvisor is NOT yet deprecated. The warning should state that the feature is going to be deprecated in 5.10...

Can we hold off on warning messages? We're reviewing all of them. Also saying "it's deprecated' without saying what to use instead leaves the user with nothing.

@simonfordarm

Can we hold off on warning messages? We're reviewing all of them. Also saying "it's deprecated' without saying what to use instead leaves the user with nothing.

@alzix is trying to resolve this to add this here to the message and to the commit message as well.

Can we hold off on warning messages?

What do you suggest?

adbridge · 2018-05-24T10:48:07Z

@danny4478 I just spoke to Sam Taylor about this deprecation. He is of the opinion that we have to be very careful with this deprecation message and the text needs to be agreed by Nicholas and probably Aaron also before being submitted to the code base.

danny4478 · 2018-05-24T10:51:56Z

@ndevillard please advise

AnotherButler · 2018-05-24T14:53:16Z

features/FEATURE_UVISOR/includes/uvisor/api/inc/uvisor_deprecation.h

+#define __UVISOR_DEPRECATION_H__
+
+#if defined(UVISOR_PRESENT) && UVISOR_PRESENT == 1
+#warning "---- WARNING: You are using FEATURE_UVISOR which is deprecated since mbed-os-5.9!! ----"


Perhaps something like:

"Warning: In 5.10, Arm Mbed uVisor is being deprecated in favor of our upcoming security features. We strongly encourage you to use these new security features instead."

I'd like to link to mention and link to PSA or PSA crypto, but I don't know if linking to a private repo makes sense.

@ndevillard @ChiefBureaucraticOfficer Please advise.

After talking with @ChiefBureaucraticOfficer we suggest "Warning: You are using FEATURE_UVISOR, which is unsupported as of Mbed OS 5.9."

@ndevillard @simonfordarm What are your thoughts on this phrasing?

cmonr · 2018-05-25T00:54:54Z

Due to having not seen a deprecation warning be agreed upon in the last nine hours, I've taken @AnotherButler's most recent suggestion and updated this PR with the message.

Going to put this through CI to make sure no surprises come up with the file addition, but will hold off on merging until key parties agree on the text. Also intend on taking advantage of the low CI utilization at the moment.

Fyi, a change to the text (aka another commit) will require CI again.

/morph build

mbed-ci · 2018-05-25T03:03:02Z

Build : SUCCESS

Build number : 2145
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7000/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-05-25T05:02:16Z

Test : FAILURE

Build number : 1939
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7000/1939

0xc0170 · 2018-05-25T07:27:21Z

/morph test

mbed-ci · 2018-05-25T08:54:12Z

Test : SUCCESS

Build number : 1941
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7000/1941

mbed-ci · 2018-05-25T09:08:17Z

Exporter Build : SUCCESS

Build number : 1774
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7000/

0xc0170 · 2018-05-25T09:37:20Z

CI completed, this is now almost green to get in.

Reviewers - approvals?

adbridge · 2018-05-25T11:40:12Z

Looking at the email from @ndevillard this warning is insufficient.

Also uvisor will still be in 5.9 so won't actually be fully deprecated until 5.10.
I'm wondering if it should say something like:

"uvisor will be deprecated in the 5.10 release and will be replaced in due course by a PSA-compliant Secure Partition Manager"

adbridge · 2018-05-25T12:09:21Z

Agreed with @ChiefBureaucraticOfficer that this can go in as is for RC1 and we will improve it for RC2

adbridge · 2018-05-31T10:27:34Z

Looking at the email from @ndevillard this warning is insufficient.
I'm wondering if it should say something like:
"uvisor will be deprecated in the 5.10 release and will be replaced in due course by a PSA-compliant >Secure Partition Manager"
Agreed with @ChiefBureaucraticOfficer that this can go in as is for RC1 and we will improve it for RC2

@danny4478 @ndevillard
Can we please have a PR with the improved warning text asap so that we can get it into RC2 as agreed?

iriark01 · 2018-05-31T10:28:39Z

Warning: uVisor is superseded by the Secure Partition Manager (SPM) defined in the ARM Platform Security Architecture (PSA). uVisor is deprecated as of Mbed OS 5.9, and being replaced by a native PSA-compliant implementation of SPM.

I think you can use that one

alzix · 2018-05-31T11:50:38Z

@orenc17 please handle asap

orenc17 · 2018-05-31T12:40:16Z

Created #7072

0xc0170 added the needs: review label May 24, 2018

0xc0170 requested a review from a team May 24, 2018 08:55

0xc0170 requested changes May 24, 2018

View reviewed changes

0xc0170 requested a review from AnotherButler May 24, 2018 09:03

0xc0170 added the release-version: 5.9.0-rc1 label May 24, 2018

adbridge self-requested a review May 24, 2018 10:26

Add warning about FEATURE_UVISOR being deprecated

aa38778

AnotherButler requested review from a user and ndevillard May 24, 2018 14:47

AnotherButler reviewed May 24, 2018

View reviewed changes

Picking a deprecation warning and moving forward

25fb8e3

cmonr added needs: CI and removed needs: review labels May 25, 2018

0xc0170 approved these changes May 25, 2018

View reviewed changes

0xc0170 added needs: review and removed needs: CI labels May 25, 2018

adbridge added the risk: A label May 25, 2018

adbridge added ready for merge risk: G labels May 25, 2018

adbridge removed risk: A needs: review labels May 25, 2018

cmonr merged commit a348b45 into ARMmbed:master May 25, 2018

cmonr removed risk: G ready for merge labels May 25, 2018

alzix deleted the uvisor_depr branch May 31, 2018 12:27

Add warning about FEATURE_UVISOR being deprecated #7000

Add warning about FEATURE_UVISOR being deprecated #7000

Uh oh!

Conversation

danny4478 commented May 24, 2018

Description

Pull request type

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adbridge commented May 24, 2018

Uh oh!

danny4478 commented May 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented May 25, 2018

Uh oh!

mbed-ci commented May 25, 2018

Triggering tests

Uh oh!

mbed-ci commented May 25, 2018

Uh oh!

0xc0170 commented May 25, 2018

Uh oh!

mbed-ci commented May 25, 2018

Uh oh!

mbed-ci commented May 25, 2018

Uh oh!

0xc0170 commented May 25, 2018

Uh oh!

adbridge commented May 25, 2018 • edited by 0xc0170 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented May 25, 2018

Uh oh!

adbridge commented May 31, 2018

Uh oh!

iriark01 commented May 31, 2018

Uh oh!

alzix commented May 31, 2018

Uh oh!

orenc17 commented May 31, 2018

Uh oh!

Uh oh!

adbridge commented May 25, 2018 •

edited by 0xc0170

Loading