On Fri, 21 Feb 2020 at 07:46, Sai Pavan Boddu <sai.pavan.bo...@xilinx.com> wrote: > > Priority bits implemented in arm-gic can be 8 to 4, un-implemented bits > are read as zeros(RAZ). > > Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com> > --- > hw/intc/arm_gic.c | 26 ++++++++++++++++++++++++-- > hw/intc/arm_gic_common.c | 1 + > include/hw/intc/arm_gic_common.h | 1 + > 3 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 1d7da7b..dec8767 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -641,6 +641,23 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, > MemTxAttrs attrs) > return ret; > } > > +static uint32_t gic_fullprio_mask(GICState *s, int cpu) > +{ > + /* > + * Return a mask word which clears the unimplemented priority > + * bits from a priority value for an interrupt. (Not to be > + * confused with the group priority, whose mask depends on BPR.) > + */ > + int unimpBits; > + > + if (gic_is_vcpu(cpu)) { > + unimpBits = GIC_VIRT_MAX_GROUP_PRIO_BITS; > + } else { > + unimpBits = 8 - s->n_prio_bits;
This isn't right; GIC_VIRT_MAX_GROUP_PRIO_BITS should be handled the same way as s->n_prio_bits. The expression I suggested in my comment on your v1 should work: if (gic_is_vcpu(cpu)) { pribits = GIC_VIRT_MAX_GROUP_PRIO_BITS; } else { pribits = s->n_prio_bits; } return ~0U << (8 - s->n_prio_bits); > + } > + return ~0U << unimpBits; > +} > + > void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val, > MemTxAttrs attrs) > { You seem to have lost the part of the patch which applies the mask in gic_dist_set_priority(). If the GIC only has 5 bits of priority we should not allow the guest to set more than that. > @@ -669,7 +686,7 @@ static uint32_t gic_dist_get_priority(GICState *s, int > cpu, int irq, > } > prio = (prio << 1) & 0xff; /* Non-secure view */ > } > - return prio; > + return prio & gic_fullprio_mask(s, cpu); > } > > static void gic_set_priority_mask(GICState *s, int cpu, uint8_t pmask, > @@ -684,7 +701,7 @@ static void gic_set_priority_mask(GICState *s, int cpu, > uint8_t pmask, > return; > } > } > - s->priority_mask[cpu] = pmask; > + s->priority_mask[cpu] = pmask & gic_fullprio_mask(s, cpu); > } > > static uint32_t gic_get_priority_mask(GICState *s, int cpu, MemTxAttrs attrs) > @@ -2055,6 +2072,11 @@ static void arm_gic_realize(DeviceState *dev, Error > **errp) > return; > } > > + if (s->n_prio_bits > 8) { > + error_setg(errp, "num-priority-bits cannot be greater than 8"); > + return; > + } You need to also check that the value is at least as large as the lowest permitted value, as I suggested in my v1 comment. thanks -- PMM