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

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

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.


>   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;
> > +}
> Thanks,
>
>         M.

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