[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the reg
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [PATCH 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR |
Date: |
Mon, 19 Mar 2018 16:30:37 +0000 |
On 19 March 2018 at 14:36, Shannon Zhao <address@hidden> 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 <address@hidden>
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