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

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

On 03/27/2018 04:00 PM, Marc Zyngier wrote:
> On 27/03/18 10:07, Manish Jaggi wrote:
> > This patch is ported to xen from linux commit
> > d70c7b31a60f2458f35c226131f2a01a7a98b6cf
> > KVM: arm64: vgic-v3: Add ICV_BPR1_EL1 handler
> >
> > Add a handler for reading/writing the guest's view of the ICC_BPR1_EL1
> > register, which is located in the ICH_VMCR_EL2.BPR1 field.
> >
> > Signed-off-by: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/arm64/vgic-v3-sr.c     | 70 +++++++++++++++++++++++++++++++++++++
> >   xen/include/asm-arm/arm64/sysregs.h |  1 +
> >   xen/include/asm-arm/gic_v3_defs.h   |  6 ++++
> >   3 files changed, 77 insertions(+)
> >
> > diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> > index 39ab1ed6ca..ed4254acf9 100644
> > --- a/xen/arch/arm/arm64/vgic-v3-sr.c
> > +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> > @@ -18,10 +18,76 @@
> >    */
> >   
> >   #include <asm/current.h>
> > +#include <asm/gic_v3_defs.h>
> >   #include <asm/regs.h>
> >   #include <asm/system.h>
> >   #include <asm/traps.h>
> >   
> > +#define vtr_to_nr_pre_bits(v)     ((((uint32_t)(v) >> 26) & 7) + 1)
> > +
> > +static int vgic_v3_bpr_min(void)
> > +{
> > +    /* See Pseudocode for VPriorityGroup */
> > +    return 8 - vtr_to_nr_pre_bits(READ_SYSREG32(ICH_VTR_EL2));
> > +}
> > +
> > +static unsigned int vgic_v3_get_bpr0(uint32_t vmcr)
> > +{
> > +    return (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
> > +}
> > +
> > +static unsigned int vgic_v3_get_bpr1(uint32_t vmcr)
> > +{
> > +    unsigned int bpr;
> > +
> > +    if ( vmcr & ICH_VMCR_CBPR_MASK )
> > +    {
> > +        bpr = vgic_v3_get_bpr0(vmcr);
> > +        if ( bpr < 7 )
> > +            bpr++;
> > +    }
> > +    else
> > +        bpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT;
> > +
> > +    return bpr;
> > +}
> > +
> > +static void vgic_v3_read_bpr1(struct cpu_user_regs *regs, int regidx)
> > +{
> > +    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
> > +
> > +    set_user_reg(regs, regidx, vgic_v3_get_bpr1(vmcr));
> > +}
> > +
> > +static void vgic_v3_write_bpr1(struct cpu_user_regs *regs, int regidx)
> > +{
> > +    register_t val = get_user_reg(regs, regidx);
> > +    uint8_t bpr_min = vgic_v3_bpr_min();
> > +    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
> > +
> > +    if ( vmcr & ICH_VMCR_CBPR_MASK )
> > +        return;
> > +
> > +    /* Enforce BPR limiting */
> > +    if ( val < bpr_min )
> > +        val = bpr_min;
> > +
> > +    val <<= ICH_VMCR_BPR1_SHIFT;
> > +    val &= ICH_VMCR_BPR1_MASK;
> > +    vmcr &= ~ICH_VMCR_BPR1_MASK;
> > +    vmcr |= val;
> > +
> > +    WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
> > +}
> > +
> > +static void vreg_emulate_bpr1(struct cpu_user_regs *regs, const union hsr hsr)
> > +{
> > +    if ( hsr.sysreg.read )
> > +        vgic_v3_read_bpr1(regs, hsr.sysreg.reg);
> > +    else
> > +        vgic_v3_write_bpr1(regs, hsr.sysreg.reg);
> > +}
> > +
> >   /*
> >    * returns true if the register is emulated.
> >    */
> > @@ -40,6 +106,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs)
> >   
> >       switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
> >       {
> > +    case HSR_SYSREG_ICC_BPR1_EL1:
> > +         vreg_emulate_bpr1(regs, hsr);
> > +         break;
> What is the rational for indirecting through a function and moving the
> reading of VMCR to the leaf functions? I appreciate that this doesn't
> change much, but since this is a port of existing code, it will make
> more complex the port of potential fixes.
I used xen template of handling sysreg traps
If you see the file xen/arch/arm/arm64/sysreg.c
a handle_XXX function is used throughout...

void do_sysreg(struct cpu_user_regs *regs,
               const union hsr hsr)
{
  ...
    /*
     * MDCR_EL2.TDRA
     *
     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
     */
    case HSR_SYSREG_MDRAR_EL1:
        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);

> This comment applies throughout the series.
>
> Thanks,
>
>         M.

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