linux-kernel - Re: [PATCH v2 16/25] KVM: x86/mmu: rename kvm_mmu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YierXBEvTdp5aw+u@google.com>
Date:   Tue, 8 Mar 2022 19:15:40 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        dmatlack@...gle.com
Subject: Re: [PATCH v2 16/25] KVM: x86/mmu: rename kvm_mmu_role union

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;
}