On 17.06.2011, at 22:39, Scott Wood wrote:
> Share the TLB array with KVM. This allows us to set the initial TLB
> both on initial boot and reset, is useful for debugging, and could
> eventually be used to support migration.
>
> Signed-off-by: Scott Wood <scottw...@freescale.com>
> ---
> hw/ppce500_mpc8544ds.c | 2 +
> target-ppc/cpu.h | 2 +
> target-ppc/kvm.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 89 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> index 5ac8843..3cdeb43 100644
> --- a/hw/ppce500_mpc8544ds.c
> +++ b/hw/ppce500_mpc8544ds.c
> @@ -192,6 +192,8 @@ static void mmubooke_create_initial_mapping(CPUState *env,
> tlb->mas2 = va & TARGET_PAGE_MASK;
> tlb->mas7_3 = pa & TARGET_PAGE_MASK;
> tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
> +
> + env->tlb_dirty = true;
> }
>
> static void mpc8544ds_cpu_reset(void *opaque)
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 46d86be..8191ed2 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -921,6 +921,8 @@ struct CPUPPCState {
> ppc_tlb_t tlb; /* TLB is optional. Allocate them only if needed
> */
> /* 403 dedicated access protection registers */
> target_ulong pb[4];
> + bool tlb_dirty; /* Set to non-zero when modifying TLB
> */
> + bool kvm_sw_tlb; /* non-zero if KVM SW TLB API is active
> */
> #endif
>
> /* Other registers */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index e7b1b10..9a88fc9 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -122,6 +122,51 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
> return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs);
> }
>
> +static int kvm_booke206_tlb_init(CPUState *env)
> +{
> +#if defined(KVM_CAP_SW_TLB) && defined(KVM_MMU_FSL_BOOKE_NOHV)
Those hopefully shouldn't be required anymore soon - when Jan's patches make it
upstream. Jan, how's progress on that front?
> + struct kvm_book3e_206_tlb_params params = {};
Hrm - I'm not familiar with that initialization. What exactly does it do? Set
the struct contents to 0? Is this properly standardized? Usually, I see
memset(0)s for that.
> + struct kvm_config_tlb cfg = {};
> + size_t array_len;
> + unsigned int entries = 0;
> + int ret, i;
> +
> + if (!kvm_enabled() ||
> + !kvm_check_extension(env->kvm_state, KVM_CAP_SW_TLB)) {
> + return 0;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(params.tlb_sizes); i++) {
Please make this MAX(..., BOOKE206_MAX_TLBN) - I'd hope the compiler is clever
enough to figure out we're dealing with a constant here and that way the code
looks more secure (even though it's the same in practice).
Alternatively, you could just do an assert(... == BOOKE206_MAX_TLBN); before.
> + params.tlb_sizes[i] = booke206_tlb_size(env, i);
> + params.tlb_ways[i] = booke206_tlb_ways(env, i);
> + entries += params.tlb_sizes[i];
> + }
> +
> + if (entries != env->nb_tlb) {
> + cpu_abort(env, "%s: nb_tlb mismatch\n", __func__);
> + }
> +
assert(sizeof(struct kvm_book3e_206_tlb_entry) == sizeof(ppcmas_tlb_t));
> + array_len = sizeof(struct kvm_book3e_206_tlb_entry) * entries;
> + env->tlb_dirty = true;
> +
> + cfg.array = (uintptr_t)env->tlb.tlbm;
> + cfg.array_len = sizeof(ppcmas_tlb_t) * entries;
> + cfg.params = (uintptr_t)¶ms;
> + cfg.mmu_type = KVM_MMU_FSL_BOOKE_NOHV;
> +
> + ret = kvm_vcpu_ioctl(env, KVM_CONFIG_TLB, &cfg);
> + if (ret < 0) {
> + fprintf(stderr, "%s: couldn't KVM_CONFIG_TLB: %s\n",
> + __func__, strerror(-ret));
> + return ret;
> + }
> +
> + env->kvm_sw_tlb = true;
> +#endif
> +
> + return 0;
> +}
> +
> int kvm_arch_init_vcpu(CPUState *cenv)
> {
> int ret;
> @@ -133,6 +178,14 @@ int kvm_arch_init_vcpu(CPUState *cenv)
>
> idle_timer = qemu_new_timer_ns(vm_clock, kvm_kick_env, cenv);
>
Please add a comment here, explaining the occasional reader what we're doing
here
> + switch (cenv->mmu_model) {
> + case POWERPC_MMU_BOOKE206:
> + ret = kvm_booke206_tlb_init(cenv);
> + break;
> + default:
> + break;
> + }
> +
> return ret;
> }
>
> @@ -140,6 +193,33 @@ void kvm_arch_reset_vcpu(CPUState *env)
> {
> }
>
> +static void kvm_sw_tlb_put(CPUState *env)
> +{
> +#if defined(KVM_CAP_SW_TLB)
See above
> + struct kvm_dirty_tlb dirty_tlb;
> + unsigned char *bitmap;
> + int ret;
> +
> + if (!env->kvm_sw_tlb) {
> + return;
> + }
> +
> + bitmap = qemu_malloc((env->nb_tlb + 7) / 8);
> + memset(bitmap, 0xFF, (env->nb_tlb + 7) / 8);
> +
> + dirty_tlb.bitmap = (uintptr_t)bitmap;
> + dirty_tlb.num_dirty = env->nb_tlb;
Pretty simple for now, but I like the idea of starting simple :). Would it make
sense to keep the bitmap allocated throughout the lifetime of env?
Alex
