[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



Hi,

On 04/06/2018 09:37 AM, Manish Jaggi wrote:


On 04/05/2018 03:10 PM, Julien Grall wrote:
Hi,

On 02/04/18 12:17, Manish Jaggi wrote:


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
b6f49035b4bf6e2709f2a5fed3107f5438c1fd02
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.

xen/arch/arm/gic.c:
gic_raise_guest_irq
    gic_find_unused_lr
Here,
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.

Stefano/Andre/Julien
Please suggest if there is some better way...

lr_mask is already exported. So I am not sure what you need here.

However, despite the fact the variable is living in gic.c it is only used by the old vGIC. Newer vGIC is based on KVM, so used_lrs would be fine to use.

So I should send the next version based on old vGIC or new vGIC ?

The new vGIC has been merged for Xen 4.11 and will cohabite with the old vGIC for a couple of release for hardening. So you want to add support for both.

Note that the new vGIC does not yet have support for GICv3, but it should not be a big issue for keeping the code around. I think it can easily be reviewed.

Cheers,

--
Julien Grall

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

 


Rackspace

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