[bug report] KVM: arm64: Relax invariance of KVM_ARM_VCPU_POWER_OFF

[bug report] KVM: arm64: Relax invariance of KVM_ARM_VCPU_POWER_OFF

[Dan Carpenter]

Hello Oliver Upton,

The patch e3c1c0cae31e: "KVM: arm64: Relax invariance of
KVM_ARM_VCPU_POWER_OFF" from Jun 9, 2023, leads to the following
Smatch static checker warning:

	arch/arm64/kvm/arm.c:1283 kvm_arch_vcpu_ioctl_vcpu_init()
	warn: bitwise AND condition is false here

arch/arm64/kvm/arm.c
    1272 static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
    1273                                          struct kvm_vcpu_init *init)
    1274 {
    1275         bool power_off = false;
    1276         int ret;
    1277 
    1278         /*
    1279          * Treat the power-off vCPU feature as ephemeral. Clear the bit to avoid
    1280          * reflecting it in the finalized feature set, thus limiting its scope
    1281          * to a single KVM_ARM_VCPU_INIT call.
    1282          */
--> 1283         if (init->features[0] & KVM_ARM_VCPU_POWER_OFF) {

Prior to this commit KVM_ARM_VCPU_POWER_OFF was a bit number.  I think
it still should be.  As in:

#define KVM_ARM_VCPU_POWER_OFF BIT(0)

    1284                 init->features[0] &= ~KVM_ARM_VCPU_POWER_OFF;
    1285                 power_off = true;
    1286         }
    1287 
    1288         ret = kvm_vcpu_set_target(vcpu, init);
    1289         if (ret)
    1290                 return ret;
    1291 

regards,
dan carpenter

Re: [bug report] KVM: arm64: Relax invariance of KVM_ARM_VCPU_POWER_OFF

[Oliver Upton]

Hey Dan,

On Thu, Jun 22, 2023 at 03:49:18PM +0300, Dan Carpenter wrote:
> Hello Oliver Upton,
> 
> The patch e3c1c0cae31e: "KVM: arm64: Relax invariance of
> KVM_ARM_VCPU_POWER_OFF" from Jun 9, 2023, leads to the following
> Smatch static checker warning:
> 
> 	arch/arm64/kvm/arm.c:1283 kvm_arch_vcpu_ioctl_vcpu_init()
> 	warn: bitwise AND condition is false here
> 
> arch/arm64/kvm/arm.c
>     1272 static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
>     1273                                          struct kvm_vcpu_init *init)
>     1274 {
>     1275         bool power_off = false;
>     1276         int ret;
>     1277 
>     1278         /*
>     1279          * Treat the power-off vCPU feature as ephemeral. Clear the bit to avoid
>     1280          * reflecting it in the finalized feature set, thus limiting its scope
>     1281          * to a single KVM_ARM_VCPU_INIT call.
>     1282          */
> --> 1283         if (init->features[0] & KVM_ARM_VCPU_POWER_OFF) {
> 
> Prior to this commit KVM_ARM_VCPU_POWER_OFF was a bit number.  I think
> it still should be.  As in:
> 
> #define KVM_ARM_VCPU_POWER_OFF BIT(0)
> 
>     1284                 init->features[0] &= ~KVM_ARM_VCPU_POWER_OFF;
>     1285                 power_off = true;
>     1286         }
>     1287 
>     1288         ret = kvm_vcpu_set_target(vcpu, init);
>     1289         if (ret)
>     1290                 return ret;
>     1291 

Thanks for the bug report. I had to go find my brown paper bag, but the
fix is now queued up for 6.5:

https://lore.kernel.org/kvmarm/20230622160922.1925530-1-oliver.upton@linux.dev/

-- 
Thanks,
Oliver