Re: [Xen-devel] [PATCH v7 4/5] xen/arm: add SGI handling for GICv3

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 11 Jul 2014, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> 
> In ARMv8, write to ICC_SGI1R_EL1 register raises trap to EL2.
> Handle the trap and inject SGI to vcpu.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> ---
> v7: - Introduced callback for sysreg emulation
>     - Removed unused parameter in inject_undef_exception()
>     - Use inject_undef64_exception for reporting sysreg
>       handling failure
> 
> v6: - Removed forward declaration of vgic_to_sgi() in vgic-v3.c
>     - Used vgic callback for SGI handling
>     - Alignment changes
> ---
>  xen/arch/arm/traps.c              |   15 +++++++++++
>  xen/arch/arm/vgic-v3.c            |   52 
> +++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic.c               |    7 +++++
>  xen/include/asm-arm/gic_v3_defs.h |    7 +++++
>  xen/include/asm-arm/sysregs.h     |    3 +++
>  xen/include/asm-arm/vgic.h        |    3 +++
>  6 files changed, 87 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 686d8b7..775bef1 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -41,6 +41,7 @@
>  #include "decode.h"
>  #include "vtimer.h"
>  #include <asm/gic.h>
> +#include <asm/vgic.h>
>  
>  /* The base of the stack must always be double-word aligned, which means
>   * that both the kernel half of struct cpu_user_regs (which is pushed in
> @@ -1641,6 +1642,20 @@ static void do_sysreg(struct cpu_user_regs *regs,
>              domain_crash_synchronous();
>          }
>          break;
> +    case HSR_SYSREG_ICC_SGI1R_EL1:
> +        if ( !vgic_emulate(regs, hsr) )
> +        {
> +            dprintk(XENLOG_WARNING,
> +                    "failed emulation of sysreg ICC_SGI1R_EL1 access\n");
> +            inject_undef64_exception(regs, hsr.len);
> +        }
> +        break;
> +    case HSR_SYSREG_ICC_SGI0R_EL1:
> +    case HSR_SYSREG_ICC_ASGI1R_EL1:
> +        /* TBD: Implement to support secure grp0/1 SGI forwarding */
> +        dprintk(XENLOG_WARNING,
> +                "Emulation of sysreg ICC_SGI0R_EL1/ASGI1R_EL1 not 
> supported\n");
> +        inject_undef64_exception(regs, hsr.len);
>      default:
>   bad_sysreg:
>          {
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 2bf0e7c..feee486 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -834,6 +834,57 @@ write_ignore_64:
>      return 1;
>  }
>  
> +static int vgicv3_to_sgi(struct vcpu *v, register_t sgir)
> +{
> +    int virq;
> +    int irqmode;
> +    enum gic_sgi_mode sgi_mode;
> +    unsigned long vcpu_mask = 0;
> +
> +    irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK;
> +    virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK;
> +    vcpu_mask = sgir & ICH_SGI_TARGETLIST_MASK;
> +
> +    /* Map GIC sgi value to enum value */
> +    switch ( irqmode )
> +    {
> +    case ICH_SGI_TARGET_LIST:
> +        sgi_mode = SGI_TARGET_LIST;
> +        break;
> +    case ICH_SGI_TARGET_OTHERS:
> +        sgi_mode = SGI_TARGET_OTHERS;
> +        break;
> +    default:
> +        BUG();

You haven't addressed my previous comments (or replied to them if you
disagree):

http://marc.info/?l=xen-devel&m=140259344723979
http://marc.info/?l=xen-devel&m=140259388524231



> +    }
> +
> +    return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
> +}
> +
> +static int vgicv3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
> +{
> +    struct vcpu *v = current;
> +    struct hsr_sysreg sysreg = hsr.sysreg;
> +    register_t *r = select_user_reg(regs, sysreg.reg);
> +
> +    ASSERT (hsr.ec == HSR_EC_SYSREG);
> +
> +    switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
> +    {
> +    case HSR_SYSREG_ICC_SGI1R_EL1:
> +        /* WO */
> +        if ( !sysreg.read )
> +            return vgicv3_to_sgi(v, *r);
> +        else
> +        {
> +            gdprintk(XENLOG_WARNING, "Reading SGI1R_EL1 - WO register\n");
> +            return 0;
> +        }
> +    default:
> +        return 0;
> +    }
> +}
> +
>  static const struct mmio_handler_ops vgic_rdistr_mmio_handler = {
>      .read_handler  = vgic_v3_rdistr_mmio_read,
>      .write_handler = vgic_v3_rdistr_mmio_write,
> @@ -884,6 +935,7 @@ static int vgicv3_domain_init(struct domain *d)
>  static const struct vgic_ops v3_ops = {
>      .vcpu_init   = vgicv3_vcpu_init,
>      .domain_init = vgicv3_domain_init,
> +    .emulate_sysreg  = vgicv3_emulate_sysreg,
>  };
>  
>  int vgic_v3_init(struct domain *d)
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 3647497..e15d509 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -343,6 +343,13 @@ out:
>          smp_send_event_check_mask(cpumask_of(v->processor));
>  }
>  
> +int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr)
> +{
> +    struct vcpu *v = current;
> +
> +    return v->domain->arch.vgic.handler->emulate_sysreg(regs, hsr);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/gic_v3_defs.h 
> b/xen/include/asm-arm/gic_v3_defs.h
> index 1c32e0c..b94ba20 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -147,6 +147,13 @@
>  #define GICH_VMCR_PRIORITY_MASK      0xff
>  #define GICH_VMCR_PRIORITY_SHIFT     24
>  
> +#define ICH_SGI_IRQMODE_SHIFT        40
> +#define ICH_SGI_IRQMODE_MASK         0x1
> +#define ICH_SGI_TARGET_OTHERS        1
> +#define ICH_SGI_TARGET_LIST          0
> +#define ICH_SGI_IRQ_SHIFT            24
> +#define ICH_SGI_IRQ_MASK             0xf
> +#define ICH_SGI_TARGETLIST_MASK      0xffff
>  #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
>  
>  /*
> diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/sysregs.h
> index b00871c..48ea498 100644
> --- a/xen/include/asm-arm/sysregs.h
> +++ b/xen/include/asm-arm/sysregs.h
> @@ -78,6 +78,9 @@
>  #define HSR_SYSREG_PMINTENCLR_EL1 HSR_SYSREG(3,0,c9,c14,2)
>  #define HSR_SYSREG_MAIR_EL1       HSR_SYSREG(3,0,c10,c2,0)
>  #define HSR_SYSREG_AMAIR_EL1      HSR_SYSREG(3,0,c10,c3,0)
> +#define HSR_SYSREG_ICC_SGI1R_EL1  HSR_SYSREG(3,0,c12,c11,5)
> +#define HSR_SYSREG_ICC_ASGI1R_EL1 HSR_SYSREG(3,1,c12,c11,6)
> +#define HSR_SYSREG_ICC_SGI0R_EL1  HSR_SYSREG(3,2,c12,c11,7)
>  #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
>  
>  #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index f580b78..63ecfa1 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -98,6 +98,8 @@ struct vgic_ops {
>      int (*vcpu_init)(struct vcpu *v);
>      /* Domain specific initialization of vGIC */
>      int (*domain_init)(struct domain *d);
> +    /* vGIC sysreg emulation */
> +    int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
>  };
>  
>  /* Number of ranks of interrupt registers for a domain */
> @@ -165,6 +167,7 @@ extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned 
> int irq);
>  extern void vgic_clear_pending_irqs(struct vcpu *v);
>  extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
>  extern struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n, 
> int s);
> +extern int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr);
>  extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n);
>  extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n);
>  extern void register_vgic_ops(struct domain *d, const struct vgic_ops *ops);
> -- 
> 1.7.9.5
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel