On Mon, Feb 21, 2022, Paolo Bonzini wrote:
> It is quite confusing that the "full" union is called kvm_mmu_role
> but is used for the "cpu_mode" field of struct kvm_mmu. Rename it
> to kvm_mmu_paging_mode.
My official vote is to use
union kvm_mmu_cpu_role cpu_role
instead of
union kvm_mmu_paging_mode cpu_mode
The latter has too many inconsistencies, e.g. helpers using "role" instead of "mode",
the union using "paging" but the field/params using "cpu". The cpu instead of paging
thing isn't a coincidence as having the param be "paging_mode" would be really, really
confusing (ditto for kvm_calc_paging_mode()).
IMO this is consistent, if imperfect.
static union kvm_mmu_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
const struct kvm_mmu_role_regs *regs)
{
union kvm_mmu_cpu_role role = {0};
role.base.access = ACC_ALL;
role.base.smm = is_smm(vcpu);
role.base.guest_mode = is_guest_mode(vcpu);
role.base.direct = !____is_cr0_pg(regs);
role.ext.valid = 1;
if (!____is_cr0_pg(regs))
return role;
...
return role;
}
Whereas this is inconsistent, and also imperfect.
static union kvm_mmu_paging_mode kvm_calc_cpu_mode(struct kvm_vcpu *vcpu,
const struct kvm_mmu_role_regs *regs)
{
union kvm_mmu_paging_mode role = {0};
role.base.access = ACC_ALL;
role.base.smm = is_smm(vcpu);
role.base.guest_mode = is_guest_mode(vcpu);
role.base.direct = !____is_cr0_pg(regs);
role.ext.valid = 1;
if (!____is_cr0_pg(regs))
return role;
...
return role;
}
