On Thu, May 30, 2024 at 11:07 PM Rick Edgecombe
<rick.p.edgecombe@xxxxxxxxx> wrote:
> +enum kvm_tdp_mmu_root_types {
> + KVM_VALID_ROOTS = BIT(0),
> +
> + KVM_ANY_ROOTS = 0,
> + KVM_ANY_VALID_ROOTS = KVM_VALID_ROOTS,
I would instead define it as
KVM_INVALID_ROOTS = BIT(0),
KVM_VALID_ROOTS = BIT(1),
KVM_ALL_ROOTS = KVM_VALID_ROOTS | KVM_INVALID_ROOTS,
and then
if (root->role.invalid)
return types & KVM_INVALID_ROOTS;
else
return types & KVM_VALID_ROOTS;
Then in the next patch you can do
KVM_INVALID_ROOTS = BIT(0),
- KVM_VALID_ROOTS = BIT(1),
+ KVM_DIRECT_ROOTS = BIT(1),
+ KVM_MIRROR_ROOTS = BIT(2),
+ KVM_VALID_ROOTS = KVM_DIRECT_ROOTS | KVM_MIRROR_ROOTS,
KVM_ALL_ROOTS = KVM_VALID_ROOTS | KVM_INVALID_ROOTS,
and likewise
if (root->role.invalid)
return types & KVM_INVALID_ROOTS;
else if (likely(!is_mirror_sp(root)))
return types & KVM_DIRECT_ROOTS;
else
return types & KVM_MIRROR_ROOTS;
This removes the need for KVM_ANY_VALID_ROOTS (btw I don't know if
it's me, but ALL sounds more grammatical than ANY in this context). So
the resulting code should be a bit clearer.
Apart from this small tweak, the overall idea is really good.
Paolo
> +};
> +
> bool kvm_tdp_mmu_zap_leafs(struct kvm *kvm, gfn_t start, gfn_t end, bool flush);
> bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp);
> void kvm_tdp_mmu_zap_all(struct kvm *kvm);
> --
> 2.34.1
>
