From: Marc Zyngier <maz@kernel.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>,
kvm@vger.kernel.org, shashi.mallela@linaro.org,
eric.auger@redhat.com, qemu-arm@nongnu.org,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@arm.com
Subject: Re: [kvm-unit-tests PATCH v1 1/4] arm64: split its-trigger test into KVM and TCG variants
Date: Wed, 28 Apr 2021 17:46:56 +0100 [thread overview]
Message-ID: <87fszanypr.wl-maz@kernel.org> (raw)
In-Reply-To: <87czues90k.fsf@linaro.org>
On Wed, 28 Apr 2021 16:37:45 +0100,
Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Marc Zyngier <maz@kernel.org> writes:
>
> > On Wed, 28 Apr 2021 15:00:15 +0100,
> > Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> >>
> >> I interpret that as that an INVALL guarantees that a change is
> >> visible, but it the change can become visible even without the
> >> INVALL.
> >
> > Yes. Expecting the LPI to be delivered or not in the absence of an
> > invalidate when its configuration has been altered is wrong. The
> > architecture doesn't guarantee anything of the sort.
>
> Is the underlying hypervisor allowed to invalidate and reload the
> configuration whenever it wants or should it only be driven by the
> guests requests?
The HW can do it at any time. It all depends on whether the RD has
cached this LPI configuration or not. KVM relies on the required
invalidation as a hook to reload the cached state, as it has an
infinite LPI configuration cache, while TCG doesn't have a cache at
all. Both approaches are valid implementations.
> I did consider a more nuanced variant of the test that allowed for a
> delivery pre-inval and a pass for post-inval as long as it had been
> delivered one way or another:
>
> --8<---------------cut here---------------start------------->8---
> modified arm/gic.c
> @@ -36,6 +36,7 @@ static struct gic *gic;
> static int acked[NR_CPUS], spurious[NR_CPUS];
> static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
> static cpumask_t ready;
> +static bool under_tcg;
>
> static void nr_cpu_check(int nr)
> {
> @@ -687,6 +688,7 @@ static void test_its_trigger(void)
> struct its_collection *col3;
> struct its_device *dev2, *dev7;
> cpumask_t mask;
> + bool before, after;
>
> if (its_setup1())
> return;
> @@ -734,15 +736,17 @@ static void test_its_trigger(void)
> /*
> * re-enable the LPI but willingly do not call invall
> * so the change in config is not taken into account.
> - * The LPI should not hit
> + * The LPI should not hit. This does however depend on
This first point is *wrong*. From the architecture spec:
<quote>
* A change to the LPI configuration is not guaranteed to be visible
until an appropriate invalidation operation has completed:
- If one or more ITS is implemented, invalidation is performed using
the INV or INVALL command. A SYNC command completes the INV and
INVALL commands.
</quote>
*not guaranteed* means that it may fire, it may not.
> + * implementation defined behaviour - under QEMU TCG emulation
> + * it can quite correctly process the event directly.
I really don't see the point in testing IMPDEF behaviours. We should
test for architectural compliance, not for implementation choices.
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2021-04-28 16:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-28 10:18 [kvm-unit-tests PATCH v1 0/4] enable LPI and ITS for TCG Alex Bennée
2021-04-28 10:18 ` [kvm-unit-tests PATCH v1 1/4] arm64: split its-trigger test into KVM and TCG variants Alex Bennée
2021-04-28 10:29 ` Marc Zyngier
2021-04-28 12:06 ` Alex Bennée
2021-04-28 14:00 ` Alexandru Elisei
2021-04-28 14:36 ` Marc Zyngier
2021-04-28 15:26 ` Auger Eric
2021-04-28 15:37 ` Alex Bennée
2021-04-28 16:31 ` Alex Bennée
2021-04-28 16:46 ` Marc Zyngier [this message]
2021-04-28 10:18 ` [kvm-unit-tests PATCH v1 2/4] scripts/arch-run: don't use deprecated server/nowait options Alex Bennée
2021-04-28 10:18 ` [kvm-unit-tests PATCH v1 3/4] arm64: enable its-migration tests for TCG Alex Bennée
2021-04-28 10:18 ` [kvm-unit-tests PATCH v1 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants Alex Bennée
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=87fszanypr.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=alex.bennee@linaro.org \
--cc=alexandru.elisei@arm.com \
--cc=christoffer.dall@arm.com \
--cc=eric.auger@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=qemu-arm@nongnu.org \
--cc=shashi.mallela@linaro.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).