search

rust-vmm / kvm-ioctls

Apache License 2.0
255 stars 103 forks source link

Assert failing when `MIDR_EL1` is `0x413fd0c1` #265

Closed MatiasVara closed 1 month ago

MatiasVara commented 1 month ago

The assert at https://github.com/rust-vmm/kvm-ioctls/blob/0ddf7ef6f6079bf217cb297278b7ba35e3e70116/src/ioctls/vcpu.rs#L2732 is supposing to fail when setting target to 0. However, when MIDR_EL1 is 0x413fd0c1, the test does not fail since 0 corresponds to KVM_ARM_TARGET_AEM_V8, which is a valid target. If the test is supposed to always fail, shall not the initial value be replaced with an invalid value like KVM_ARM_NUM_TARGETS? (see arch/arm64/kvm/guest.c::kvm_target_cpu())

Thanks.

ShadowCurse commented 1 month ago

Hi, As I can see the idea of this assert it to demonstrate that vcpu_init will fail until kvi has values set by get_preferred_target. But as you correctly pointed out, zeroed kvm_vcpu_init can be valid if the host cpu of the machine this test is run on has KVM_ARM_TARGET_AEM_V8 target. I think this assert should be moved from test_get_preferred_target test into separate test for vcpu_init where we can do a test with known incorrect value like KVM_ARM_NUM_TARGETS.

MatiasVara commented 1 month ago

Hi, As I can see the idea of this assert it to demonstrate that vcpu_init will fail until kvi has values set by get_preferred_target. But as you correctly pointed out, zeroed kvm_vcpu_init can be valid if the host cpu of the machine this test is run on has KVM_ARM_TARGET_AEM_V8 target. I think this assert should be moved from test_get_preferred_target test into separate test for vcpu_init where we can do a test with known incorrect value like KVM_ARM_NUM_TARGETS.

Do you mean a single test to test a known incorrect value?

ShadowCurse commented 1 month ago

Yes, I think this will be a correct option here. At least it will follow the overall testing pattern in this crate.