Re: [Xen-devel] [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler

[Julien Grall <julien.grall@xxxxxxx>]

On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> This patch is ported to xen from linux commit:
> f8b630bc542e0368886ae193d3519c832b270359
>
> Add a handler for reading/writing the guest's view of the
> ICC_IGRPEN1_EL1
The wrapping looks wrong.

> register, which is located in the ICH_VMCR_EL2.VENG1 field.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
>
> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> index 364785d3ac..114d5107a9 100644
> --- a/xen/arch/arm/arm64/vgic-v3-sr.c
> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> @@ -86,6 +86,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int regidx, 
> const union hsr hsr)
>           __vgic_v3_write_bpr1(regs, regidx);
>   }
>   
> +static void  __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, int regidx)
Please remove the __ for all the functions.

> +{
> +    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
Newline.

> +    set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG1_MASK));
> +}
> +
> +static void  __vgic_v3_write_igrpen1(struct cpu_user_regs *regs, int regidx)
> +{
> +    register_t val = get_user_reg(regs, regidx);
> +    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
> +
> +    if ( val & 1 )
> +        vmcr |= ICH_VMCR_ENG1_MASK;
> +    else
> +        vmcr &= ~ICH_VMCR_ENG1_MASK;
> +
> +    WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
So basically, you ported commit without even looking if there were 
change on top. For instance the latest code has a specific has an helper 
to update vmcr. Can you please make sure that all the code is ported?
> +}
> +
> +void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
> +                    const union hsr hsr)
> +{
> +    if ( hsr.sysreg.read )
> +        __vgic_v3_read_igrpen1(regs, regidx);
> +    else
> +        __vgic_v3_write_igrpen1(regs, regidx);
> +}
> +
>   bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr 
> hsr)
>   {
>       bool ret = true;
> @@ -105,6 +133,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs 
> *regs, const union hsr hsr
>            handle_bpr1(regs, regidx, hsr);
>            break;
>   
> +    case HSR_SYSREG_ICC_IGRPEN1_EL1:
> +        handle_igrpen1(regs, regidx, hsr);
> +        break;
> +
>       default:
>           ret = false;
>           break;
> diff --git a/xen/include/asm-arm/arm64/sysregs.h 
> b/xen/include/asm-arm/arm64/sysregs.h
> index 025a27b0b4..731cabc74a 100644
> --- a/xen/include/asm-arm/arm64/sysregs.h
> +++ b/xen/include/asm-arm/arm64/sysregs.h
> @@ -90,6 +90,7 @@
>   #define HSR_SYSREG_ICC_SGI0R_EL1  HSR_SYSREG(3,2,c12,c11,7)
>   #define HSR_SYSREG_ICC_SRE_EL1    HSR_SYSREG(3,0,c12,c12,5)
>   #define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
> +#define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,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/gic_v3_defs.h 
> b/xen/include/asm-arm/gic_v3_defs.h
> index 68a34cc353..ff8bda37d1 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -163,6 +163,8 @@
>   #define ICH_VMCR_BPR0_MASK           (7 << ICH_VMCR_BPR0_SHIFT)
>   #define ICH_VMCR_BPR1_SHIFT          18
>   #define ICH_VMCR_BPR1_MASK           (7 << ICH_VMCR_BPR1_SHIFT)
> +#define ICH_VMCR_ENG1_SHIFT          1
> +#define ICH_VMCR_ENG1_MASK           (1 << ICH_VMCR_ENG1_SHIFT)
>   
>   #define GICH_LR_VIRTUAL_MASK         0xffff
>   #define GICH_LR_VIRTUAL_SHIFT        0
Cheers,
--
Julien Grall

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