Re: [PATCH 2/2] hw/arm/imx8mp-evk: Add KVM support

[Bernhard Beschow <shentey@gmail.com>]

Am 28. Oktober 2025 12:46:34 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Sun, 29 Jun 2025 at 21:49, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Allows the imx8mp-evk machine to be run with KVM acceleration as a guest.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  docs/system/arm/imx8mp-evk.rst |  7 +++++++
>>  hw/arm/fsl-imx8mp.c            | 33 ++++++++++++++++++++++++++++-----
>>  hw/arm/imx8mp-evk.c            | 11 +++++++++++
>>  hw/arm/Kconfig                 |  3 ++-
>>  hw/arm/meson.build             |  2 +-
>>  5 files changed, 49 insertions(+), 7 deletions(-)
>>
>> diff --git a/docs/system/arm/imx8mp-evk.rst b/docs/system/arm/imx8mp-evk.rst
>> index b2f7d29ade..1399820163 100644
>> --- a/docs/system/arm/imx8mp-evk.rst
>> +++ b/docs/system/arm/imx8mp-evk.rst
>> @@ -60,3 +60,10 @@ Now that everything is prepared the machine can be started as follows:
>>        -dtb imx8mp-evk.dtb \
>>        -append "root=/dev/mmcblk2p2" \
>>        -drive file=sdcard.img,if=sd,bus=2,format=raw,id=mmcblk2
>> +
>> +
>> +KVM Virtualization
>> +------------------
>> +
>> +To enable hardware-assisted acceleration via KVM, append
>> +``-accel kvm -cpu host`` to the command line.
>
>Coming back to this now we've resolved the "does this put
>things inside our security-promises that we don't want"
>question...
>
>I think we should be a bit clearer in the documentation
>about what tradeoffs the user is making here when they select
>KVM. Specifically:
>
> * we should note that this is intended only to improve
>   performance, and is not covered by QEMU's security policy

Sure, I'll add it.

> * we should say that you will not get a Cortex-A53, so any
>   guest code with tight dependencies on the host CPU type
>   might not work correctly

Ack. I'd also hardcode the CPU type to host since asking for a Cortex-A53 always failed on me with KVM.

> * we should say that the guest will only be able to run
>   at EL1, and (unlike TCG) there is no EL2 or EL3

Real U-Boot calls back into the on-chip ROM which isn't implemented yet. Furthermore, there are some unimplemented USDHC extensions which prevent complete loading of binaries into RAM by U-Boot (similar limitation exists for e500 boards). Therefore the board documentation only advertises direct kernel boot. AFAIU EL2 and EL3 aren't usable there anyway. Correct? Do we need to mention this limitation regardless?

>
>> diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
>> index 866f4d1d74..7e61392abb 100644
>> --- a/hw/arm/fsl-imx8mp.c
>> +++ b/hw/arm/fsl-imx8mp.c
>> @@ -12,11 +12,13 @@
>>  #include "system/address-spaces.h"
>>  #include "hw/arm/bsa.h"
>>  #include "hw/arm/fsl-imx8mp.h"
>> -#include "hw/intc/arm_gicv3.h"
>
>Why does this include get removed ?

It was used for accessing `TYPE_ARM_GICV3` which has been replaced by `gicv3_class_name()` whose header is included in fsl-imx8mp.h already.

>
>>  #include "hw/misc/unimp.h"
>>  #include "hw/boards.h"
>> +#include "system/kvm.h"
>>  #include "system/system.h"
>> +#include "target/arm/cpu.h"
>>  #include "target/arm/cpu-qom.h"
>> +#include "target/arm/kvm_arm.h"
>>  #include "qapi/error.h"
>>  #include "qobject/qlist.h"
>
>> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
>> index d90be8f4c9..a4212a6ab2 100644
>> --- a/hw/arm/meson.build
>> +++ b/hw/arm/meson.build
>> @@ -59,7 +59,7 @@ arm_common_ss.add(when: 'CONFIG_MUSCA', if_true: files('musca.c'))
>>  arm_common_ss.add(when: 'CONFIG_ARMSSE', if_true: files('armsse.c'))
>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre.c'))
>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
>> -arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
>> +arm_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
>>  arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
>>  arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c'))
>>  arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
>
>Philippe, Pierrick: is it OK that this moves the
>fsl-imx8p.c file from arm_common to arm_ss, or is there
>a preferable way to do this from a single-binary point
>of view?

Hardcoding to host CPU type in the KVM case might also resolve this issue.

Thanks,
Bernhard

>
>thanks
>-- PMM