| Message ID | 20160404210928.0d9ae644@kryten (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
> Am 04.04.2016 um 13:09 schrieb Anton Blanchard <anton@samba.org>: > > We don't support transactional memory in PR KVM, so don't tell > the OS that we do. > > Signed-off-by: Anton Blanchard <anton@samba.org> > --- > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index e7be21e..538bd87 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -696,6 +696,12 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, > } else /* env->mmu_model == POWERPC_MMU_2_07 */ { > pa_features = pa_features_207; > pa_size = sizeof(pa_features_207); > + > + /* Don't enable TM in PR KVM mode */ > + if (kvm_enabled() && > + kvm_vm_check_extension(cs->kvm_state, KVM_CAP_PPC_GET_PVINFO)) Does this compile on non-kvm systems? Alex > { > + pa_features[24] &= ~0x80; > + } > } > if (env->ci_large_pages) { > pa_features[3] |= 0x20;
On Mon, Apr 04, 2016 at 09:09:28PM +1000, Anton Blanchard wrote: > We don't support transactional memory in PR KVM, so don't tell > the OS that we do. This assumes PR KVM won't ever support TM, which is hopefully not true. If PR KVM does get TM support in future, then QEMU will have no clear way to know whether it needs to clear the pa-features bit or not. I think we need to define some way for the KVM implementation to tell qemu which of these kinds of CPU features it supports. However, we could defer implementing that mechanism until PR KVM does get support for TM, I guess. In that case this patch could go in now, though it seems slightly icky to be using the pvinfo stuff to distinguish PR from HV. Paul.
On Tue, Apr 05, 2016 at 12:12:01PM +1000, Paul Mackerras wrote: > On Mon, Apr 04, 2016 at 09:09:28PM +1000, Anton Blanchard wrote: > > We don't support transactional memory in PR KVM, so don't tell > > the OS that we do. > > This assumes PR KVM won't ever support TM, which is hopefully not > true. If PR KVM does get TM support in future, then QEMU will have no > clear way to know whether it needs to clear the pa-features bit or > not. I think we need to define some way for the KVM implementation to > tell qemu which of these kinds of CPU features it supports. Yeah, I think we need some sort of capability flag for this. We also need to isolate this KVM capability testing better into the KVM code, so we won't break things on TCG. Speaking of which... I don't imagine we implement TM instructions in TCG either, so we should probably make sure TM isn't advertised there either. > However, we could defer implementing that mechanism until PR KVM does > get support for TM, I guess. In that case this patch could go in now, > though it seems slightly icky to be using the pvinfo stuff to > distinguish PR from HV. It is icky, but it's the best idiom we have for distinguishing PR KVM. We try to only do it as a fallback when we can't determine availability of the specific feature based on a capability, though.
On 04/05/2016 02:09 PM, David Gibson wrote: > On Tue, Apr 05, 2016 at 12:12:01PM +1000, Paul Mackerras wrote: >> On Mon, Apr 04, 2016 at 09:09:28PM +1000, Anton Blanchard wrote: >>> We don't support transactional memory in PR KVM, so don't tell >>> the OS that we do. >> >> This assumes PR KVM won't ever support TM, which is hopefully not >> true. If PR KVM does get TM support in future, then QEMU will have no >> clear way to know whether it needs to clear the pa-features bit or >> not. I think we need to define some way for the KVM implementation to >> tell qemu which of these kinds of CPU features it supports. > > Yeah, I think we need some sort of capability flag for this. We also > need to isolate this KVM capability testing better into the KVM code, > so we won't break things on TCG. > > Speaking of which... I don't imagine we implement TM instructions in > TCG either, so we should probably make sure TM isn't advertised there > either. TM is "supported" in TCG: 56a846157 "target-ppc: Introduce TM Noops" === Add degenerate implementations of the non-privileged Transactional Memory instructions tend., tabort*. and tsr. This implementation simply checks the MSR[TM] bit and then sets CR0 to 0b0000. This is a reasonable degenerate implementation since transactions are never allowed to begin and hence MSR[TS] is always 0b00. Signed-off-by: Tom Musta <tommusta@gmail.com> Signed-off-by: Alexander Graf <agraf@suse.de> ===
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e7be21e..538bd87 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -696,6 +696,12 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, } else /* env->mmu_model == POWERPC_MMU_2_07 */ { pa_features = pa_features_207; pa_size = sizeof(pa_features_207); + + /* Don't enable TM in PR KVM mode */ + if (kvm_enabled() && + kvm_vm_check_extension(cs->kvm_state, KVM_CAP_PPC_GET_PVINFO)) { + pa_features[24] &= ~0x80; + } } if (env->ci_large_pages) { pa_features[3] |= 0x20;
We don't support transactional memory in PR KVM, so don't tell the OS that we do. Signed-off-by: Anton Blanchard <anton@samba.org> ---