Re: [Qemu-devel] [PATCH 3/3] ppc/kvm: check some capabilities with kvm_vm_check_extension()

[Thomas Huth <thuth@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]

On 15.09.2017 10:43, Greg Kurz wrote:
> On Fri, 15 Sep 2017 07:15:41 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> 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?
>>
> 
> Heh you're right :)
> 
> The only user for cap_htab_fd is kvmppc_get_htab_fd(), in order to provide a
> sensible error message that KVM doesn't allow saving HPTEs (ie, migration isn't
> supported with some older KVM HV). It doesn't need kvmppc_has_cap_htab_fd() for
> that since all the code sits in target/ppc/kvm.c.
> 
> I can't think of any other use, so I guess we can drop it.

OK. If you've got some spare minutes, could you maybe send a patch?

[...]
>>> @@ -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?
> 
> It already does :)
> 
> commit a8acaece5d88db234d0b82b8692dea15d602f622

Oh, right, seems like I was looking at an older kernel branch :-/ So
never mind, we're fine here.

 Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]