[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCHv3 5/7] plat/common: Find and register IRQ for arch_timer
Hello, On 9/17/19 12:44 PM, Julien Grall wrote: Hi, On 9/17/19 11:08 AM, Sharan Santhanam wrote:On 9/17/19 11:17 AM, Julien Grall wrote:On 9/17/19 9:44 AM, Justin He (Arm Technology China) wrote:Hi JulienHi,-----Original Message----- From: Julien Grall <julien.grall@xxxxxxx> Sent: 2019年9月17日 16:39 To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; SantiagoPagani <Santiago.Pagani@xxxxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;Simon Kuenzer <simon.kuenzer@xxxxxxxxx>; Sharan Santhanam <Sharan.Santhanam@xxxxxxxxx> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; Jianyong Wu (Arm Technology China) <Jianyong.Wu@xxxxxxx>; nd <nd@xxxxxxx>Subject: Re: [UNIKRAFT PATCHv3 5/7] plat/common: Find and register IRQfor arch_timer On 9/17/19 8:01 AM, Justin He (Arm Technology China) wrote:Hi Julien (welcome back from holiday 😊 )Hi, Thanks :).-----Original Message----- From: Julien Grall <julien.grall@xxxxxxx> Sent: 2019年9月17日 3:53 To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; SantiagoPagani <Santiago.Pagani@xxxxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;Simon Kuenzer <simon.kuenzer@xxxxxxxxx>; Sharan Santhanam <Sharan.Santhanam@xxxxxxxxx> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; Wei Chen(ArmTechnology China) <Wei.Chen@xxxxxxx>; Jianyong Wu (ArmTechnologyChina) <Jianyong.Wu@xxxxxxx> Subject: Re: [UNIKRAFT PATCHv3 5/7] plat/common: Find and registerIRQfor arch_timer On 9/16/19 8:52 AM, Justin He (Arm Technology China) wrote:Hi SantiagoHi all,@Santiago, it is quite difficult to follow the thread when you startyour answer with "COMMENT". May I ask you to configure your e-mail client to quote properly (i.e >)?Furthermore, disclaimer footer should be avoided on the mailing list. You are basically saying this is confidential but you send to everyone(mailing list are archived)...OKIt wasn't directed to you ;).[...]COMMENT: There is nothing that we would like to do here? Not even disable the IRQ? As the timer is not stopped, when the counteroverflowswe would get a new interrupt otherwise (although the overflow couldhappen in a very very long time, right?)In previous version, we added a generic_timer_mask_irq() ingeneric_timer_irq_handler. But as per the suggestion [1] from Julien,weremoved it. Besides, we referred to the minios logic at [2], it only calledunmask and mask in block_domain (which is equivalent to unikraft's generic_timer_cpu_block)Looking at my comments again, I am not sure where I suggested toremoveOkay... sorry for my mistakes. I will add generic_timer_mask_irq() back.generic_timer_mask_irq()... Can you expand it?FWIW, the two main comments on the previous versions were:1) isb() should be added after updating the system register toensure that the system system is synchronized2) This is common code between arm32 and arm64. But the systemregister name are arm64... Accesses should be stub in arch-specific header so the code can work for both arm32 and arm64.I renamed plat/common/arm/time.c to plat/common/arm/time_arm64.cSeems that is not enough for you? If no, I have no objections to makea stub for arm32.Well, the only bits arm64 specifics in this file are the access to thesystem registers. So renaming to time_arm64.c seems a bit overkill... If there are plan to make arm32 a correct port on Unikraft, thensplitting the code would be the best. If there are no plan to get arm32,then maybe you should think of killing it completely.Arm32 xen plat is initially supported but no one has touched that for a long time. Currently let’s focus on arm64 kvm plat only. If the requirements changes,we can support arm32 additionally. What do you think about it?I am not asking to implement arm32, I am only suggesting to try to split the code rather than trying to mix common code vs arch specific code in plat/common/arm. That directory in particular is looking messier and messier as new series are posted.I agree with Julien it is better to split the arm32 code from the arm64 code. My suggestion would beplat/common/arm for 32-bit code plat/common/arm64 for the 64-bit.Well you can share a lot of code between 32-bit and 64-bit. If we take the example of the arch timer, the only difference is the way to access the registers (i.e. system registers vs co-processor registers). Since it is primarily about the co processor and system register. How about pushing the functionality into the respective header. plat/common/include/arm/time.h The header includes arch specific header files. plat/common/include/arm/arm64/time.hProvides a architecture specific implementation for reading system registers while providing a macro definition for reading register like: #define el0_cntv_ctl_get SYSREG_READ32(cntv_ctl_el0) #define el0_cntv_ctl_set(val) SYSREG_WRITE32(cntv_ctl_el0, val) plat/common/include/arm/arm/time.hProvide a similar interface to the arm32 arch. For now we define the macro as an empty implementation to make it compile on the arm32 with a "#warning" to indicate incomplete implementation. So it would be best to consider: plat/common/arm for anything common between arm32 and arm64 plat/common/arm/arm32 for anything arm32 specific plat/common/arm/arm64 for anything arm64 specific The directory arm{32,64} should be pretty empty. Cheers, _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |