| Message ID | 20240429032403.74910-19-smostafa@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SMMUv3 nested translation support | expand |
Hi Mostafa, On 4/29/24 05:24, Mostafa Saleh wrote: > Use the new SMMU property to make the SMMU OAS match the CPU PARANGE. > That's according to SMMU manual ARM IHI 0070F.b: > 6.3.6 SMMU_IDR5, OAS must match the system physical address size. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/virt.c | 14 ++++++++++++-- > target/arm/cpu.h | 2 ++ > target/arm/cpu64.c | 5 +++++ > 3 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 3c93c0c0a6..f203b1f8e1 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -252,6 +252,13 @@ static bool ns_el2_virt_timer_present(void) > arm_feature(env, ARM_FEATURE_EL2) && cpu_isar_feature(aa64_vh, cpu); > } > > +/* We rely on CPU to define system OAS. */ > +static int32_t get_system_oas(void) > +{ > + ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0)); > + return cpu_arm_get_oas(cpu); > +} > + > static void create_fdt(VirtMachineState *vms) > { > MachineState *ms = MACHINE(vms); > @@ -1384,7 +1391,7 @@ static void create_pcie_irq_map(const MachineState *ms, > } > > static void create_smmu(const VirtMachineState *vms, > - PCIBus *bus) > + PCIBus *bus, int32_t oas) > { > char *node; > const char compat[] = "arm,smmu-v3"; > @@ -1404,6 +1411,9 @@ static void create_smmu(const VirtMachineState *vms, > > object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus), > &error_abort); > + > + qdev_prop_set_uint64(dev, "oas", oas); to me you cannot handle that this way because for older machine types the smmu oas needs to stay the same as it was before, ie. 44. So you need to properly handle the compats. Since the new default value depends on the CPU PARANGE this is a bit more tricky but maybe you get can inspired from no_its class field trick in virt.c and introduce a no_smmu_cpu_oas_align to disable the settings up to 9.0 included. Thanks Eric > + > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > for (i = 0; i < NUM_SMMU_IRQS; i++) { > @@ -1578,7 +1588,7 @@ static void create_pcie(VirtMachineState *vms) > > switch (vms->iommu) { > case VIRT_IOMMU_SMMUV3: > - create_smmu(vms, vms->bus); > + create_smmu(vms, vms->bus, get_system_oas()); > qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map", > 0x0, vms->iommu_phandle, 0x0, 0x10000); > break; > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 17efc5d565..68261ffbf9 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -3287,4 +3287,6 @@ static inline target_ulong cpu_untagged_addr(CPUState *cs, target_ulong x) > } > #endif > > +int32_t cpu_arm_get_oas(ARMCPU *cpu); > + > #endif > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 985b1efe16..08da83c082 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -787,6 +787,11 @@ static const gchar *aarch64_gdb_arch_name(CPUState *cs) > return "aarch64"; > } > > +int32_t cpu_arm_get_oas(ARMCPU *cpu) > +{ > + return FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE); > +} > + > static void aarch64_cpu_class_init(ObjectClass *oc, void *data) > { > CPUClass *cc = CPU_CLASS(oc);
Hi Mostafa, On 29/04/2024 04:24, Mostafa Saleh wrote: > Use the new SMMU property to make the SMMU OAS match the CPU PARANGE. > That's according to SMMU manual ARM IHI 0070F.b: > 6.3.6 SMMU_IDR5, OAS must match the system physical address size. > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/virt.c | 14 ++++++++++++-- > target/arm/cpu.h | 2 ++ > target/arm/cpu64.c | 5 +++++ When trying to build qemu-system-arm, I get the following error: [1/3028] Generating subprojects/dtc/version_gen.h with a custom command [2/3028] Generating qemu-version.h with a custom command (wrapped by meson to capture output) [3/3021] Linking target qemu-system-aarch64 [4/3021] Linking target qemu-system-arm FAILED: qemu-system-arm clang -m64 -mcx16 @qemu-system-arm.rsp libqemu-arm-softmmu.fa.p/hw_arm_virt.c.o: In function `get_system_oas': /home/jgrall/works/oss/qemu/build/../hw/arm/virt.c:259: undefined reference to `cpu_arm_get_oas' clang-11: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed. make: *** [run-ninja] Error 1 I think you need to provide cpu_arm_get_oas() also for 32-bit arm (I guess it is implemented in target/arm/cpu.c). Cheers,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 3c93c0c0a6..f203b1f8e1 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -252,6 +252,13 @@ static bool ns_el2_virt_timer_present(void) arm_feature(env, ARM_FEATURE_EL2) && cpu_isar_feature(aa64_vh, cpu); } +/* We rely on CPU to define system OAS. */ +static int32_t get_system_oas(void) +{ + ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0)); + return cpu_arm_get_oas(cpu); +} + static void create_fdt(VirtMachineState *vms) { MachineState *ms = MACHINE(vms); @@ -1384,7 +1391,7 @@ static void create_pcie_irq_map(const MachineState *ms, } static void create_smmu(const VirtMachineState *vms, - PCIBus *bus) + PCIBus *bus, int32_t oas) { char *node; const char compat[] = "arm,smmu-v3"; @@ -1404,6 +1411,9 @@ static void create_smmu(const VirtMachineState *vms, object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus), &error_abort); + + qdev_prop_set_uint64(dev, "oas", oas); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { @@ -1578,7 +1588,7 @@ static void create_pcie(VirtMachineState *vms) switch (vms->iommu) { case VIRT_IOMMU_SMMUV3: - create_smmu(vms, vms->bus); + create_smmu(vms, vms->bus, get_system_oas()); qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map", 0x0, vms->iommu_phandle, 0x0, 0x10000); break; diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 17efc5d565..68261ffbf9 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -3287,4 +3287,6 @@ static inline target_ulong cpu_untagged_addr(CPUState *cs, target_ulong x) } #endif +int32_t cpu_arm_get_oas(ARMCPU *cpu); + #endif diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 985b1efe16..08da83c082 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -787,6 +787,11 @@ static const gchar *aarch64_gdb_arch_name(CPUState *cs) return "aarch64"; } +int32_t cpu_arm_get_oas(ARMCPU *cpu) +{ + return FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE); +} + static void aarch64_cpu_class_init(ObjectClass *oc, void *data) { CPUClass *cc = CPU_CLASS(oc);
Use the new SMMU property to make the SMMU OAS match the CPU PARANGE. That's according to SMMU manual ARM IHI 0070F.b: 6.3.6 SMMU_IDR5, OAS must match the system physical address size. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- hw/arm/virt.c | 14 ++++++++++++-- target/arm/cpu.h | 2 ++ target/arm/cpu64.c | 5 +++++ 3 files changed, 19 insertions(+), 2 deletions(-)