From: Alexandru Elisei <alexandru.elisei@arm.com> To: Andre Przywara <andre.przywara@arm.com>, Andrew Jones <drjones@redhat.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: Marc Zyngier <maz@kernel.org>, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org Subject: Re: [kvm-unit-tests PATCH 04/17] arm: gic: Support no IRQs test case Date: Tue, 12 Nov 2019 13:26:17 +0000 Message-ID: <db89b983-425c-8b45-3f26-1a33b9817836@arm.com> (raw) In-Reply-To: <20191108144240.204202-5-andre.przywara@arm.com> Hi, On 11/8/19 2:42 PM, Andre Przywara wrote: > For some tests it would be important to check that an IRQ was *not* > triggered, for instance to test certain masking operations. > > Extend the check_added() function to recognise an empty cpumask to > detect this situation. The timeout duration is reduced, and the "no IRQs Why is the timeout duration reduced? > triggered" case is actually reported as a success in this case. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arm/gic.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arm/gic.c b/arm/gic.c > index a114009..eca9188 100644 > --- a/arm/gic.c > +++ b/arm/gic.c > @@ -66,9 +66,10 @@ static void check_acked(const char *testname, cpumask_t *mask) > int missing = 0, extra = 0, unexpected = 0; > int nr_pass, cpu, i; > bool bad = false; > + bool noirqs = cpumask_empty(mask); > > /* Wait up to 5s for all interrupts to be delivered */ This comment needs updating. > - for (i = 0; i < 50; ++i) { > + for (i = 0; i < (noirqs ? 15 : 50); ++i) { > mdelay(100); > nr_pass = 0; > for_each_present_cpu(cpu) { > @@ -88,7 +89,7 @@ static void check_acked(const char *testname, cpumask_t *mask) > bad = true; > } > } > - if (nr_pass == nr_cpus) { > + if (!noirqs && nr_pass == nr_cpus) { This condition is pretty hard to read - what you are doing here is making sure that when check_acked tests that no irqs have been received, you do the entire for loop and wait the entire timeout duration. Did I get that right? How about this (compile tested only): + if (noirqs) + /* Wait for the entire timeout duration. */ + continue; + if (nr_pass == nr_cpus) { report("%s", !bad, testname); if (i) > report("%s", !bad, testname); > if (i) > report_info("took more than %d ms", i * 100); > @@ -96,6 +97,11 @@ static void check_acked(const char *testname, cpumask_t *mask) > } > } > > + if (noirqs && nr_pass == nr_cpus) { > + report("%s", !bad, testname); bad is true only when bad_sender[cpu] != -1 or bad_irq[cpu] != -1, which only get set in the irq or ipi handlesr, meaning when you do get an interrupt. If nr_pass == nr_cpus and noirqs, then you shouldn't have gotten an interrupt. I think it's safe to write it as report("%s", true, testname). I think a short comment above explaining why we do this check (timeout expired and we haven't gotten any interrupts) would also improve readability of the code, but that's up to you. Thanks, Alex > + return; > + } > + > for_each_present_cpu(cpu) { > if (cpumask_test_cpu(cpu, mask)) { > if (!acked[cpu]) _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply index Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-08 14:42 [kvm-unit-tests PATCH 00/17] arm: gic: Test SPIs and interrupt groups Andre Przywara 2019-11-08 14:42 ` [kvm-unit-tests PATCH 01/17] arm: gic: Enable GIC MMIO tests for GICv3 as well Andre Przywara 2019-11-08 17:28 ` Alexandru Elisei 2019-11-12 12:49 ` Auger Eric 2019-11-08 14:42 ` [kvm-unit-tests PATCH 02/17] arm: gic: Generalise function names Andre Przywara 2019-11-12 11:11 ` Alexandru Elisei 2019-11-12 12:49 ` Auger Eric 2019-11-08 14:42 ` [kvm-unit-tests PATCH 03/17] arm: gic: Provide per-IRQ helper functions Andre Przywara 2019-11-12 12:51 ` Alexandru Elisei 2019-11-12 15:53 ` Auger Eric 2019-11-12 16:53 ` Alexandru Elisei 2019-11-12 13:49 ` Auger Eric 2019-11-08 14:42 ` [kvm-unit-tests PATCH 04/17] arm: gic: Support no IRQs test case Andre Przywara 2019-11-12 13:26 ` Alexandru Elisei [this message] 2019-11-12 21:14 ` Auger Eric 2019-11-08 14:42 ` [kvm-unit-tests PATCH 05/17] arm: gic: Prepare IRQ handler for handling SPIs Andre Przywara 2019-11-12 13:36 ` Alexandru Elisei 2019-11-12 20:56 ` Auger Eric 2019-11-08 14:42 ` [kvm-unit-tests PATCH 06/17] arm: gic: Add simple shared IRQ test Andre Przywara 2019-11-12 13:54 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 07/17] arm: gic: Extend check_acked() to allow silent call Andre Przywara 2019-11-12 15:23 ` Alexandru Elisei 2019-11-14 12:32 ` Andrew Jones 2019-11-15 11:32 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 08/17] arm: gic: Add simple SPI MP test Andre Przywara 2019-11-12 15:41 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 09/17] arm: gic: Add test for flipping GICD_CTLR.DS Andre Przywara 2019-11-12 16:42 ` Alexandru Elisei 2019-11-14 13:39 ` Vladimir Murzin 2019-11-14 14:17 ` Andre Przywara 2019-11-14 14:50 ` Vladimir Murzin 2019-11-14 15:21 ` Alexandru Elisei 2019-11-14 15:27 ` Peter Maydell 2019-11-14 15:47 ` Alexandru Elisei 2019-11-14 15:56 ` Peter Maydell 2019-11-08 14:42 ` [kvm-unit-tests PATCH 10/17] arm: gic: Check for writable IGROUPR registers Andre Przywara 2019-11-12 16:51 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 11/17] arm: gic: Check for validity of both group enable bits Andre Przywara 2019-11-12 16:58 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 12/17] arm: gic: Change gic_read_iar() to take group parameter Andre Przywara 2019-11-12 17:19 ` Alexandru Elisei 2019-11-14 12:50 ` Andrew Jones 2019-11-08 14:42 ` [kvm-unit-tests PATCH 13/17] arm: gic: Change write_eoir() " Andre Przywara 2019-11-08 14:42 ` [kvm-unit-tests PATCH 14/17] arm: gic: Prepare for receiving GIC group 0 interrupts via FIQs Andre Przywara 2019-11-12 17:30 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 15/17] arm: gic: Provide FIQ handler Andre Przywara 2019-11-13 10:14 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 16/17] arm: gic: Prepare interrupt statistics for both groups Andre Przywara 2019-11-13 10:44 ` Alexandru Elisei 2019-11-08 14:42 ` [kvm-unit-tests PATCH 17/17] arm: gic: Test Group0 SPIs Andre Przywara 2019-11-13 11:26 ` Alexandru Elisei
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=db89b983-425c-8b45-3f26-1a33b9817836@arm.com \ --to=alexandru.elisei@arm.com \ --cc=andre.przywara@arm.com \ --cc=drjones@redhat.com \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=maz@kernel.org \ --cc=pbonzini@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
KVM ARM Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \ kvmarm@lists.cs.columbia.edu public-inbox-index kvmarm Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm AGPL code for this site: git clone https://public-inbox.org/public-inbox.git