Hi Raghu and Varun,
This patch is a complete, tested and verified reference implementation for FVP platform. Splitting it will create a set of separate non-buildable patches causing more complexity in following and understanding the code changes and dependencies. The whole patch with all the code present in it should be reviewed anyway, and the time spent will be less than the time used for reviewing separate patches (mass defect).
Alexei
________________________________ From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Raghu Krishnamurthy via TF-A tf-a@lists.trustedfirmware.org Sent: 02 April 2020 05:11 To: tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Subject: Re: [TF-A] Event Log for Measured Boot
Hi Alexei,
I second Varun on this. The patch is huge. I recommend breaking it up into multiple commits. I've reviewed it but since it is a large patch, it might require a few more sittings to grasp all the changes(which also means there may be some stupid review comments :)).
-Raghu
On 3/31/20 10:28 AM, Varun Wadekar via TF-A wrote:
Hello Alexei,
Just curious, the patch is huge and will take some time to review. Do you expect this change to be merged before the v2.3 release?
-Varun
*From:* TF-A tf-a-bounces@lists.trustedfirmware.org *On Behalf Of *Alexei Fedorov via TF-A *Sent:* Tuesday, March 31, 2020 7:19 AM *To:* tf-a@lists.trustedfirmware.org *Subject:* [TF-A] Event Log for Measured Boot
*External email: Use caution opening links or attachments*
Hi,
Please review and provide your comments for the patch which adds
Event Log generation for the Measured Boot.
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3806
Thanks.
Alexei
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Fair enough. I have no doubt it was tested. It is just extremely difficult to review such patches and I disagree with your statement. There is almost always a way to split patches up by using feature flags for example, that will help with not breaking the build. You can test them all together once you have all the patches. I also think it is perfectly reasonable to say measured boot cannot be turned on until a certain commit id present. However, if you think this is the right approach, i have no issues.
Thanks Raghu
On 4/20/20 8:44 AM, Alexei Fedorov wrote:
Hi Raghu and Varun,
This patch is a complete, tested and verified reference implementation for FVP platform. Splitting it will create a set of separate non-buildable patches causing more complexity in following and understanding the code changes and dependencies. The whole patch with all the code present in it should be reviewed anyway, and the time spent will be less than the time used for reviewing separate patches (mass defect).
Alexei
*From:* TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Raghu Krishnamurthy via TF-A tf-a@lists.trustedfirmware.org *Sent:* 02 April 2020 05:11 *To:* tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org *Subject:* Re: [TF-A] Event Log for Measured Boot Hi Alexei,
I second Varun on this. The patch is huge. I recommend breaking it up into multiple commits. I've reviewed it but since it is a large patch, it might require a few more sittings to grasp all the changes(which also means there may be some stupid review comments :)).
-Raghu
On 3/31/20 10:28 AM, Varun Wadekar via TF-A wrote:
Hello Alexei,
Just curious, the patch is huge and will take some time to review. Do you expect this change to be merged before the v2.3 release?
-Varun
*From:* TF-A tf-a-bounces@lists.trustedfirmware.org *On Behalf Of *Alexei Fedorov via TF-A *Sent:* Tuesday, March 31, 2020 7:19 AM *To:* tf-a@lists.trustedfirmware.org *Subject:* [TF-A] Event Log for Measured Boot
*External email: Use caution opening links or attachments*
Hi,
Please review and provide your comments for the patch which adds
Event Log generation for the Measured Boot.
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3806
Thanks.
Alexei
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
tf-a@lists.trustedfirmware.org
-
Alexei Fedorov -
Raghu Krishnamurthy