From: Thomas Huth <thuth@redhat.com>
To: Eric Auger <eric.auger@redhat.com>,
eric.auger.pro@gmail.com, maz@kernel.org,
kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
qemu-devel@nongnu.org, qemu-arm@nongnu.org
Cc: drjones@redhat.com, andre.przywara@arm.com,
peter.maydell@linaro.org, yuzenghui@huawei.com,
alexandru.elisei@arm.com
Subject: Re: [kvm-unit-tests PATCH v3 12/14] arm/run: Allow Migration tests
Date: Wed, 29 Jan 2020 09:07:54 +0100 [thread overview]
Message-ID: <3962373a-0e03-5ab9-30cc-3b385fc55702@redhat.com> (raw)
In-Reply-To: <20200128103459.19413-13-eric.auger@redhat.com>
On 28/01/2020 11.34, Eric Auger wrote:
> Let's link getchar.o to use puts and getchar from the
> tests.
>
> Then allow tests belonging to the migration group to
> trigger the migration from the test code by putting
> "migrate" into the uart. Then the code can wait for the
> migration completion by using getchar().
>
> The __getchar implement is minimalist as it just reads the
> data register. It is just meant to read the single character
> emitted at the end of the migration by the runner script.
>
> It is not meant to read more data (FIFOs are not enabled).
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v2 -> v3:
> - take the lock
> - assert if more than 16 chars
> - removed Thomas' R-b
> ---
> arm/Makefile.common | 2 +-
> arm/run | 2 +-
> lib/arm/io.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index b8988f2..a123e85 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -32,7 +32,7 @@ CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
> asm-offsets = lib/$(ARCH)/asm-offsets.h
> include $(SRCDIR)/scripts/asm-offsets.mak
>
> -cflatobjs += lib/util.o
> +cflatobjs += lib/util.o lib/getchar.o
> cflatobjs += lib/alloc_phys.o
> cflatobjs += lib/alloc_page.o
> cflatobjs += lib/vmalloc.o
> diff --git a/arm/run b/arm/run
> index 277db9b..a390ca5 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -61,6 +61,6 @@ fi
> M+=",accel=$ACCEL"
> command="$qemu -nodefaults $M -cpu $processor $chr_testdev $pci_testdev"
> command+=" -display none -serial stdio -kernel"
> -command="$(timeout_cmd) $command"
> +command="$(migration_cmd) $(timeout_cmd) $command"
>
> run_qemu $command "$@"
> diff --git a/lib/arm/io.c b/lib/arm/io.c
> index 99fd315..d8e7745 100644
> --- a/lib/arm/io.c
> +++ b/lib/arm/io.c
> @@ -87,6 +87,34 @@ void puts(const char *s)
> spin_unlock(&uart_lock);
> }
>
> +static int ____getchar(void)
Three underscores? ... that's quite a lot already. I'd maybe rather name
the function "do_getchar" or something similar instead. Or simply merge
the code into the __getchar function below - it's just three lines.
> +{
> + int c;
> +
> + spin_lock(&uart_lock);
> + c = readb(uart0_base);
> + spin_unlock(&uart_lock);
> +
> + return c ? : -1;
Just a matter of taste, but I prefer the elvis operator without space in
between.
> +}
> +
> +/*
> + * Minimalist implementation for migration completion detection.
> + * Without FIFOs enabled on the QEMU UART device we just read
> + * the data register: we cannot read more than 16 characters.
Where are the 16 bytes buffered if FIFOs are disabled?
> + */
> +int __getchar(void)
> +{
> + int c = ____getchar();
> + static int count;
> +
> + if (c != -1)
> + ++count;
> +
> + assert(count < 16);
> +
> + return c;
> +}
The above comments were only nits ... feel free to ignore them if you
don't want to respin the series just because of this.
Thomas
next prev parent reply other threads:[~2020-01-29 8:08 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-28 10:34 [kvm-unit-tests PATCH v3 00/14] arm/arm64: Add ITS tests Eric Auger
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 01/14] libcflat: Add other size defines Eric Auger
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 02/14] arm: gic: Provide per-IRQ helper functions Eric Auger
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 03/14] arm/arm64: gic: Introduce setup_irq() helper Eric Auger
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 04/14] arm/arm64: gicv3: Add some re-distributor defines Eric Auger
2020-02-06 14:35 ` Zenghui Yu
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 05/14] arm/arm64: ITS: Introspection tests Eric Auger
2020-02-06 15:12 ` Zenghui Yu
2020-02-07 10:19 ` Andrew Jones
2020-03-04 14:20 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 06/14] arm/arm64: gicv3: Set the LPI config and pending tables Eric Auger
2020-02-07 2:12 ` Zenghui Yu
2020-03-05 19:40 ` Auger Eric
2020-02-07 12:11 ` Andrew Jones
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 07/14] arm/arm64: gicv3: Enable/Disable LPIs at re-distributor level Eric Auger
2020-02-07 12:14 ` Andrew Jones
2020-02-07 12:19 ` Andrew Jones
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 08/14] arm/arm64: ITS: its_enable_defaults Eric Auger
2020-02-07 3:20 ` Zenghui Yu
2020-03-04 14:26 ` Auger Eric
2020-03-05 6:30 ` Zenghui Yu
2020-02-07 12:41 ` Andrew Jones
2020-03-05 17:59 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 09/14] arm/arm64: ITS: Device and collection Initialization Eric Auger
2020-02-07 5:41 ` Zenghui Yu
2020-03-05 19:42 ` Auger Eric
2020-02-07 12:51 ` Andrew Jones
2020-03-06 8:47 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 10/14] arm/arm64: ITS: commands Eric Auger
2020-02-07 13:37 ` Andrew Jones
2020-03-06 9:13 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 11/14] arm/arm64: ITS: INT functional tests Eric Auger
2020-02-07 13:15 ` Andrew Jones
2020-03-06 12:55 ` Auger Eric
2020-03-06 13:29 ` Andrew Jones
2020-03-06 13:40 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 12/14] arm/run: Allow Migration tests Eric Auger
2020-01-29 8:07 ` Thomas Huth [this message]
2020-01-29 9:29 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 13/14] arm/arm64: ITS: migration tests Eric Auger
2020-02-07 13:49 ` Andrew Jones
2020-03-06 13:06 ` Auger Eric
2020-01-28 10:34 ` [kvm-unit-tests PATCH v3 14/14] arm/arm64: ITS: pending table migration test Eric Auger
2020-02-07 14:06 ` Andrew Jones
2020-03-06 13:21 ` Auger Eric
2020-03-06 13:36 ` Andrew Jones
2020-03-06 13:41 ` Auger Eric
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=3962373a-0e03-5ab9-30cc-3b385fc55702@redhat.com \
--to=thuth@redhat.com \
--cc=alexandru.elisei@arm.com \
--cc=andre.przywara@arm.com \
--cc=drjones@redhat.com \
--cc=eric.auger.pro@gmail.com \
--cc=eric.auger@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=maz@kernel.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=yuzenghui@huawei.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
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).