diff mbox series

[v3,18/26] hw/arm/boot: Mark all guest memory as RIPAS_RAM.

Message ID 20241125195626.856992-20-jean-philippe@linaro.org
State New
Headers show
Series arm: Run Arm CCA VMs with KVM | expand

Commit Message

Jean-Philippe Brucker Nov. 25, 2024, 7:56 p.m. UTC 
All Realm IPA states are by default RIPAS_EMPTY, and accessing them in
that state causes injection of synchronous exception. Either the loader
or the guest needs to set IPA state to RIPAS_RAM before accessing it.
Since a Linux guest needs all memory ready at boot [1], initialize it
here.

[1] https://docs.kernel.org/arch/arm64/booting.html
    https://lore.kernel.org/all/20241004144307.66199-12-steven.price@arm.com/

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v2->v3: New: the Linux guest does not initialize RIPAS itself anymore,
and expects the loader to do it.
---
 hw/arm/boot.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Gavin Shan Feb. 4, 2025, 7:27 a.m. UTC | #1 
On 11/26/24 5:56 AM, Jean-Philippe Brucker wrote:
> All Realm IPA states are by default RIPAS_EMPTY, and accessing them in
> that state causes injection of synchronous exception. Either the loader
> or the guest needs to set IPA state to RIPAS_RAM before accessing it.
> Since a Linux guest needs all memory ready at boot [1], initialize it
> here.
> 
> [1] https://docs.kernel.org/arch/arm64/booting.html
>      https://lore.kernel.org/all/20241004144307.66199-12-steven.price@arm.com/
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> v2->v3: New: the Linux guest does not initialize RIPAS itself anymore,
> and expects the loader to do it.
> ---
>   hw/arm/boot.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 

I think the changes in this patch can be combined to the following one,
or move this after it. It logically to initialize guest's RAM covered
by PATCH[9]. I'm not sure if there is particular reasonable why we
don't do that.

PATCH[09/26] target/arm/kvm-rme: Initialize Realm memory

> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 5301d8d318..a2414b1f98 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -26,6 +26,7 @@
>   #include "qemu/config-file.h"
>   #include "qemu/option.h"
>   #include "qemu/units.h"
> +#include "kvm_arm.h"
>   
>   /* Kernel boot protocol is specified in the kernel docs
>    * Documentation/arm/Booting and Documentation/arm64/booting.txt
> @@ -1238,6 +1239,9 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, struct arm_boot_info *info)
>       info->dtb_filename = ms->dtb;
>       info->dtb_limit = 0;
>   
> +    /* Mark all Realm memory as RAM */
> +    kvm_arm_rme_init_guest_ram(info->loader_start, info->ram_size);
> +
>       /* Load the kernel.  */
>       if (!info->kernel_filename || info->firmware_loaded) {
>           arm_setup_firmware_boot(cpu, info);

Thanks,
Gavin
Jean-Philippe Brucker Feb. 7, 2025, 4:02 p.m. UTC | #2 
On Tue, Feb 04, 2025 at 05:27:17PM +1000, Gavin Shan wrote:
> On 11/26/24 5:56 AM, Jean-Philippe Brucker wrote:
> > All Realm IPA states are by default RIPAS_EMPTY, and accessing them in
> > that state causes injection of synchronous exception. Either the loader
> > or the guest needs to set IPA state to RIPAS_RAM before accessing it.
> > Since a Linux guest needs all memory ready at boot [1], initialize it
> > here.
> > 
> > [1] https://docs.kernel.org/arch/arm64/booting.html
> >      https://lore.kernel.org/all/20241004144307.66199-12-steven.price@arm.com/
> > 
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> > v2->v3: New: the Linux guest does not initialize RIPAS itself anymore,
> > and expects the loader to do it.
> > ---
> >   hw/arm/boot.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> 
> I think the changes in this patch can be combined to the following one,
> or move this after it. It logically to initialize guest's RAM covered
> by PATCH[9]. I'm not sure if there is particular reasonable why we
> don't do that.

If I understand correctly you mean I should merge patches
9 and 18, or at least bring them closer together in the series?

I'd like to keep the hw/arm and target/arm changes in separate patches,
because it's a nice way to have smaller patches and gives a clean commit
message. But I could interleave the hw/arm and target/arm changes within
the series (where it makes sense like here), it may be easier to review

For now I pushed the reworked series to branch cca/latest
https://git.codelinaro.org/linaro/dcap/qemu/-/tree/cca/latest

Thanks,
Jean

> 
> PATCH[09/26] target/arm/kvm-rme: Initialize Realm memory
diff mbox series

Patch

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5301d8d318..a2414b1f98 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -26,6 +26,7 @@ 
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/units.h"
+#include "kvm_arm.h"
 
 /* Kernel boot protocol is specified in the kernel docs
  * Documentation/arm/Booting and Documentation/arm64/booting.txt
@@ -1238,6 +1239,9 @@  void arm_load_kernel(ARMCPU *cpu, MachineState *ms, struct arm_boot_info *info)
     info->dtb_filename = ms->dtb;
     info->dtb_limit = 0;
 
+    /* Mark all Realm memory as RAM */
+    kvm_arm_rme_init_guest_ram(info->loader_start, info->ram_size);
+
     /* Load the kernel.  */
     if (!info->kernel_filename || info->firmware_loaded) {
         arm_setup_firmware_boot(cpu, info);