Re: [Qemu-devel] [RFC PATCH 00/21] target-arm: add CPU core TrustZone support

[Peter Crosthwaite]

Hi Sergey,

On Wed, Dec 4, 2013 at 8:08 PM, Fedorov Sergey <address@hidden> wrote:
> On 12/03/2013 12:48 PM, Sergey Fedorov wrote:
>>
>> This patch set implements a basic support of CPU core TrustZone feature.
>> The
>> following major functionalities are implemented:
>>    * CPU monitor mode
>>    * Separate code translation for each secure state
>>    * CPACR & NSACR co-processor access control
>>    * Separate TLB for each secure state
>>    * Co-processor register banking
>>    * SMC instruction
>>    * FIQ/IRQ routing to monitor mode
>>
>> There is no support for banked co-processor register migration, save/load
>> its
>> VM state yet. That is an open question how to implement this
>> functionality. Any
>> suggestions is greatly appreciated.
>>
>> This patch set is a request for comments for the proof of concept.
>>
>> Sergey Fedorov (18):
>>    target-arm: move SCR & VBAR into TrustZone register list
>>    target-arm: adjust TTBCR for TrustZone feature
>>    target-arm: add arm_is_secure() helper
>>    target-arm: reject switching to monitor mode from non-secure state
>>    target-arm: adjust arm_current_pl() for TrustZone
>>    target-arm: adjust SCR CP15 register access rights
>>    target-arm: add non-secure Translation Block flag
>>    target-arm: implement CPACR register logic
>>    target-arm: add NSACR support
>>    target-arm: add SDER definition
>>    target-arm: split TLB for secure state
>>    target-arm: add banked coprocessor register type
>>    target-arm: convert appropriate coprocessor registers to banked type
>>    target-arm: use c13_context field for CONTEXTIDR
>>    target-arm: switch banked CP registers
>>    target-arm: add MVBAR support
>>    target-arm: implement SMC instruction
>>    target-arm: implement IRQ/FIQ routing to Monitor mode
>>
>> Svetlana Fedoseeva (3):
>>    target-arm: add TrustZone CPU feature
>>    target-arm: preserve RAO/WI bits of ARMv7 SCTLR
>>    target-arm: add CPU Monitor mode
>>
>>   target-arm/cpu.c       |    6 +-
>>   target-arm/cpu.h       |  126 ++++++++++++----
>>   target-arm/helper.c    |  308 +++++++++++++++++++++++++++++---------
>>   target-arm/machine.c   |   12 +-
>>   target-arm/translate.c |  388
>> ++++++++++++++++++++++++++++++------------------
>>   target-arm/translate.h |    2 +
>>   6 files changed, 585 insertions(+), 257 deletions(-)
>>
>
> We'd like this patch series finally to be merged into mainstream. What
> should be done to achieve this goal?
>

Send patch series' just like this and either action or challenge
reviewer comments (as you have already done). Next time, you can drop
the RFC, that means you intend the series for review+merge. If you get
no reply after a week ping the series.

I'm about half way through the series on my first pass. And I guess
PMM has had a look as he has commented on one my comments.

A 21 patch series with +500/-250 is long, and queue maintainers are
generally hesitant to take partial series (as its often contrary to
author intention). Things generally go faster with smaller series. You
have some low-complexity cleanup type patches that dont add new
features than you could potentially send as a smaller initial series
to get upstream quickly, then take more time on the harder stuff as
follow up series with reduced patch count.

Regards,
Peter

> Best regards,
> Sergey Fedorov
>