[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 5/7] plat/kvm: Implement intctrl APIs for Arm64



Hello,

please find the view comment inline:

Thanks & Regards
Sharan

On 12/18/18 5:51 AM, Wei Chen (Arm Technology China) wrote:
Hi Julien,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxxxxx>
Sent: 2018年12月14日 18:51
To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
devel@xxxxxxxxxxxxxxxxxxxx; simon.kuenzer@xxxxxxxxx; florian.schmidt@xxxxxxxxx;
yuri.volchkov@xxxxxxxxx; Sharan.Santhanam@xxxxxxxxx; Felipe.Huici@xxxxxxxxx
Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>;
Jianyong Wu (Arm Technology China) <Jianyong.Wu@xxxxxxx>; Justin He (Arm
Technology China) <Justin.He@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCH 5/7] plat/kvm: Implement intctrl
APIs for Arm64

Hi Wei,

On 13/12/2018 09:18, Wei Chen wrote:
Before GICv2 become ready, we had marked the intctrl APIs as TODO.
Now, we have enabled the GICv2, we can implement intctrl APIs with
related GIC APIs.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
   plat/kvm/arm/intctrl.c | 25 ++++++++++++++++++-------
   plat/kvm/arm/setup.c   |  4 ++++
   2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/plat/kvm/arm/intctrl.c b/plat/kvm/arm/intctrl.c
index ac604a7..0662159 100644
--- a/plat/kvm/arm/intctrl.c
+++ b/plat/kvm/arm/intctrl.c
@@ -31,24 +31,35 @@
    *
    * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
    */
+#include <uk/assert.h>
+#include <kvm/kernel.h>
   #include <kvm/intctrl.h>
+#include <arm/cpu.h>
+#include <arm/irq.h>
+#include <arm/gic-v2.h>

   void intctrl_init(void)
   {
-       // TO DO
+       int ret;
+
+       /* Initialize GIC from DTB */
+       ret = _dtb_init_gic(_libkvmplat_dtb);
+       if (ret)
+               UK_CRASH("Initialize GIC from DTB failed, ret=%d\n", ret);
+
   }

-void intctrl_ack_irq(unsigned int irq)
+void intctrl_ack_irq(uint32_t irq)
   {
-       // TO DO
+       gic_eoi_irq(irq);

I think you want this helper to be a NOP. Otherwise you may end up to EOI
twice
the same interrupts (see patch #7).

You can't drop the one in patch #7 because this function may not be called
resulting to block the interrupts forever on that processor.


I think your suggestion is better. I had been confused by call eoi twice.
I would make this API as NOP


The handling of the interrupt has changed since the patch was submitted. the handler calls intctrl_ack_irq at the end of the interrupt handler. With the current implementation, I would rather remove the gic_eoi_irq in patch #7.

Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.