| Message ID | 20220120173905.1047015-3-ricarkol@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: selftests: aarch64: some fixes for vgic_irq | expand |
On Thu, Jan 20, 2022 at 09:39:05AM -0800, Ricardo Koller wrote: > Fix the formatting of some comments and the wording of one of them (in > gicv3_access_reg). > > Signed-off-by: Ricardo Koller <ricarkol@google.com> > Reported-by: Reiji Watanabe <reijiw@google.com> > Cc: Andrew Jones <drjones@redhat.com> > --- > tools/testing/selftests/kvm/aarch64/vgic_irq.c | 12 ++++++++---- > tools/testing/selftests/kvm/lib/aarch64/gic_v3.c | 11 +++++++---- > tools/testing/selftests/kvm/lib/aarch64/vgic.c | 3 ++- > 3 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > index e6c7d7f8fbd1..258bb5150a07 100644 > --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c > +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > @@ -306,7 +306,8 @@ static void guest_restore_active(struct test_args *args, > uint32_t prio, intid, ap1r; > int i; > > - /* Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs > + /* > + * Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs > * in descending order, so intid+1 can preempt intid. > */ > for (i = 0, prio = (num - 1) * 8; i < num; i++, prio -= 8) { > @@ -315,7 +316,8 @@ static void guest_restore_active(struct test_args *args, > gic_set_priority(intid, prio); > } > > - /* In a real migration, KVM would restore all GIC state before running > + /* > + * In a real migration, KVM would restore all GIC state before running > * guest code. > */ > for (i = 0; i < num; i++) { > @@ -503,7 +505,8 @@ static void guest_code(struct test_args args) > test_injection_failure(&args, f); > } > > - /* Restore the active state of IRQs. This would happen when live > + /* > + * Restore the active state of IRQs. This would happen when live > * migrating IRQs in the middle of being handled. > */ > for_each_supported_activate_fn(&args, set_active_fns, f) > @@ -837,7 +840,8 @@ int main(int argc, char **argv) > } > } > > - /* If the user just specified nr_irqs and/or gic_version, then run all > + /* > + * If the user just specified nr_irqs and/or gic_version, then run all > * combinations. > */ > if (default_args) { > diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > index e4945fe66620..93fc35b88410 100644 > --- a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > +++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > @@ -19,7 +19,7 @@ struct gicv3_data { > unsigned int nr_spis; > }; > > -#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) > +#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) > #define DIST_BIT (1U << 31) > > enum gicv3_intid_range { > @@ -105,7 +105,8 @@ static void gicv3_set_eoi_split(bool split) > { > uint32_t val; > > - /* All other fields are read-only, so no need to read CTLR first. In > + /* > + * All other fields are read-only, so no need to read CTLR first. In > * fact, the kernel does the same. > */ > val = split ? (1U << 1) : 0; > @@ -160,8 +161,10 @@ static void gicv3_access_reg(uint32_t intid, uint64_t offset, > > GUEST_ASSERT(bits_per_field <= reg_bits); > GUEST_ASSERT(!write || *val < (1U << bits_per_field)); > - /* Some registers like IROUTER are 64 bit long. Those are currently not > - * supported by readl nor writel, so just asserting here until then. > + /* > + * This function does not support 64 bit accesses as those are > + * currently not supported by readl nor writel, so just asserting here > + * until then. Well, readl and writel will never support 64 bit accesses. We'd need to implement readq and writeq for that. If we're going to rewrite this comment please state it that way instead. > */ > GUEST_ASSERT(reg_bits == 32); > > diff --git a/tools/testing/selftests/kvm/lib/aarch64/vgic.c b/tools/testing/selftests/kvm/lib/aarch64/vgic.c > index b3a0fca0d780..79864b941617 100644 > --- a/tools/testing/selftests/kvm/lib/aarch64/vgic.c > +++ b/tools/testing/selftests/kvm/lib/aarch64/vgic.c > @@ -150,7 +150,8 @@ static void vgic_poke_irq(int gic_fd, uint32_t intid, > attr += SZ_64K; > } > > - /* All calls will succeed, even with invalid intid's, as long as the > + /* > + * All calls will succeed, even with invalid intid's, as long as the > * addr part of the attr is within 32 bits (checked above). An invalid > * intid will just make the read/writes point to above the intended > * register space (i.e., ICPENDR after ISPENDR). > -- > 2.35.0.rc0.227.g00780c9af4-goog > Thanks, drew
On Wed, Jan 26, 2022 at 04:22:03PM +0100, Andrew Jones wrote: > On Thu, Jan 20, 2022 at 09:39:05AM -0800, Ricardo Koller wrote: > > Fix the formatting of some comments and the wording of one of them (in > > gicv3_access_reg). > > > > Signed-off-by: Ricardo Koller <ricarkol@google.com> > > Reported-by: Reiji Watanabe <reijiw@google.com> > > Cc: Andrew Jones <drjones@redhat.com> > > --- > > tools/testing/selftests/kvm/aarch64/vgic_irq.c | 12 ++++++++---- > > tools/testing/selftests/kvm/lib/aarch64/gic_v3.c | 11 +++++++---- > > tools/testing/selftests/kvm/lib/aarch64/vgic.c | 3 ++- > > 3 files changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > > index e6c7d7f8fbd1..258bb5150a07 100644 > > --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c > > +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > > @@ -306,7 +306,8 @@ static void guest_restore_active(struct test_args *args, > > uint32_t prio, intid, ap1r; > > int i; > > > > - /* Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs > > + /* > > + * Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs > > * in descending order, so intid+1 can preempt intid. > > */ > > for (i = 0, prio = (num - 1) * 8; i < num; i++, prio -= 8) { > > @@ -315,7 +316,8 @@ static void guest_restore_active(struct test_args *args, > > gic_set_priority(intid, prio); > > } > > > > - /* In a real migration, KVM would restore all GIC state before running > > + /* > > + * In a real migration, KVM would restore all GIC state before running > > * guest code. > > */ > > for (i = 0; i < num; i++) { > > @@ -503,7 +505,8 @@ static void guest_code(struct test_args args) > > test_injection_failure(&args, f); > > } > > > > - /* Restore the active state of IRQs. This would happen when live > > + /* > > + * Restore the active state of IRQs. This would happen when live > > * migrating IRQs in the middle of being handled. > > */ > > for_each_supported_activate_fn(&args, set_active_fns, f) > > @@ -837,7 +840,8 @@ int main(int argc, char **argv) > > } > > } > > > > - /* If the user just specified nr_irqs and/or gic_version, then run all > > + /* > > + * If the user just specified nr_irqs and/or gic_version, then run all > > * combinations. > > */ > > if (default_args) { > > diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > > index e4945fe66620..93fc35b88410 100644 > > --- a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > > +++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c > > @@ -19,7 +19,7 @@ struct gicv3_data { > > unsigned int nr_spis; > > }; > > > > -#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) > > +#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) > > #define DIST_BIT (1U << 31) > > > > enum gicv3_intid_range { > > @@ -105,7 +105,8 @@ static void gicv3_set_eoi_split(bool split) > > { > > uint32_t val; > > > > - /* All other fields are read-only, so no need to read CTLR first. In > > + /* > > + * All other fields are read-only, so no need to read CTLR first. In > > * fact, the kernel does the same. > > */ > > val = split ? (1U << 1) : 0; > > @@ -160,8 +161,10 @@ static void gicv3_access_reg(uint32_t intid, uint64_t offset, > > > > GUEST_ASSERT(bits_per_field <= reg_bits); > > GUEST_ASSERT(!write || *val < (1U << bits_per_field)); > > - /* Some registers like IROUTER are 64 bit long. Those are currently not > > - * supported by readl nor writel, so just asserting here until then. > > + /* > > + * This function does not support 64 bit accesses as those are > > + * currently not supported by readl nor writel, so just asserting here > > + * until then. > > Well, readl and writel will never support 64 bit accesses. We'd need to > implement readq and writeq for that. If we're going to rewrite this > comment please state it that way instead. Good point, will make it clearer in v2. Thanks! > > > */ > > GUEST_ASSERT(reg_bits == 32); > > > > diff --git a/tools/testing/selftests/kvm/lib/aarch64/vgic.c b/tools/testing/selftests/kvm/lib/aarch64/vgic.c > > index b3a0fca0d780..79864b941617 100644 > > --- a/tools/testing/selftests/kvm/lib/aarch64/vgic.c > > +++ b/tools/testing/selftests/kvm/lib/aarch64/vgic.c > > @@ -150,7 +150,8 @@ static void vgic_poke_irq(int gic_fd, uint32_t intid, > > attr += SZ_64K; > > } > > > > - /* All calls will succeed, even with invalid intid's, as long as the > > + /* > > + * All calls will succeed, even with invalid intid's, as long as the > > * addr part of the attr is within 32 bits (checked above). An invalid > > * intid will just make the read/writes point to above the intended > > * register space (i.e., ICPENDR after ISPENDR). > > -- > > 2.35.0.rc0.227.g00780c9af4-goog > > > > Thanks, > drew >
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c index e6c7d7f8fbd1..258bb5150a07 100644 --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c @@ -306,7 +306,8 @@ static void guest_restore_active(struct test_args *args, uint32_t prio, intid, ap1r; int i; - /* Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs + /* + * Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs * in descending order, so intid+1 can preempt intid. */ for (i = 0, prio = (num - 1) * 8; i < num; i++, prio -= 8) { @@ -315,7 +316,8 @@ static void guest_restore_active(struct test_args *args, gic_set_priority(intid, prio); } - /* In a real migration, KVM would restore all GIC state before running + /* + * In a real migration, KVM would restore all GIC state before running * guest code. */ for (i = 0; i < num; i++) { @@ -503,7 +505,8 @@ static void guest_code(struct test_args args) test_injection_failure(&args, f); } - /* Restore the active state of IRQs. This would happen when live + /* + * Restore the active state of IRQs. This would happen when live * migrating IRQs in the middle of being handled. */ for_each_supported_activate_fn(&args, set_active_fns, f) @@ -837,7 +840,8 @@ int main(int argc, char **argv) } } - /* If the user just specified nr_irqs and/or gic_version, then run all + /* + * If the user just specified nr_irqs and/or gic_version, then run all * combinations. */ if (default_args) { diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c index e4945fe66620..93fc35b88410 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c +++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c @@ -19,7 +19,7 @@ struct gicv3_data { unsigned int nr_spis; }; -#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) +#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K) #define DIST_BIT (1U << 31) enum gicv3_intid_range { @@ -105,7 +105,8 @@ static void gicv3_set_eoi_split(bool split) { uint32_t val; - /* All other fields are read-only, so no need to read CTLR first. In + /* + * All other fields are read-only, so no need to read CTLR first. In * fact, the kernel does the same. */ val = split ? (1U << 1) : 0; @@ -160,8 +161,10 @@ static void gicv3_access_reg(uint32_t intid, uint64_t offset, GUEST_ASSERT(bits_per_field <= reg_bits); GUEST_ASSERT(!write || *val < (1U << bits_per_field)); - /* Some registers like IROUTER are 64 bit long. Those are currently not - * supported by readl nor writel, so just asserting here until then. + /* + * This function does not support 64 bit accesses as those are + * currently not supported by readl nor writel, so just asserting here + * until then. */ GUEST_ASSERT(reg_bits == 32); diff --git a/tools/testing/selftests/kvm/lib/aarch64/vgic.c b/tools/testing/selftests/kvm/lib/aarch64/vgic.c index b3a0fca0d780..79864b941617 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/vgic.c +++ b/tools/testing/selftests/kvm/lib/aarch64/vgic.c @@ -150,7 +150,8 @@ static void vgic_poke_irq(int gic_fd, uint32_t intid, attr += SZ_64K; } - /* All calls will succeed, even with invalid intid's, as long as the + /* + * All calls will succeed, even with invalid intid's, as long as the * addr part of the attr is within 32 bits (checked above). An invalid * intid will just make the read/writes point to above the intended * register space (i.e., ICPENDR after ISPENDR).
Fix the formatting of some comments and the wording of one of them (in gicv3_access_reg). Signed-off-by: Ricardo Koller <ricarkol@google.com> Reported-by: Reiji Watanabe <reijiw@google.com> Cc: Andrew Jones <drjones@redhat.com> --- tools/testing/selftests/kvm/aarch64/vgic_irq.c | 12 ++++++++---- tools/testing/selftests/kvm/lib/aarch64/gic_v3.c | 11 +++++++---- tools/testing/selftests/kvm/lib/aarch64/vgic.c | 3 ++- 3 files changed, 17 insertions(+), 9 deletions(-)