On Sat, 1 Oct 2022 at 17:52, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> So far, limit the change to S1_ptw_translate, arm_ldl_ptw, and
> arm_ldq_ptw. Use probe_access_full to find the host address,
> and if so use a host load. If the probe fails, we've got our
> fault info already. On the off chance that page tables are not
> in RAM, continue to use the address_space_ld* functions.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> -static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs)
> +static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs)
> {
> /*
> * For an S1 page table walk, the stage 1 attributes are always
> @@ -202,41 +203,72 @@ static bool ptw_attrs_are_device(uint64_t hcr,
> ARMCacheAttrs cacheattrs)
> * With HCR_EL2.FWB == 1 this is when descriptor bit [4] is 0, ie
> * when cacheattrs.attrs bit [2] is 0.
> */
> - assert(cacheattrs.is_s2_format);
> if (hcr & HCR_FWB) {
> - return (cacheattrs.attrs & 0x4) == 0;
> + return (attrs & 0x4) == 0;
> } else {
> - return (cacheattrs.attrs & 0xc) == 0;
> + return (attrs & 0xc) == 0;
> }
The upcoming v8R support has its stage 2 attributes in the MAIR
format, so it might be a little awkward to assume the v8A-stage-2
format here rather than being able to add the "if !is_s2_format"
condition. I guess we'll deal with that when we get to it...
> + env->tlb_fi = fi;
> + flags = probe_access_full(env, addr, MMU_DATA_LOAD,
> + arm_to_core_mmu_idx(s2_mmu_idx),
> + true, hphys, &full, 0);
> + env->tlb_fi = NULL;
> +
> + if (unlikely(flags & TLB_INVALID_MASK)) {
> + goto fail;
> + }
> + *gphys = full->phys_addr;
> + pte_attrs = full->pte_attrs;
> + pte_secure = full->attrs.secure;
> + }
> +
> --- a/target/arm/tlb_helper.c
> +++ b/target/arm/tlb_helper.c
> @@ -208,10 +208,21 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> bool probe, uintptr_t retaddr)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> - ARMMMUFaultInfo fi = {};
> GetPhysAddrResult res = {};
> + ARMMMUFaultInfo local_fi, *fi;
> int ret;
>
> + /*
> + * Allow S1_ptw_translate to see any fault generated here.
> + * Since this may recurse, read and clear.
> + */
> + fi = cpu->env.tlb_fi;
> + if (fi) {
> + cpu->env.tlb_fi = NULL;
> + } else {
> + fi = memset(&local_fi, 0, sizeof(local_fi));
> + }
This makes two architectures now that want to do "call a probe_access
function, and get information that's known in the architecture-specific
tlb_fill function", and need to do it via this awkward "have tlb_fill
know that it should stash the info away in the CPU state struct somewhere"
trick (the other being s390 tlb_fill_exc/tlb_fill_tec). But I don't
really have a better idea, so
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
