| Message ID | 1521470177-14760-3-git-send-email-zhaoshenglong@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | two fixes for KVM GICv3 dist get/put functions | expand |
On 19 March 2018 at 14:36, Shannon Zhao <zhaoshenglong@huawei.com> wrote: > It should skip to getting/putting the registers banked by GICR so that > it could get/put the correct ones. > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> Can you explain what the consequences of the current code are (ie is there a bug here that we are fixing?), and why the changes are correct, please? > --- > hw/intc/arm_gicv3_kvm.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c > index 7000716..e967e4f 100644 > --- a/hw/intc/arm_gicv3_kvm.c > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -136,6 +136,8 @@ static void kvm_dist_get_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) > int irq; > > field = (uint32_t *)bmp; > + /* The first 8 GICD_IPRIORITYR should skip. */ It would be helpful to explain why we can skip the first 8 registers; similarly below. (I suspect the answer here is "for the KVM GICv3, affinity routing is always enabled, and the first 8 GICD_IPRIORITYR<n> registers are always RAZ/WI, so we don't need to sync them", for instance.) > + offset += (8 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 8) { > kvm_gicd_access(s, offset, ®, false); > *field = reg; > @@ -150,6 +152,8 @@ static void kvm_dist_put_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) > int irq; > > field = (uint32_t *)bmp; > + /* The first 8 GICD_IPRIORITYR should skip. */ > + offset += (8 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 8) { > reg = *field; > kvm_gicd_access(s, offset, ®, true); > @@ -164,6 +168,8 @@ static void kvm_dist_get_edge_trigger(GICv3State *s, uint32_t offset, > uint32_t reg; > int irq; > > + /* The first 2 GICD_ICFGR should skip. */ > + offset += (2 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 2) { > kvm_gicd_access(s, offset, ®, false); > reg = half_unshuffle32(reg >> 1); > @@ -181,6 +187,8 @@ static void kvm_dist_put_edge_trigger(GICv3State *s, uint32_t offset, > uint32_t reg; > int irq; > > + /* The first 2 GICD_ICFGR should skip. */ > + offset += (2 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 2) { > reg = *gic_bmp_ptr32(bmp, irq); > if (irq % 32 != 0) { > @@ -222,6 +230,8 @@ static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp) > uint32_t reg; > int irq; > > + /* The first 1 GICD_XXXX should skip. */ > + offset += (1 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 1) { > kvm_gicd_access(s, offset, ®, false); > *gic_bmp_ptr32(bmp, irq) = reg; > @@ -236,6 +246,10 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, > int irq; > > clroffset_index = clroffset; > + /* The first 1 GICD_XXXX should skip. */ This looks like placeholder text that hasn't been filled in ? > + offset += (1 * sizeof(uint32_t)); > + if (clroffset != 0) > + clroffset_index += (1 * sizeof(uint32_t)); > for_each_dist_irq_reg(irq, s->num_irq, 1) { > /* If this bitmap is a set/clear register pair, first write to the > * clear-reg to clear all bits before using the set-reg to write > -- > 2.0.4 thanks -- PMM
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 7000716..e967e4f 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -136,6 +136,8 @@ static void kvm_dist_get_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) int irq; field = (uint32_t *)bmp; + /* The first 8 GICD_IPRIORITYR should skip. */ + offset += (8 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 8) { kvm_gicd_access(s, offset, ®, false); *field = reg; @@ -150,6 +152,8 @@ static void kvm_dist_put_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) int irq; field = (uint32_t *)bmp; + /* The first 8 GICD_IPRIORITYR should skip. */ + offset += (8 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 8) { reg = *field; kvm_gicd_access(s, offset, ®, true); @@ -164,6 +168,8 @@ static void kvm_dist_get_edge_trigger(GICv3State *s, uint32_t offset, uint32_t reg; int irq; + /* The first 2 GICD_ICFGR should skip. */ + offset += (2 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 2) { kvm_gicd_access(s, offset, ®, false); reg = half_unshuffle32(reg >> 1); @@ -181,6 +187,8 @@ static void kvm_dist_put_edge_trigger(GICv3State *s, uint32_t offset, uint32_t reg; int irq; + /* The first 2 GICD_ICFGR should skip. */ + offset += (2 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 2) { reg = *gic_bmp_ptr32(bmp, irq); if (irq % 32 != 0) { @@ -222,6 +230,8 @@ static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp) uint32_t reg; int irq; + /* The first 1 GICD_XXXX should skip. */ + offset += (1 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 1) { kvm_gicd_access(s, offset, ®, false); *gic_bmp_ptr32(bmp, irq) = reg; @@ -236,6 +246,10 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, int irq; clroffset_index = clroffset; + /* The first 1 GICD_XXXX should skip. */ + offset += (1 * sizeof(uint32_t)); + if (clroffset != 0) + clroffset_index += (1 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 1) { /* If this bitmap is a set/clear register pair, first write to the * clear-reg to clear all bits before using the set-reg to write
It should skip to getting/putting the registers banked by GICR so that it could get/put the correct ones. Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> --- hw/intc/arm_gicv3_kvm.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)