On 2017/5/16 9:19, wanghaibin wrote:

> Boot a virtual machine with the emulated GICv2 on the GICv3 hardware.
> Migrate the virtual machine will be successful, but the virtual machine will
> hang at the destination.
> 
> The GICC_CTLR and ICC_CTLR_EL1 have the different layout. Currently, the 
> set/get
> the VMCR interface just take vmcr ctlr field as the ICC_CTLR_EL1 layout.
> Should we consider the GICC_CTLR layout to avoid this problem?


Ping ..

BTW: I test this case on hisilicon D03 board, and it's like the commit 
(5fb247d79c04240dce86c842976cde1edde7f7ed)
introduced this problem which doesn't consider the compatible of GICC_CTLR 
layoyut.

+ the patch author Vijaya

Thanks.

> 
> Signed-off-by: wanghaibin <wanghaibin.w...@huawei.com>
> ---
>  include/linux/irqchip/arm-gic-v3.h |  2 ++
>  virt/kvm/arm/vgic/vgic-v3.c        | 48 
> ++++++++++++++++++++++++--------------
>  2 files changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/irqchip/arm-gic-v3.h 
> b/include/linux/irqchip/arm-gic-v3.h
> index 97cbca1..e40c711 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -403,6 +403,8 @@
>  #define ICH_HCR_EN                   (1 << 0)
>  #define ICH_HCR_UIE                  (1 << 1)
>  
> +#define ICH_VMCR_CTLR_SHIFT          0
> +#define ICH_VMCR_CTLR_MASK           (0x21f << ICH_VMCR_CTLR_SHIFT)
>  #define ICH_VMCR_CBPR_SHIFT          4
>  #define ICH_VMCR_CBPR_MASK           (1 << ICH_VMCR_CBPR_SHIFT)
>  #define ICH_VMCR_EOIM_SHIFT          9
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index be0f4c3..66defae 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -174,19 +174,25 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr)
>  void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp)
>  {
>       u32 vmcr;
> +     u32 model = vcpu->kvm->arch.vgic.vgic_model;
> +
> +     if (model == KVM_DEV_TYPE_ARM_VGIC_V2)
> +             vmcr  = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & 
> ICH_VMCR_CTLR_MASK;
> +     else {
> +             /*
> +              * Ignore the FIQen bit, because GIC emulation always implies
> +              * SRE=1 which means the vFIQEn bit is also RES1.
> +              */
> +             vmcr = ((vmcrp->ctlr >> ICC_CTLR_EL1_EOImode_SHIFT) <<
> +                      ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK;
> +             vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & 
> ICH_VMCR_CBPR_MASK;
> +             vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & 
> ICH_VMCR_ENG0_MASK;
> +             vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & 
> ICH_VMCR_ENG1_MASK;
> +     }
>  
> -     /*
> -      * Ignore the FIQen bit, because GIC emulation always implies
> -      * SRE=1 which means the vFIQEn bit is also RES1.
> -      */
> -     vmcr = ((vmcrp->ctlr >> ICC_CTLR_EL1_EOImode_SHIFT) <<
> -              ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK;
> -     vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & ICH_VMCR_CBPR_MASK;
>       vmcr |= (vmcrp->abpr << ICH_VMCR_BPR1_SHIFT) & ICH_VMCR_BPR1_MASK;
>       vmcr |= (vmcrp->bpr << ICH_VMCR_BPR0_SHIFT) & ICH_VMCR_BPR0_MASK;
>       vmcr |= (vmcrp->pmr << ICH_VMCR_PMR_SHIFT) & ICH_VMCR_PMR_MASK;
> -     vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & ICH_VMCR_ENG0_MASK;
> -     vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & ICH_VMCR_ENG1_MASK;
>  
>       vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = vmcr;
>  }
> @@ -194,19 +200,25 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct 
> vgic_vmcr *vmcrp)
>  void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp)
>  {
>       u32 vmcr = vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr;
> +     u32 model = vcpu->kvm->arch.vgic.vgic_model;
> +
> +     if (model == KVM_DEV_TYPE_ARM_VGIC_V2)
> +             vmcrp->ctlr = (vmcr & ICH_VMCR_CTLR_MASK) >> 
> ICH_VMCR_CTLR_SHIFT;
> +     else {
> +             /*
> +              * Ignore the FIQen bit, because GIC emulation always implies
> +              * SRE=1 which means the vFIQEn bit is also RES1.
> +              */
> +             vmcrp->ctlr = ((vmcr >> ICH_VMCR_EOIM_SHIFT) <<
> +                             ICC_CTLR_EL1_EOImode_SHIFT) & 
> ICC_CTLR_EL1_EOImode_MASK;
> +             vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> 
> ICH_VMCR_CBPR_SHIFT;
> +             vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> 
> ICH_VMCR_ENG0_SHIFT;
> +             vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> 
> ICH_VMCR_ENG1_SHIFT;
> +     }
>  
> -     /*
> -      * Ignore the FIQen bit, because GIC emulation always implies
> -      * SRE=1 which means the vFIQEn bit is also RES1.
> -      */
> -     vmcrp->ctlr = ((vmcr >> ICH_VMCR_EOIM_SHIFT) <<
> -                     ICC_CTLR_EL1_EOImode_SHIFT) & ICC_CTLR_EL1_EOImode_MASK;
> -     vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT;
>       vmcrp->abpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT;
>       vmcrp->bpr  = (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
>       vmcrp->pmr  = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT;
> -     vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> ICH_VMCR_ENG0_SHIFT;
> -     vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> ICH_VMCR_ENG1_SHIFT;
>  }
>  
>  #define INITIAL_PENDBASER_VALUE                                              
>   \



_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to