On 08.07.2011, at 01:41, Scott Wood wrote:
> Split out the portions of tlbe_priv that should be associated with host
> entries into tlbe_ref. Base victim selection on the number of hardware
> entries, not guest entries.
>
> For TLB1, where one guest entry can be mapped by multiple host entries,
> we use the host tlbe_ref for tracking page references. For the guest
> TLB0 entries, we still track it with gtlb_priv, to avoid having to
> retranslate if the entry is evicted from the host TLB but not the
> guest TLB.
>
> Signed-off-by: Scott Wood <scottwood@xxxxxxxxxxxxx>
> ---
> arch/powerpc/include/asm/kvm_e500.h | 20 +++-
> arch/powerpc/include/asm/mmu-book3e.h | 1 +
> arch/powerpc/kvm/e500_tlb.c | 265 +++++++++++++++++++++++----------
> arch/powerpc/kvm/e500_tlb.h | 17 --
> 4 files changed, 207 insertions(+), 96 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_e500.h b/arch/powerpc/include/asm/kvm_e500.h
> index adbfca9..87aab98 100644
> --- a/arch/powerpc/include/asm/kvm_e500.h
> +++ b/arch/powerpc/include/asm/kvm_e500.h
> @@ -32,11 +32,15 @@ struct tlbe{
> #define E500_TLB_VALID 1
> #define E500_TLB_DIRTY 2
>
> -struct tlbe_priv {
> +struct tlbe_ref {
> pfn_t pfn;
> unsigned int flags; /* E500_TLB_* */
> };
>
> +struct tlbe_priv {
> + struct tlbe_ref ref; /* TLB0 only -- TLB1 uses tlb_refs */
> +};
> +
> struct vcpu_id_table;
>
> struct kvmppc_vcpu_e500 {
> @@ -49,6 +53,20 @@ struct kvmppc_vcpu_e500 {
> unsigned int gtlb_size[E500_TLB_NUM];
> unsigned int gtlb_nv[E500_TLB_NUM];
>
> + /*
> + * information associated with each host TLB entry --
> + * TLB1 only for now. If/when guest TLB1 entries can be
> + * mapped with host TLB0, this will be used for that too.
> + *
> + * We don't want to use this for guest TLB0 because then we'd
> + * have the overhead of doing the translation again even if
> + * the entry is still in the guest TLB (e.g. we swapped out
> + * and back, and our host TLB entries got evicted).
> + */
> + struct tlbe_ref *tlb_refs[E500_TLB_NUM];
> +
> + unsigned int host_tlb1_nv;
> +
> u32 host_pid[E500_PID_NUM];
> u32 pid[E500_PID_NUM];
> u32 svr;
> diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
> index 3ea0f9a..4c30de3 100644
> --- a/arch/powerpc/include/asm/mmu-book3e.h
> +++ b/arch/powerpc/include/asm/mmu-book3e.h
> @@ -165,6 +165,7 @@
> #define TLBnCFG_MAXSIZE 0x000f0000 /* Maximum Page Size (v1.0) */
> #define TLBnCFG_MAXSIZE_SHIFT 16
> #define TLBnCFG_ASSOC 0xff000000 /* Associativity */
> +#define TLBnCFG_ASSOC_SHIFT 24
>
> /* TLBnPS encoding */
> #define TLBnPS_4K 0x00000004
> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> index a872732..526170f 100644
> --- a/arch/powerpc/kvm/e500_tlb.c
> +++ b/arch/powerpc/kvm/e500_tlb.c
> @@ -12,6 +12,7 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/kernel.h>
> #include <linux/types.h>
> #include <linux/slab.h>
> #include <linux/string.h>
> @@ -26,7 +27,7 @@
> #include "trace.h"
> #include "timing.h"
>
> -#define to_htlb1_esel(esel) (tlb1_entry_num - (esel) - 1)
> +#define to_htlb1_esel(esel) (tlb_host_entries[1] - (esel) - 1)
>
> struct id {
> unsigned long val;
> @@ -63,7 +64,9 @@ static DEFINE_PER_CPU(struct pcpu_id_table, pcpu_sids);
> * The valid range of shadow ID is [1..255] */
> static DEFINE_PER_CPU(unsigned long, pcpu_last_used_sid);
>
> -static unsigned int tlb1_entry_num;
> +static unsigned int tlb_host_entries[2];
> +static unsigned int tlb_host_ways[2];
> +static unsigned int tlb_host_sets[2];
The 2 probably means "number of host TLBs", right? Isn't there a #define for this? In fact, it might be even more readable if we just shoved those into a struct:
tlb_host_entries[1] would then become host_tlb[1].entries for example :). If you don't think it'd improve the code, I'd be fine without a struct too.
Also, since you're putting knowledge about the target MMU into this anyways (2 TLBs), can we prepopulate the contents of these and declare them const? The compiler should then be able to optimize the code a bit more.
In fact, reading the code a bit more, we could use the same information for the guest side, no? So we could have a host_tlb and a guest_tlb info struct and fold some of your helper functions below to merely accessing one of the two globals.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
