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

Re: [Xen-devel] [PATCH v2 07/17] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler

On 04/02/2018 04:33 PM, Manish Jaggi wrote:

On 03/27/2018 03:48 PM, Marc Zyngier wrote:
On 27/03/18 10:07, Manish Jaggi wrote:
This patch is ported to xen from linux commit
KVM: arm64: vgic-v3: Add ICV_EOIR1_EL1 handler

Add a handler for writing the guest's view of the ICC_EOIR1_EL1
register. This involves dropping the priority of the interrupt,
and deactivating it if required (EOImode == 0).

Signed-off-by : Manish Jaggi <manish.jaggi@xxxxxxxxxx>
  xen/arch/arm/arm64/vgic-v3-sr.c     | 136 ++++++++++++++++++++++++++++++++++++
  xen/include/asm-arm/arm64/sysregs.h |   1 +
  xen/include/asm-arm/gic_v3_defs.h   |   4 ++
  3 files changed, 141 insertions(+)

diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 026d64506f..e32ec01f56 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -33,6 +33,7 @@
    #define ICC_IAR1_EL1_SPURIOUS    0x3ff
  #define VGIC_MAX_SPI             1019
+#define VGIC_MIN_LPI             8192
    static int vgic_v3_bpr_min(void)
@@ -482,6 +483,137 @@ static void vreg_emulate_iar(struct cpu_user_regs *regs, const union hsr hsr)
      vgic_v3_read_iar(regs, hsr);
  +static int vgic_v3_find_active_lr(int intid, uint64_t *lr_val)
+    int i;
+    unsigned int used_lrs =  gic_get_num_lrs();
This is quite a departure from the existing code. KVM always allocate
LRs sequentially, and used_lrs represents the current upper bound.
IIUC, Xen uses a function gic_find_unused_lr to find an unused LR.

you seem to be looking at *all* the LRs. Is that safe?
IIUC Xen does not maintain a used_lrs, it does have an lr_mask, but that is static in gic.c

To do something like
+for_each_set_bit(i, lr_mask, nr_lrs)
+ {
+      u64 val = __gic_v3_get_lr(i);
+      u8 lr_prio = (val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
+     /* Not pending in the state? */
+     if ((val & ICH_LR_STATE) != ICH_LR_PENDING_BIT)
+         continue;

I need to do some jugglery to make lr_mask visible outside of xen/arch/arm/gic.c The easiest would be to add an extern function, harder way would be to add it in gic_hw_operations

- vgic_v3_highest_priority_lr iterates is interested in used LR's which sre in Pending state.
- emulating IAR is done with interrupts disabled
- iterating over all the LRs and finding which ones are in Pending.

Just to add I was answering for using num_lrs for used_lrs, above was for IAR flow.
This holds the same for EOIR flow as well.

The bigger point is unless I add some jugglery to access static value outside gic.c
this is the only solution.

Please suggest if there is some better way...
  Are you
guaranteed not to have any stale state?
I would request Stefano/Andre/Julien to comment here...

In any case, the change should be documented.

+    for ( i = 0; i < used_lrs; i++ )
+    {
+        uint64_t val = gicv3_ich_read_lr(i);
+        if ( (val & ICH_LR_VIRTUAL_ID_MASK) == intid &&
+            (val & ICH_LR_ACTIVE_BIT) )
+        {
+            *lr_val = val;
+            return i;
+        }
+    }
+    *lr_val = ICC_IAR1_EL1_SPURIOUS;
+    return -1;


Xen-devel mailing list



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