[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 00/24] target/ppc: Clean up mmu translation

From: Richard Henderson
Subject: Re: [PATCH 00/24] target/ppc: Clean up mmu translation
Date: Wed, 19 May 2021 17:47:05 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

On 5/19/21 3:37 PM, Richard Henderson wrote:
On 5/18/21 9:52 PM, David Gibson wrote:
I've applied 1..15, still looking at the rest.

Please dequeue.  I want to create a new mmu-internal.h, which affects all the patches from #1.

Alternately, don't. I can move the function later, and it may be a while before I can get back to this.

Two outstanding bugs:

(1) mmu-radix64.c vs hypervisors. You'll not see these unless you run kvm inside of tcg.

Basically, all usage of msr_{hv,pr,ir,dr} within ppc_*_xlate is incorrect. We should be pulling these from the 3 bits of mmu_idx, as outlined in the table in hreg_compute_hflags_value.

When you start propagating that around, you see that the second-level translation for loading the pte (2 of the 3 calls to ppc_radix64_partition_scoped_xlate) should not be using the mmu_idx related to the user-mode guest access, but should be using the mmu_idx of the kernel/hypervisor that owns the page table.

I can't see that mmu-radix64.c has the same problem. I'm not really sure how the second-level translation for hypervisors works there. Is it by the hypervisor altering the hash table as it is loaded?

(2) The direct-segment accesses for 6xx and hash32 use ACCESS_* to conditionally reject an mmu access. This is flawed, because we only arrive into these translation routines on the softtlb slow path. If we have an ACCESS_INT and then an ACCESS_FLOAT, the first will load a tlb entry which the second will use to stay on the fast path.

There are several possible solutions:

 (A) Give tlb_set_page size == 1 for direct-segment addresses.
     This will cause cputlb.c to force all references to the page
     back through the slow path and through tlb_fill.  At which
     point env->access_type is correct for each access, and we
     can reject on type.

     This could be really slow.  But since these direct-segment
     accesses are also uncached, I would expect the guest OS to
     only be using them for i/o access.  Which is already slow,
     so perhaps the extra trip through tlb_fill isn't noticeable.

 (B) Use additional mmu_idx.  Not ideal, since we wouldn't be
     sharing softtlb entries for ACCESS_INT and ACCESS_FLOAT
     and ACCESS_RES for the normal case.  But the relevant
     mmu_models do not have hypervisor support, so we could
     still fit them in to 8 mmu_idx.  We'd probably want to
     use special code for ACCESS_CACHE and ACCESS_EXT here.

 (C) Ignore this special case entirely, dropping everything
     related to env->access_type.  The section of code that
     uses them is all for really old machine types with
     comments like

        /* Direct-store segment : absolutely *BUGGY* for now */

     which is not encouraging.  And short of writing our own
     test cases we're not likely to have any code to exercise


reply via email to

[Prev in Thread] Current Thread [Next in Thread]