Hi Andrew,
On 3/20/19 5:57 PM, Andrew Jones wrote:
> The errata framework doesn't work with kvmtool, so just live
> dangerously and force all tests to run.
>
> Cc: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> ---
> configure | 3 +++
> lib/errata.h | 11 ++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 30112a812d0b..e6b4ca1015bb 100755
> --- a/configure
> +++ b/configure
> @@ -17,6 +17,7 @@ pretty_print_stacks=yes
> environ_default=yes
> u32_long=
> vmm="qemu"
> +errata_force=0
>
> usage() {
> cat <<-EOF
> @@ -118,6 +119,7 @@ elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> arm_uart_early_addr=0x09000000
> elif [ "$vmm" = "kvmtool" ]; then
> arm_uart_early_addr=0x3f8
> + errata_force=1
> else
> echo '--vmm must be one of "qemu" or "kvmtool"!'
> usage
> @@ -226,6 +228,7 @@ if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> cat <<EOF >> lib/config.h
>
> #define CONFIG_UART_EARLY_BASE ${arm_uart_early_addr}
> +#define CONFIG_ERRATA_FORCE ${errata_force}
>
> EOF
> fi
> diff --git a/lib/errata.h b/lib/errata.h
> index f3ebca2d8a6c..5af0eb3bf8e2 100644
> --- a/lib/errata.h
> +++ b/lib/errata.h
> @@ -6,6 +6,11 @@
> */
> #ifndef _ERRATA_H_
> #define _ERRATA_H_
> +#include "config.h"
> +
> +#ifndef CONFIG_ERRATA_FORCE
> +#define CONFIG_ERRATA_FORCE 0
> +#endif
This is a minor thing, but since the CONFIG_ERRATA_FORCE is used in a machine
independent file, perhaps it is better to generate the define unconditionally,
not just for arm/arm64? Since errata_force is set to 1 only on arm/arm64 with
kvmtool, CONFIG_ERRATA_FORCE will be 0 for all other cases, exactly what you are
trying to achieve here.
>
> #define _ERRATA(erratum) errata("ERRATA_" # erratum)
> #define ERRATA(erratum) _ERRATA(erratum)
> @@ -15,8 +20,12 @@
>
> static inline bool errata_force(void)
> {
> - char *s = getenv("ERRATA_FORCE");
> + char *s;
> +
> + if (CONFIG_ERRATA_FORCE == 1)
> + return true;
>
> + s = getenv("ERRATA_FORCE");
> return s && (*s == '1' || *s == 'y' || *s == 'Y');
> }
>
Thank you very much for doing this. I was on holiday and I saw the patch and the
pull request today.
I noticed that the errata framework doesn't work with kvmtool and from my
initial investigations it seems that this is because (1) kvmtool refuses to use
an initrd which hasn't been archived with gzip or cpio and (2) kvmtool writes
the initrd properties to the dtb by using cpu_to_fdt64, and kvm-unit-tests reads
them with fdt32_to_cpu. I was planning to get back to this after the patch set
was merged.
I've tested your patch with kvmtool on the Armv8 simulator and ThunderX2 by
using the psci test. Without the patch, the cpu-on test was being skipped. With
the patch, the cpu-on test was being executed. Also tested with
qemu-system-aarch64; and tested by building kvm-unit-tests for x86.
In case it's not too late, you can add:
Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Tested-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
