linux-kernel - Re: [PATCH] KVM: x86/mmu: Embed direct bits into gpa for KVM_PRE_FAULT

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] [day] [month] [year] [list]

Message-ID: <c3cfd230eab895b186ce2bcbf404dc86f9874e15.camel@intel.com>
Date: Fri, 13 Jun 2025 16:12:17 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "seanjc@...gle.com" <seanjc@...gle.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Huang,
 Kai" <kai.huang@...el.com>, "binbin.wu@...ux.intel.com"
	<binbin.wu@...ux.intel.com>, "Li, Xiaoyao" <xiaoyao.li@...el.com>, "Chatre,
 Reinette" <reinette.chatre@...el.com>, "Hunter, Adrian"
	<adrian.hunter@...el.com>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"Zhao, Yan Y" <yan.y.zhao@...el.com>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH] KVM: x86/mmu: Embed direct bits into gpa for
 KVM_PRE_FAULT_MEMORY

On Thu, 2025-06-12 at 17:09 -0700, Sean Christopherson wrote:
> A few reasons.
> 
>  a. The error_code is used in other paths, e.g. see the PFERR_IMPLICIT_ACCESS
>     usage in emulator_write_std(), and the @access parameter from FNAME(gva_to_gpa)
>     to FNAME(walk_addr_generic) (which is why FNAME(walk_addr) takes a sanitized
>     "access", a.k.a. error code, instead of e.g. kvm_page_fault.

It makes sense to not have to re-constitute it.

>  b. Keeping the entire error code allowed adding kvm_page_fault without having
>     to churn *everything*.
>  c. Preserving the entire error code simplifies the handoff to async #PF.
>  d. Unpacking error_code into bools makes downstream code much cleaner, e.g.
>     page_fault_can_be_fast() is a good example.
>  e. Waiting until kvm_mmu_do_page_fault() to fill kvm_page_fault deduplicates a
>     _lot_ of boilerplate, and allows for many fields to be "const".
>  f. I really, really want to make (most of) kvm_page_fault a structure that's
>     common to all architectures, at which point tracking e.g. exec, read, write,
>     etc. using bool is pretty much the only sane option.

Aha on (f)!

It still seems a bit unfortunate to me, but thanks for sharing the practical
reasons. You kind of have to know to try to read the bools over error code (like
Yan pointed out in this case) to keep it from looking like reading it one way or
the other means something.