[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 4/5] xen/arm: add SGI handling for GICv3
Hi Vijay, On 07/03/2014 09:37 AM, 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> > --- > xen/arch/arm/traps.c | 30 +++++++++++++++++ > xen/arch/arm/vgic-v3.c | 65 > +++++++++++++++++++++++++++++++++---- > xen/include/asm-arm/gic_v3_defs.h | 7 ++++ > xen/include/asm-arm/sysregs.h | 3 ++ > xen/include/asm-arm/vgic.h | 1 + > 5 files changed, 100 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 686d8b7..adc316b 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 > @@ -496,6 +497,18 @@ static void inject_dabt_exception(struct cpu_user_regs > *regs, > #endif > } > > +static void inject_undef_exception(struct cpu_user_regs *regs, > + register_t addr, > + int instr_len) Odd, you give in parameter addr, but never use it... > +{ > + if ( is_32bit_domain(current->domain) ) > + inject_undef32_exception(regs); > +#ifdef CONFIG_ARM_64 > + else > + inject_undef64_exception(regs, instr_len); > +#endif > +} > + > struct reg_ctxt { > /* Guest-side state */ > uint32_t sctlr_el1; > @@ -1591,6 +1604,7 @@ static void do_sysreg(struct cpu_user_regs *regs, > union hsr hsr) > { > register_t *x = select_user_reg(regs, hsr.sysreg.reg); > + register_t addr; > > switch ( hsr.bits & HSR_SYSREG_REGS_MASK ) > { > @@ -1641,6 +1655,22 @@ static void do_sysreg(struct cpu_user_regs *regs, > domain_crash_synchronous(); > } > break; > + case HSR_SYSREG_ICC_SGI1R_EL1: > + if ( !vgic_emulate(regs, hsr) ) > + { > + addr = READ_SYSREG64(FAR_EL2); > + dprintk(XENLOG_WARNING, > + "failed emulation of sysreg ICC_SGI1R_EL1 access\n"); > + inject_undef_exception(regs, addr, hsr.len); AFAIK, sysreg only exist for 64 bits guest. At least the precondition to call this function is the guest is 64 bits (see the caller). So you can use directly inject_undef64_exception. > + } > + 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"); > + addr = READ_SYSREG64(FAR_EL2); > + inject_undef_exception(regs, addr, hsr.len); Same remark here. > default: > bad_sysreg: > { > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 63948ed..7beed17 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -35,7 +35,7 @@ > #include <asm/vgic.h> > > static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, > - uint32_t gicr_reg) > + uint32_t gicr_reg) Spurious change. > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -141,7 +141,7 @@ read_as_zero: > } > > static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info, > - uint32_t gicr_reg) > + uint32_t gicr_reg) Spurious change. > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -214,7 +214,7 @@ write_ignore: > } > > static int __vgic_v3_distr_common_mmio_read(struct vcpu *v, mmio_info_t > *info, > - uint32_t reg) > + uint32_t reg) > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -313,7 +313,7 @@ read_as_zero: > } > > static int __vgic_v3_distr_common_mmio_write(struct vcpu *v, mmio_info_t > *info, > - uint32_t reg) > + uint32_t reg) Spurious change. > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -423,7 +423,7 @@ write_ignore: > } > > static int vgic_v3_rdistr_sgi_mmio_read(struct vcpu *v, mmio_info_t *info, > - uint32_t gicr_reg) > + uint32_t gicr_reg) Spurious change. > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -484,7 +484,7 @@ read_as_zero: > } > > static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info, > - uint32_t gicr_reg) > + uint32_t gicr_reg) Spurious change. > { > struct hsr_dabt dabt = info->dabt; > struct cpu_user_regs *regs = guest_cpu_user_regs(); > @@ -835,6 +835,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(); > + } > + > + return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask); > +} > + > +int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr) Please see my comment on "[PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver". > +{ > + 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 vgic_send_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, > @@ -866,6 +917,7 @@ static int vgicv3_domain_init(struct domain *d) > { > int i; > > + /* We rely on gicv init to get dbase and size */ Why do you add the comment in this patch. Shouldn't be part of the vgic v3 emulation? Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |