On 27.07.21 11:38, Dongjiu Geng wrote: > Jan Kiszka <[email protected]> 于2021年7月26日周一 下午10:29写道： >> >> On 26.07.21 14:45, Dongjiu Geng wrote: >>> Jan Kiszka <[email protected]> 于2021年7月26日周一 下午6:22写道： >>>> >>>> On 26.07.21 12:05, Dongjiu Geng wrote: >>>>> According to spec, the {I}PS should be never larger >>>>> than the CPU hardware implemented physical address >>>>> size(ID_AA64MMFR0_EL1.PARange). Otherwise, it >>>>> may lead to some unexpected issues. >>>>> >>>>> we can refer to DDI0487G_a_armv8_arm's description below: >>>>> If {I}PS is programmed to a value larger than the >>>>> implemented PA size, then the PE behaves as if programmed >>>>> with the implemented PA size, but software must not rely >>>>> on this behavior. That is, the output address size is never >>>>> largerthan the implemented PA size. >>>>> >>>>> Signed-off-by: Dongjiu Geng <[email protected]> >>>>> --- >>>>> DDI0487G_a_armv8_arm: Physical address size implementation options >>>>> ID_AA64MMFR0_EL1.PARange Total PA size PA address size >>>>> 0000 4GB 32 bits, PA[31:0] >>>>> 0001 64GB 36 bits, PA[35:0] >>>>> 0010 1TB 40 bits, PA[39:0] >>>>> 0011 4TB 42 bits, PA[41:0] >>>>> 0100 16TB 44 bits, PA[43:0] >>>>> 0101 256TB 48 bits, PA[47:0] >>>>> 0110 4PB 52 bits, PA[51:0] >>>>> --- >>>>> hypervisor/arch/arm64/entry.S | 14 +++++++++++++- >>>>> hypervisor/arch/arm64/include/asm/paging.h | 5 +++++ >>>>> 2 files changed, 18 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/hypervisor/arch/arm64/entry.S b/hypervisor/arch/arm64/entry.S >>>>> index 3f4ee871..99738f28 100644 >>>>> --- a/hypervisor/arch/arm64/entry.S >>>>> +++ b/hypervisor/arch/arm64/entry.S >>>>> @@ -388,6 +388,16 @@ el1_trap: >>>>> isb >>>>> .endm >>>>> >>>>> +/* >>>>> + * set TCR.(I)PS to the highest supported ID_AA64MMFR0_EL1.PARange value >>>>> + */ >>>>> +.macro tcr_compute_pa_size, tcr >>>>> + mrs x9, id_aa64mmfr0_el1 >>>>> + // Narrow PARange to fit the PS field in TCR_ELx >>>>> + ubfx x9, x9, #ID_AA64MMFR0_PARANGE_SHIFT, #3 >>>>> + bfi \tcr, x9, #TCR_PS_SHIFT, #3 >>>>> +.endm >>>>> + >>>> >>>> Why a macro, why not inlined? >>> >>> Thanks very much for your point out, yes, It's best to inline rather >>> than macro definitions >>> >>>> >>>>> /* >>>>> * These are the default vectors. They are used on early startup and if >>>>> no >>>>> * Spectre v2 mitigation is available. >>>>> @@ -460,8 +470,10 @@ enable_mmu_el2: >>>>> ldr x1, =(T0SZ(48) | (TCR_RGN_WB_WA << TCR_IRGN0_SHIFT) \ >>>>> | (TCR_RGN_WB_WA << TCR_ORGN0_SHIFT) \ >>>>> | (TCR_INNER_SHAREABLE << TCR_SH0_SHIFT) \ >>>>> - | (PARANGE_48B << TCR_PS_SHIFT) \ >>>>> | TCR_EL2_RES1) >>>>> + >>>>> + tcr_compute_pa_size x1 >>>>> + >>>> >>>> So this is aiming at devices that have less than 48 bits parange, right? >>>> Did you stumble over such a hardware? Or is this rather about devices >>>> having more than 48 bits? Sorry, still trying to understand the details. >>> >>> Yes, I have a board with ARMv8 Cortex-A53 CPU, the Cortex-A53 CPU only >>> supports 40 bits[1], >>> not 48 bits。 And even some ARMv8 CPU support 53 bits parange, also not 48 >>> bits. >>> >> >> But the problem is around parange < 48, right? >> >>> >>> [1] >>> https://montcs.bloomu.edu/Information/RaspberryPi-ARMv8/technical-reference-manual.DDI0500J_cortex_a53_trm.pdf >>> Table 4-56 ID_AA64MMFR0_EL1 bit assignments >>> >>> [3:0] PARange Physical address range supported: >>> 0b0010 40 bits, 1 TB. >>> >> >> OK - but the raspi was apparently still working, although out of spec. >> Or did you see something breaking? Looking for a classification of this >> issue ("ugly, but we were lucky so far" vs. "never worked with X / when >> doing Y"). > > ugly, but we were lucky so far, it was still working, The reason is > that the CPU will ignore this configured value if this configured > value is larger than the CPU supported, but software must not rely. > We can refer to Linux kernel code that sets the TCR.{I}PS according > to ID_AA64MMFR0_EL1.PARange. > > I found this mismatch issue when I was debugging, and think it is > better not using hard coded to set it, so change it. >
I don't disagree. Awaiting v2 of your patch. Thanks, Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/417abc6d-42e5-61ff-1b7f-cdcf8775b609%40siemens.com.
