[PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE

[Cédric Le Goater]

On 2/25/19 5:35 AM, Paul Mackerras wrote:
> On Fri, Feb 22, 2019 at 12:28:27PM +0100, Cédric Le Goater wrote:
>> The user interface exposes a new capability to let QEMU connect the
>> vCPU to the XIVE KVM device if required. The capability is only
>> advertised on a PowerNV Hypervisor as support for nested guests
>> (pseries KVM Hypervisor) is not yet available.
> 
> If a bisection happened to land on this commit, we would have KVM
> saying it had the ability to support guests using XIVE natively, but
> it wouldn't actually work since we don't have all the code that is in
> the following patches.

OK. I didn't think migration was a must-have for bisection. I will move
the enablement at end.

> Thus, in order to avoid breaking bisection, you should either add the
> capability now but have it always return false until the rest of the
> code is in place, or else defer the addition of the capability until
> the end of the patch series.

I will introduce the capability early in the patchset and return false
as you are proposing. It seems to be the best approach.

>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 8c69af10f91d..a38a643a24dd 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -570,6 +570,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_PPC_GET_CPU_CHAR:
>>  		r = 1;
>>  		break;
>> +#ifdef CONFIG_KVM_XIVE
>> +	case KVM_CAP_PPC_IRQ_XIVE:
>> +		/* only for PowerNV */
>> +		r = !!cpu_has_feature(CPU_FTR_HVMODE);
> 
> Shouldn't this be r = xive_enabled() && !!cpu_has_feature(CPU_FTR_HVMODE)

yes. we need the '__xive_enabled' toggle to be set also :/ 

It can set to off with the "xive=off" on the command line and on old P9 
skiboot. That could be simplified one day.

> (or alternatively r = xics_on_xive(), though that would be confusing
> to the reader)?

This is correct. I didn't want to use the xics_on_xive() which is not
the capability we are activating. 

I will keep the open-coded version.

> As it stands this would report true on POWER8, unless I'm missing
> something.

Ah yes. I forgot this combination also. 

This should not be too complex to fix.

Thanks,

C.