On 14.09.2017 21:25, Greg Kurz wrote: > The following capabilities are VM specific: > - KVM_CAP_PPC_SMT_POSSIBLE > - KVM_CAP_PPC_HTAB_FD
BTW, looks like kvmppc_has_cap_htab_fd() is dead code ... should we either remove it or check it somewhere? > - KVM_CAP_PPC_ALLOC_HTAB > > If both KVM HV and KVM PR are present, checking them always return > the HV value, even if we explicitely requested to use PR. > > This has no visible effect for KVM_CAP_PPC_ALLOC_HTAB, because we also > try the KVM_PPC_ALLOCATE_HTAB ioctl which is only suppored by HV. As > a consequence, the spapr code doesn't even check KVM_CAP_PPC_HTAB_FD. > > However, this will cause kvmppc_hint_smt_possible(), introduced by > commit fa98fbfcdfcb9, to report several VSMT modes (eg, Available > VSMT modes: 8 4 2 1) whereas PR only support mode 1. > > This patch fixes all three anyway to use kvm_vm_check_extension(). It > is okay since the VM is already created at the time kvm_arch_init() or > kvmppc_reset_htab() is called. > > Signed-off-by: Greg Kurz <gr...@kaod.org> > --- > target/ppc/kvm.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 1deaf106d2b9..208c70e81426 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -131,7 +131,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > cap_interrupt_level = kvm_check_extension(s, KVM_CAP_PPC_IRQ_LEVEL); > cap_segstate = kvm_check_extension(s, KVM_CAP_PPC_SEGSTATE); > cap_booke_sregs = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_SREGS); > - cap_ppc_smt_possible = kvm_check_extension(s, KVM_CAP_PPC_SMT_POSSIBLE); > + cap_ppc_smt_possible = kvm_vm_check_extension(s, > KVM_CAP_PPC_SMT_POSSIBLE); > cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA); > cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE); > cap_spapr_tce_64 = kvm_check_extension(s, KVM_CAP_SPAPR_TCE_64); > @@ -143,7 +143,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG); > /* Note: we don't set cap_papr here, because this capability is > * only activated after this by kvmppc_set_papr() */ > - cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); > + cap_htab_fd = kvm_vm_check_extension(s, KVM_CAP_PPC_HTAB_FD); > cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); > cap_ppc_smt = kvm_vm_check_extension(s, KVM_CAP_PPC_SMT); > cap_htm = kvm_vm_check_extension(s, KVM_CAP_PPC_HTM); > @@ -2353,7 +2353,7 @@ int kvmppc_reset_htab(int shift_hint) > /* Full emulation, tell caller to allocate htab itself */ > return 0; > } > - if (kvm_check_extension(kvm_state, KVM_CAP_PPC_ALLOC_HTAB)) { > + if (kvm_vm_check_extension(kvm_state, KVM_CAP_PPC_ALLOC_HTAB)) { > int ret; > ret = kvm_vm_ioctl(kvm_state, KVM_PPC_ALLOCATE_HTAB, &shift); > if (ret == -ENOTTY) { Looking at the comment in the code after the "if (ret == -ENOTTY)" line, it sounds like there is a bug in the kernel and the KVM_CAP_PPC_ALLOC_HTAB should depend on the hv_enabled variable, too? Anyway, that's another topic, your patch is fine! Reviewed-by: Thomas Huth <th...@redhat.com>