| Message ID | 20110617203929.GA16894@schlenkerla.am.freescale.net |
|---|---|
| State | New |
| Headers | show |
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 <scottwood@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
On 06/17/2011 04:28 PM, Alexander Graf wrote: >> > + 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? Yes and yes. r~
On 18.06.2011, at 18:13, Richard Henderson wrote: > On 06/17/2011 04:28 PM, Alexander Graf wrote: >>>> + 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? > > Yes and yes. Ah, very nice. I wonder why I don't see it used more in code then :). Seems to be very handy to not clutter code with memset(0)s. Alex
On 2011-06-18 01:28, Alexander Graf wrote: > > 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 <scottwood@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? I can only forward this question: Avi, what are the plans for http://thread.gmane.org/gmane.comp.emulators.kvm.devel/73917? Jan
On 06/20/2011 10:41 AM, Jan Kiszka wrote: > > > Those hopefully shouldn't be required anymore soon - when Jan's patches make it upstream. Jan, how's progress on that front? > > I can only forward this question: Avi, what are the plans for > http://thread.gmane.org/gmane.comp.emulators.kvm.devel/73917? Will apply once all comments are addressed.
On 2011-06-20 10:03, Avi Kivity wrote: > On 06/20/2011 10:41 AM, Jan Kiszka wrote: >> >>> Those hopefully shouldn't be required anymore soon - when Jan's patches make it upstream. Jan, how's progress on that front? >> >> I can only forward this question: Avi, what are the plans for >> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/73917? > > Will apply once all comments are addressed. Well, then go ahead :) - or did I miss a comment? Jan
On 06/20/2011 11:47 AM, Jan Kiszka wrote: > On 2011-06-20 10:03, Avi Kivity wrote: > > On 06/20/2011 10:41 AM, Jan Kiszka wrote: > >> > >>> Those hopefully shouldn't be required anymore soon - when Jan's patches make it upstream. Jan, how's progress on that front? > >> > >> I can only forward this question: Avi, what are the plans for > >> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/73917? > > > > Will apply once all comments are addressed. > > Well, then go ahead :) - or did I miss a comment? If everyone's happy I (or rather Marcelo this week) will be happy to apply.
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) + struct kvm_book3e_206_tlb_params params = {}; + 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++) { + 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__); + } + + 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); + 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) + 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; + + ret = kvm_vcpu_ioctl(env, KVM_DIRTY_TLB, &dirty_tlb); + if (ret) { + fprintf(stderr, "%s: KVM_DIRTY_TLB: %s\n", + __func__, strerror(-ret)); + } + + qemu_free(bitmap); +#endif +} + int kvm_arch_put_registers(CPUState *env, int level) { struct kvm_regs regs; @@ -177,6 +257,11 @@ int kvm_arch_put_registers(CPUState *env, int level) if (ret < 0) return ret; + if (env->tlb_dirty) { + kvm_sw_tlb_put(env); + env->tlb_dirty = false; + } + return ret; }
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 <scottwood@freescale.com> --- hw/ppce500_mpc8544ds.c | 2 + target-ppc/cpu.h | 2 + target-ppc/kvm.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 0 deletions(-)