diff mbox series

[v2,5/5] arm: qemu: override flash accessors to use virtualizable instructions

Message ID 20200611081707.195994-6-ardb@kernel.org
State New
Headers show
Series Fixes for running U-boot under QEMU/KVM | expand

Commit Message

Ard Biesheuvel June 11, 2020, 8:17 a.m. UTC
Some instructions in the ARM ISA have multiple output registers, such
as ldrd/ldp (load pair), where two registers are loaded from memory,
but also ldr with indexing, where the memory base register is incremented
as well when the value is loaded to the destination register.

MMIO emulation under KVM is based on using the architecturally defined
syndrome information that is provided when an exception is taken to the
hypervisor. This syndrome information describes whether the instruction
that triggered the exception is a load or a store, what the faulting
address was, and which register was the destination register.

This syndrome information can only describe one destination register, and
when the trapping instruction is one with multiple outputs, KVM throws an
error like

  kvm [615929]: Data abort outside memslots with no valid syndrome info

on the host and kills the QEMU process with the following error:

  U-Boot 2020.07-rc3-00208-g88bd5b179360-dirty (Jun 06 2020 - 11:59:22 +0200)

  DRAM:  1 GiB
  Flash: error: kvm run failed Function not implemented
  R00=00000001 R01=00000040 R02=7ee0ce20 R03=00000000
  R04=7ffd9eec R05=00000004 R06=7ffda3f8 R07=00000055
  R08=7ffd9eec R09=7ef0ded0 R10=7ee0ce20 R11=00000000
  R12=00000004 R13=7ee0cdf8 R14=00000000 R15=7ff72d08
  PSR=200001d3 --C- A svc32
  QEMU: Terminated

This means that, in order to run U-Boot in QEMU under KVM, we need to
avoid such instructions when accessing emulated devices. For the flash
in particular, which is a hybrid between a ROM (backed by a read-only
KVM memslot) when in array mode, and an emulated MMIO device (when in
write mode), we need to take care to only use instructions that KVM can
deal with when they trap.

So override the flash read accessors that are used when running on QEMU
under KVM. Note that the the 64-bit wide read accessors and all the write
accessors have been omitted: they are either never used to begin with, or
don't suffer from the MMIO emulation issue (as str instructions don't have
multiple output registers)

Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
---
 board/emulation/qemu-arm/qemu-arm.c | 30 ++++++++++++++++++++
 include/configs/qemu-arm.h          |  1 +
 2 files changed, 31 insertions(+)

Comments

Ard Biesheuvel June 13, 2020, 9:45 p.m. UTC | #1
On Thu, 11 Jun 2020 at 10:17, Ard Biesheuvel <ardb at kernel.org> wrote:
>
> Some instructions in the ARM ISA have multiple output registers, such
> as ldrd/ldp (load pair), where two registers are loaded from memory,
> but also ldr with indexing, where the memory base register is incremented
> as well when the value is loaded to the destination register.
>
> MMIO emulation under KVM is based on using the architecturally defined
> syndrome information that is provided when an exception is taken to the
> hypervisor. This syndrome information describes whether the instruction
> that triggered the exception is a load or a store, what the faulting
> address was, and which register was the destination register.
>
> This syndrome information can only describe one destination register, and
> when the trapping instruction is one with multiple outputs, KVM throws an
> error like
>
>   kvm [615929]: Data abort outside memslots with no valid syndrome info
>
> on the host and kills the QEMU process with the following error:
>
>   U-Boot 2020.07-rc3-00208-g88bd5b179360-dirty (Jun 06 2020 - 11:59:22 +0200)
>
>   DRAM:  1 GiB
>   Flash: error: kvm run failed Function not implemented
>   R00=00000001 R01=00000040 R02=7ee0ce20 R03=00000000
>   R04=7ffd9eec R05=00000004 R06=7ffda3f8 R07=00000055
>   R08=7ffd9eec R09=7ef0ded0 R10=7ee0ce20 R11=00000000
>   R12=00000004 R13=7ee0cdf8 R14=00000000 R15=7ff72d08
>   PSR=200001d3 --C- A svc32
>   QEMU: Terminated
>
> This means that, in order to run U-Boot in QEMU under KVM, we need to
> avoid such instructions when accessing emulated devices. For the flash
> in particular, which is a hybrid between a ROM (backed by a read-only
> KVM memslot) when in array mode, and an emulated MMIO device (when in
> write mode), we need to take care to only use instructions that KVM can
> deal with when they trap.
>
> So override the flash read accessors that are used when running on QEMU
> under KVM. Note that the the 64-bit wide read accessors and all the write
> accessors have been omitted: they are either never used to begin with, or
> don't suffer from the MMIO emulation issue (as str instructions don't have
> multiple output registers)
>

This is inaccurate: str instructions do not have multiple output
registers, but they may have an output register (for pre/post
increment) as well as an input register (for the value being stored),
and this cannot be described by the syndrome information. So I will
need to respin this once more.


> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  board/emulation/qemu-arm/qemu-arm.c | 30 ++++++++++++++++++++
>  include/configs/qemu-arm.h          |  1 +
>  2 files changed, 31 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 1b0d543b93c1..524e302001ec 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -142,3 +142,33 @@ efi_status_t platform_get_rng_device(struct udevice **dev)
>         return EFI_SUCCESS;
>  }
>  #endif /* CONFIG_EFI_RNG_PROTOCOL */
> +
> +#ifdef CONFIG_ARM64
> +#define __W    "w"
> +#else
> +#define __W
> +#endif
> +
> +u8 flash_read8(void *addr)
> +{
> +       u8 ret;
> +
> +       asm("ldrb %" __W "0, %1" : "=r"(ret) : "m"(*(u8 *)addr));
> +       return ret;
> +}
> +
> +u16 flash_read16(void *addr)
> +{
> +       u16 ret;
> +
> +       asm("ldrh %" __W "0, %1" : "=r"(ret) : "m"(*(u16 *)addr));
> +       return ret;
> +}
> +
> +u32 flash_read32(void *addr)
> +{
> +       u32 ret;
> +
> +       asm("ldr %" __W "0, %1" : "=r"(ret) : "m"(*(u32 *)addr));
> +       return ret;
> +}
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 1ef75a87836b..bc8b7c5c1238 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -53,5 +53,6 @@
>  #define CONFIG_SYS_MAX_FLASH_BANKS     2
>  #endif
>  #define CONFIG_SYS_MAX_FLASH_SECT      256 /* Sector: 256K, Bank: 64M */
> +#define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
>
>  #endif /* __CONFIG_H */
> --
> 2.26.2
>
diff mbox series

Patch

diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 1b0d543b93c1..524e302001ec 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -142,3 +142,33 @@  efi_status_t platform_get_rng_device(struct udevice **dev)
 	return EFI_SUCCESS;
 }
 #endif /* CONFIG_EFI_RNG_PROTOCOL */
+
+#ifdef CONFIG_ARM64
+#define __W	"w"
+#else
+#define __W
+#endif
+
+u8 flash_read8(void *addr)
+{
+	u8 ret;
+
+	asm("ldrb %" __W "0, %1" : "=r"(ret) : "m"(*(u8 *)addr));
+	return ret;
+}
+
+u16 flash_read16(void *addr)
+{
+	u16 ret;
+
+	asm("ldrh %" __W "0, %1" : "=r"(ret) : "m"(*(u16 *)addr));
+	return ret;
+}
+
+u32 flash_read32(void *addr)
+{
+	u32 ret;
+
+	asm("ldr %" __W "0, %1" : "=r"(ret) : "m"(*(u32 *)addr));
+	return ret;
+}
diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index 1ef75a87836b..bc8b7c5c1238 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -53,5 +53,6 @@ 
 #define CONFIG_SYS_MAX_FLASH_BANKS	2
 #endif
 #define CONFIG_SYS_MAX_FLASH_SECT	256 /* Sector: 256K, Bank: 64M */
+#define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
 
 #endif /* __CONFIG_H */