Linux Confidential Computing Development
 help / color / mirror / Atom feed
* Re: [PATCH v8 06/46] KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined
From: Xiaoyao Li @ 2026-07-01  9:19 UTC (permalink / raw)
  To: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
	Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
	Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
	Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
  Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
	linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-6-9d2959357853@google.com>

On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based
> on kvm_arch_has_private_mem being #defined in anticipation of decoupling
> kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES.

Well, after this series, kvm_supported_mem_attributes() is renamed to 
kvm_supported_vm_mem_attributes(), and it's still under 
CONFIG_KVM_VM_MEMORY_ATTRIBUTES.

> guest_memfd support for memory attributes will be unconditional to avoid
> yet more macros (all architectures that support guest_memfd are expected to
> use per-gmem attributes at some point), at which point enumerating support
> KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being
> supported _somewhere_ would result in KVM over-reporting support on arm64.

I don't understand it. This patch only changes the behavior of 
kvm_supported_mem_attributes(), the usage of which is guarded by 
CONFIG_KVM_VM_MEMORY_ATTRIBUTES. This is config is only visible to x86 
due to patch 03. How does it affect arm64?

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>   virt/kvm/kvm_main.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1ccc4895a4c26..7b989b659cf82 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2421,8 +2421,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
>   #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
>   static u64 kvm_supported_mem_attributes(struct kvm *kvm)
>   {
> +#ifdef kvm_arch_has_private_mem
>   	if (!kvm || kvm_arch_has_private_mem(kvm))
>   		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +#endif
>   
>   	return 0;
>   }
> 


^ permalink raw reply

* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Kiryl Shutsemau @ 2026-07-01  9:22 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco,
	linux-kernel, Sashiko Bot, Joerg Roedel, Yan Zhao, Ackerley Tng
In-Reply-To: <20260630213711.479692-3-seanjc@google.com>

On Tue, Jun 30, 2026 at 02:37:11PM -0700, Sean Christopherson wrote:
> Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
> pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
> consistent between TDX and SNP (for LAUNCH_UPDATE).  EOPNOTSUPP was chosen
> to be a forward-looking error code for when guest_memfd supports in-place
> conversion, but even when in-place conversion comes along, it's an awkward
> error code as KVM is deliberately choosing to disallow virtual address '0',
> which is technically a legal userspace address.  I.e. it's not so much a
> lack of support as it is that KVM reserves address '0' to simplify KVM's
> internal implementation.
> 
> Opportunistically move the check so that it's co-located with the other
> checks on the userspace address, and so that it's more obvious that a NULL
> source address is explicitly disallowed.
> 
> Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Acked-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Jethro Beekman @ 2026-07-01  9:40 UTC (permalink / raw)
  To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
	peterz, thomas.lendacky, herbert, davem, ardb
  Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
	Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
	xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
	darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <205a5259f9fd353dc0ca6b00565c8175a96768c7.1782841284.git.ashish.kalra@amd.com>

[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]

Hi Ashish,

I don't believe my concern has been addressed

https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/

--
Jethro Beekman | CTO | Fortanix

On 2026-06-30 20:11, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> While SNP is active, every memory write is checked against the RMP to
> protect SEV-SNP guest memory.  A core performs these RMP checks only once
> SNP has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is
> set on that core; the firmware requires the SNP-enable bit to be set on
> every present CPU before SNP initialization.  A core that is not
> SNP-enabled and not SNP-initialized performs no RMP checks at all, so
> there is no valid configuration with SNP active and any CPU exempt from
> RMP checks.
> 
> The firmware determines which CPUs are present from the processor and the
> BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates
> them at SNP init; it is not aware of the OS bringing CPUs online or
> offline afterwards.  SNP_INIT fails unless SnpEn is set on all CPUs, so a
> CPU that is offline at SNP init does not have SnpEn set, SNP_INIT fails,
> and there can be no SNP guest memory.  OS CPU hotplug can thus diverge
> from the firmware's expectations and break SNP.
> 
> Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
> SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
> disabled SNP.  If snp_prepare() fails before enabling SNP it re-enables
> hotplug itself; once SNP is enabled hotplug stays disabled, including
> across a failed SNP_INIT and across the legacy SNP_SHUTDOWN_EX path, both
> of which leave SNP enabled.  A kexec target that boots with SNP already
> enabled disables hotplug once in snp_rmptable_init(), since snp_prepare()
> bails when SNP is already enabled.
> 
> This also keeps the CPU set stable for the asynchronous RMPOPT scan added
> later in this series, and ensures cpus_read_lock() in the scan is
> uncontended.
> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index dab6e1c290bc..04a58ac4339c 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -535,6 +535,15 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> +	/*
> +	 * Disable CPU hotplug before enabling SNP, so no CPU can come online
> +	 * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
> +	 * once SNP is disabled.  Must be before cpus_read_lock():
> +	 * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
> +	 * cpu_hotplug_lock.
> +	 */
> +	cpu_hotplug_disable();
> +
>  	cpus_read_lock();
>  
>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
> @@ -560,6 +569,10 @@ int snp_prepare(void)
>  unlock:
>  	cpus_read_unlock();
>  
> +	/* Re-enable CPU hotplug; SnpEn was never set. */
> +	if (ret)
> +		cpu_hotplug_enable();
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
> @@ -587,6 +600,13 @@ void snp_shutdown(void)
>  
>  	rmpopt_cleanup();
>  
> +	/*
> +	 * Re-enable CPU hotplug now that the firmware has disabled SNP; CPU
> +	 * hotplug is not re-enabled for a legacy SNP shutdown.  After
> +	 * rmpopt_cleanup() so RMPOPT_BASE is cleared with hotplug still disabled.
> +	 */
> +	cpu_hotplug_enable();
> +
>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);
>  }
> @@ -645,6 +665,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
>   */
>  int __init snp_rmptable_init(void)
>  {
> +	u64 val;
> +
>  	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>  		return -ENOSYS;
>  
> @@ -654,6 +676,15 @@ int __init snp_rmptable_init(void)
>  	if (!setup_rmptable())
>  		return -ENOSYS;
>  
> +	/*
> +	 * On a kexec boot SNP may already be enabled (legacy firmware leaves
> +	 * SnpEn set across shutdown), in which case snp_prepare() bails without
> +	 * disabling CPU hotplug, so disable it here.
> +	 */
> +	rdmsrq(MSR_AMD64_SYSCFG, val);
> +	if (val & MSR_AMD64_SYSCFG_SNP_EN)
> +		cpu_hotplug_disable();
> +
>  	/*
>  	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>  	 * notifier is invoked to do SNP IOMMU shutdown before kdump.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4839 bytes --]

^ permalink raw reply

* Re: [PATCH v14 00/44] arm64: Support for Arm CCA in KVM
From: Steven Price @ 2026-07-01 10:53 UTC (permalink / raw)
  To: Kohei Enju
  Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
	James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
	Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <akR0eX0nCumIgnlh@FCCLS0092175.localdomain>

On 01/07/2026 03:15, Kohei Enju wrote:
> On 05/13 14:17, Steven Price wrote:
>> This series adds support for running protected VMs using KVM under the
>> Arm Confidential Compute Architecture (CCA).
>>
>> This is rebased on v7.1-rc1, but still targets RMM v2.0-bet1[1].
>>
>> The major updates from v13 remain but have been more fully implemented:
>> the RMM uses the host's page size, range based RMI APIs mean we don't
>> have to break everything down to base page sizes, the GIC state is
>> passed via system registers, and the uAPI has been simplified.
>>
>> The main changes since v13 are:
>>
>>  * The RMI definitions and wrappers have been fully updated for RMM
>>    v2.0-bet1. In particular the temporary RMM v1.0 SMC compatibility
>>    patch has been dropped.
>>
>>  * The PSCI completion ioctl has been removed. RMM v2.0-bet1 still
>>    requires the host to provide the target REC for PSCI calls which
>>    name another vCPU, but KVM now performs the RMI PSCI completion
>>    automatically before entering the REC again. Userspace no longer
>>    needs to issue KVM_ARM_VCPU_RMI_PSCI_COMPLETE. A future spec should
>>    remove the need for the host to provide the MPIDR mapping.
>>
>>  * The generic RMI init, RMM configuration, GPT setup,
>>    delegate/undelegate helpers and SRO infrastructure have moved out of
>>    KVM into arch/arm64/kernel/rmi.c. RMI is expected to be used by
>>    features outside KVM, so this code should be available even when KVM
>>    is not built.
>>
>>  * RMI_GRANULE_TRACKING_GET has been updated to work on a range, this
>>    allows it to work when the region is not aligned to the tracking
>>    size. Solves the problem reported by Mathieu[2].
>>
>>  * SRO support has been moved earlier in the series and improved. It
>>    provides a cleaner way for the host to provide the RMM with the extra
>>    memory it requires. However support is still incomplete where the
>>    TF-RMM code does not yet implement it. This is noted by FIXMEs in the
>>    code.
>>
>>  * The ARM VM type encoding has been reworked to coexist with the
>>    upstream pKVM KVM_VM_TYPE_ARM_PROTECTED bit.
>>
>>  * The private-memory documentation now notes that arm64 uses
>>    KVM_CAP_MEMORY_ATTRIBUTES.
>>
>>  * PMU support is dropped for now. It will be added later in a separate
>>    series. Similarly for selecting the hash algorithm and RPV.
> 
> Hi Steven,

Hi,

> Is there any plan to add support for selecting the MEC policy (shared or
> private)? We have been working on adding support for this on top of your
> series. If this is not already in the works, we may upstream our
> implementation later.

I've been trying to focus on getting the minimum useful series
upstreamed before looking at additional features (such as hash
algorithm, MEC policy etc). If you've already got support then yes
please do upstream it later when we've got this series landed.

Thanks,
Steve

> Thanks,
> Kohei
> 
>>
>> There are also the usual rebase updates and smaller fixes, including
>> changes to the RMM v2.0-bet1 range APIs, removal of REC auxiliary
>> granule handling, fixes to the address range descriptor encoding, and
>> cleanups around realm stage-2 teardown.
>>
>> Stateful RMI Operations
>> -----------------------
>>
>> The RMM v2.0 spec introduces Stateful RMI Operations (SROs), which allow
>> the RMM to complete an operation over several SMC calls while requesting
>> or returning memory to the host. This allows interrupts to be handled in
>> the middle of an operation and lets the RMM dynamically allocate memory
>> for internal tracking purposes. For example, RMI_REC_CREATE no longer
>> needs auxiliary granules to be provided up front, and can instead
>> request memory during the operation.
>>
>> This series includes the generic SRO infrastructure in
>> arch/arm64/kernel/rmi.c and uses it for REC create/destroy. The other
>> cases are not yet used by TF-RMM and a future revision will be needed to
>> finish those paths in Linux.
>>
>> This series is based on v7.1-rc1. It is also available as a git
>> repository:
>>
>> https://gitlab.arm.com/linux-arm/linux-cca cca-host/v14
>>
>> Work in progress changes for kvmtool are available from the git
>> repository below:
>>
>> https://gitlab.arm.com/linux-arm/kvmtool-cca cca/v12
>>
>> The TF-RMM has not yet merged the RMM v2.0 support, so you will need to
>> use a branch with RMM v2.0-bet1 support. At the time of writing the
>> following branch is being used:
>>
>> https://git.trustedfirmware.org/TF-RMM/tf-rmm.git topics/rmm-v2.0-poc_2
>> (tested on commit 3340667a291a)
>>
>> There is a kvm-unit-test branch which has been updated to support the
>> attestation used in RMMv2.0 available here:
>>
>> https://gitlab.arm.com/linux-arm/kvm-unit-tests-cca cca/v4
>>
>> [1] https://developer.arm.com/documentation/den0137/2-0bet1/
>> [2] https://lore.kernel.org/all/acrj-cKphy4hJsEG@p14s/


^ permalink raw reply

* [PATCH v5 0/3] x86/tdx: Fix port I/O handling bugs
From: Kiryl Shutsemau @ 2026-07-01 11:05 UTC (permalink / raw)
  To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: Sean Christopherson, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, David Laight,
	Andi Kleen, Dan Williams, Borys Tsyrulnikov, kvm, linux-coco,
	linux-kernel, stable, Kiryl Shutsemau (Meta)

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

Three fixes for emulated port I/O in the TDX guest #VE handler.

Patch 1 fixes an off-by-one in the GENMASK() used by handle_in() and
handle_out(): the mask was one bit too wide for every I/O size.

Patch 3 fixes 32-bit port IN to zero-extend into RAX, per x86
semantics, instead of preserving the upper 32 bits. To avoid
open-coding the partial-register-write rules, patch 2 first lifts KVM's
assign_register() helper into <asm/insn-eval.h> as insn_assign_reg() so
both KVM and the #VE handler can share it.

Patch 2 touches arch/x86/kvm/emulate.c (it removes assign_register() and
routes the callers through the new helper), so an ack from the KVM
maintainers would be appreciated before this goes through the x86 tree.

Changes since v4:
  - Rebase onto v7.2-rc1.
  - insn_assign_reg(): drop the arithmetic read-modify-write body from
    v4 and lift KVM's assign_register() verbatim (typed-pointer writes).
    This fixes the unaligned access / adjacent-register clobber for
    high-byte registers (AH/CH/DH/BH) that Sashiko flagged on v4, where
    the emulator hands the helper a pointer offset by one byte. Update
    the changelog to match.
  - Cc: stable on the insn_assign_reg() patch, as it is a prerequisite
    for the patch 3 fix.
  - Collect Reviewed-by from Binbin Wu and Rick Edgecombe.

v4: https://lore.kernel.org/all/cover.1780584300.git.kas@kernel.org/

Kiryl Shutsemau (Meta) (3):
  x86/tdx: Fix off-by-one in port I/O handling
  x86/insn-eval: Add insn_assign_reg() helper
  x86/tdx: Fix zero-extension for 32-bit port I/O

 arch/x86/coco/tdx/tdx.c          | 10 ++++------
 arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
 arch/x86/kvm/emulate.c           | 26 ++++----------------------
 3 files changed, 38 insertions(+), 28 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.54.0


^ permalink raw reply

* [PATCH v5 1/3] x86/tdx: Fix off-by-one in port I/O handling
From: Kiryl Shutsemau @ 2026-07-01 11:05 UTC (permalink / raw)
  To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: Sean Christopherson, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, David Laight,
	Andi Kleen, Dan Williams, Borys Tsyrulnikov, kvm, linux-coco,
	linux-kernel, stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-1-kirill@shutemov.name>

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:

    u64 mask = GENMASK(BITS_PER_BYTE * size, 0);

GENMASK(h, l) includes bit h. For size=1 (INB), this produces
GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8
bits). The mask is one bit too wide for all I/O sizes.

Fix the mask calculation.

Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: stable@vger.kernel.org
---
 arch/x86/coco/tdx/tdx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 29b6f1ed59ec..b8bbd715fb62 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -694,7 +694,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
 		.r13 = PORT_READ,
 		.r14 = port,
 	};
-	u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+	u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
 	bool success;
 
 	/*
@@ -714,7 +714,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
 
 static bool handle_out(struct pt_regs *regs, int size, int port)
 {
-	u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+	u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
 
 	/*
 	 * Emulate the I/O write via hypercall. More info about ABI can be found
-- 
2.54.0


^ permalink raw reply related

* [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Kiryl Shutsemau @ 2026-07-01 11:05 UTC (permalink / raw)
  To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: Sean Christopherson, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, David Laight,
	Andi Kleen, Dan Williams, Borys Tsyrulnikov, kvm, linux-coco,
	linux-kernel, stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-1-kirill@shutemov.name>

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

KVM's instruction emulator has a small helper, assign_register(), that
writes a value into a sub-register with x86 partial-register-write
semantics: 1- and 2-byte writes leave the upper bits of the destination
untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
the full register.

The TDX guest #VE handler needs the same logic for port I/O emulation
to get 32-bit zero-extension right.  Rather than copy-pasting the
helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
use it.

Add <asm/insn.h> to the header's includes so it builds standalone in
callers that have not pulled it in transitively.

No functional change.

Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
---
 arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
 arch/x86/kvm/emulate.c           | 26 ++++----------------------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index 4733e9064ee5..0c87759816d3 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -9,6 +9,7 @@
 #include <linux/compiler.h>
 #include <linux/bug.h>
 #include <linux/err.h>
+#include <asm/insn.h>
 #include <asm/ptrace.h>
 
 #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
@@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
 
 bool insn_is_nop(struct insn *insn);
 
+/*
+ * Write @val into *@reg with x86 partial-register-write semantics: a 1-
+ * or 2-byte write leaves the upper bits of the destination untouched; a
+ * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]
+ * etc.); an 8-byte write overwrites the full register.
+ *
+ * @reg need not be 8-byte aligned: KVM's instruction emulator points
+ * into the middle of a register slot to address the high-byte
+ * registers (AH, CH, DH, BH).  Use narrow stores for the sub-word
+ * cases so that the access width matches @bytes.
+ */
+static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
+{
+	switch (bytes) {
+	case 1:
+		*(u8 *)reg = (u8)val;
+		break;
+	case 2:
+		*(u16 *)reg = (u16)val;
+		break;
+	case 4:
+		*reg = (u32)val;
+		break;
+	case 8:
+		*reg = val;
+		break;
+	}
+}
+
 #endif /* _ASM_X86_INSN_EVAL_H */
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index b566ab5c7515..c6dcb5ac48af 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -24,6 +24,7 @@
 #include "kvm_emulate.h"
 #include <linux/stringify.h>
 #include <asm/debugreg.h>
+#include <asm/insn-eval.h>
 #include <asm/nospec-branch.h>
 #include <asm/ibt.h>
 #include <asm/text-patching.h>
@@ -439,25 +440,6 @@ static void assign_masked(ulong *dest, ulong src, ulong mask)
 	*dest = (*dest & ~mask) | (src & mask);
 }
 
-static void assign_register(unsigned long *reg, u64 val, int bytes)
-{
-	/* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
-	switch (bytes) {
-	case 1:
-		*(u8 *)reg = (u8)val;
-		break;
-	case 2:
-		*(u16 *)reg = (u16)val;
-		break;
-	case 4:
-		*reg = (u32)val;
-		break;	/* 64b: zero-extend */
-	case 8:
-		*reg = val;
-		break;
-	}
-}
-
 static inline unsigned long ad_mask(struct x86_emulate_ctxt *ctxt)
 {
 	return (1UL << (ctxt->ad_bytes << 3)) - 1;
@@ -505,7 +487,7 @@ register_address_increment(struct x86_emulate_ctxt *ctxt, int reg, int inc)
 {
 	ulong *preg = reg_rmw(ctxt, reg);
 
-	assign_register(preg, *preg + inc, ctxt->ad_bytes);
+	insn_assign_reg(preg, *preg + inc, ctxt->ad_bytes);
 }
 
 static void rsp_increment(struct x86_emulate_ctxt *ctxt, int inc)
@@ -1767,7 +1749,7 @@ static int load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
 
 static void write_register_operand(struct operand *op)
 {
-	return assign_register(op->addr.reg, op->val, op->bytes);
+	return insn_assign_reg(op->addr.reg, op->val, op->bytes);
 }
 
 static int writeback(struct x86_emulate_ctxt *ctxt, struct operand *op)
@@ -2008,7 +1990,7 @@ static int em_popa(struct x86_emulate_ctxt *ctxt)
 		rc = emulate_pop(ctxt, &val, ctxt->op_bytes);
 		if (rc != X86EMUL_CONTINUE)
 			break;
-		assign_register(reg_rmw(ctxt, reg), val, ctxt->op_bytes);
+		insn_assign_reg(reg_rmw(ctxt, reg), val, ctxt->op_bytes);
 		--reg;
 	}
 	return rc;
-- 
2.54.0


^ permalink raw reply related

* [PATCH v5 3/3] x86/tdx: Fix zero-extension for 32-bit port I/O
From: Kiryl Shutsemau @ 2026-07-01 11:05 UTC (permalink / raw)
  To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: Sean Christopherson, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, David Laight,
	Andi Kleen, Dan Williams, Borys Tsyrulnikov, kvm, linux-coco,
	linux-kernel, stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-1-kirill@shutemov.name>

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

According to x86 architecture rules, 32-bit operations zero-extend the
result to 64 bits. The current implementation of handle_in() only masks
the lower 32 bits, which preserves the upper 32 bits of RAX when a
32-bit port IN instruction is emulated.

Use insn_assign_reg() to write the result back into RAX with proper
partial-register-write semantics: 1- and 2-byte forms leave the upper
bits untouched, the 4-byte form zero-extends to the full register.

Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Cc: stable@vger.kernel.org
---
 arch/x86/coco/tdx/tdx.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index b8bbd715fb62..f904a636d449 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -694,8 +694,8 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
 		.r13 = PORT_READ,
 		.r14 = port,
 	};
-	u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
 	bool success;
+	u64 val;
 
 	/*
 	 * Emulate the I/O read via hypercall. More info about ABI can be found
@@ -703,11 +703,9 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
 	 * "TDG.VP.VMCALL<Instruction.IO>".
 	 */
 	success = !__tdx_hypercall(&args);
+	val = success ? args.r11 : 0;
 
-	/* Update part of the register affected by the emulated instruction */
-	regs->ax &= ~mask;
-	if (success)
-		regs->ax |= args.r11 & mask;
+	insn_assign_reg(&regs->ax, val, size);
 
 	return success;
 }
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Xiaoyao Li @ 2026-07-01 11:07 UTC (permalink / raw)
  To: Sean Christopherson, Yan Zhao
  Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, forkloop, pratyush, suzuki.poulose,
	aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
	Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
	Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
	Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
	kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
	linux-mm, linux-coco
In-Reply-To: <aj7NwCRwWEfLK-gQ@google.com>

On 6/27/2026 3:06 AM, Sean Christopherson wrote:
> On Fri, Jun 26, 2026, Yan Zhao wrote:
>> My first impression of gmem_in_place_conversion=true was that it enforces gmem
>> in-place conversion. However, it actually only enforces per-gmem private/shared
>> attribute.
>> My worry was that people might think it's a kernel bug if userspace can still
>> have shared memory from other sources after they configured
>> gmem_in_place_conversion=true.
> Ah, I see where you're coming from.  FWIW, truly enforcing in-place conversion
> is flat out impossible.  E.g. userspace can simply replace the memslot, at which
> point the memory effectively reverts to shared.

would something like below enforce the in-place conversion?

Userspace can create a memslot without gmem fd, but that memslot can 
only serve as shared memory and cannot be converted. So it doesn't 
violate the in-place conversion.

--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2122,6 +2122,8 @@ static int kvm_set_memory_region(struct kvm *kvm,
         new->flags = mem->flags;
         new->userspace_addr = mem->userspace_addr;
         if (mem->flags & KVM_MEM_GUEST_MEMFD) {
+               if (gmem_in_place_conversion)
+                       new->flags |= KVM_MEMSLOT_GMEM_ONLY;
                 r = kvm_gmem_bind(kvm, new, mem->guest_memfd, 
mem->guest_memfd_offset);
                 if (r)
                         goto out;

^ permalink raw reply

* Re: [PATCH v8 25/46] KVM: guest_memfd: Enable INIT_SHARED on guest_memfd for x86 Coco VMs
From: Xiaoyao Li @ 2026-07-01 11:18 UTC (permalink / raw)
  To: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
	Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
	Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
	Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
  Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
	linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-25-9d2959357853@google.com>

On 6/19/2026 8:32 AM, Ackerley Tng via B4 Relay wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> Now that guest_memfd supports tracking private vs. shared within gmem
> itself, allow userspace to specify INIT_SHARED on a guest_memfd instance
> for x86 Confidential Computing (CoCo) VMs, so long as in-place conversion
> is enabled, i.e. when it's actually possible for a guest_memfd instance to
> contain shared memory.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   arch/x86/kvm/x86.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2fde594e86d72..57a543dadb851 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -14116,14 +14116,15 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
>   }
>   
>   #ifdef CONFIG_KVM_GUEST_MEMFD
> -/*
> - * KVM doesn't yet support initializing guest_memfd memory as shared for VMs
> - * with private memory (the private vs. shared tracking needs to be moved into
> - * guest_memfd).
> - */
>   bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
>   {
> -	return !kvm_arch_has_private_mem(kvm);
> +	/*
> +	 * INIT_SHARED is supported if in-place conversion is enabled, or if
> +	 * the VM doesn't support private memory.  If the VM has private memory
> +	 * and in-place conversion is disabled, then guest_memfd can _only_ be
> +	 * used for private memory.
> +	 */
> +	return gmem_in_place_conversion || !kvm_arch_has_private_mem(kvm);
>   }
>   
>   #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE
> 


^ permalink raw reply

* Re: [RFC PATCH 08/15] x86/virt/tdx: Add interface to check Quoting availability
From: Nikolay Borisov @ 2026-07-01 11:25 UTC (permalink / raw)
  To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
  Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
	zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-9-yilun.xu@linux.intel.com>



On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
> 
> KVM needs to know if the Quoting extension is available to determine
> whether userspace must be involved in Quote generation.
> 
> Since the Quote buffer is always created during Quoting extension
> bringup, checking whether the buffer exists is sufficient.
> 
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>   arch/x86/include/asm/tdx.h  |  2 ++
>   arch/x86/virt/vmx/tdx/tdx.c | 15 +++++++++++++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 15eac89b0afb..7b257088aa1e 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -176,6 +176,8 @@ struct tdx_vp {
>   	struct page **tdcx_pages;
>   };
>   
> +bool tdx_quote_enabled(void);
> +
>   static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
>   {
>   	u64 ret;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 9d04293394d7..b305fa5aab5c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1213,6 +1213,21 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
>   	return page_to_phys(td->tdr_page);
>   }
>   
> +/**
> + * tdx_quote_enabled() - Check whether TDX Quoting extension is available
> + *
> + * Return: %true if the Quoting extension is available, otherwise %false.
> + */
> +bool tdx_quote_enabled(void)

nit: Probably rename the function to tdx_quoting_ext_enabled or 
tdx_quote_ext_enabled, so it's abundantly clear it's about an extension 
and not the quoting functionality in general.


> +{
> +	/*
> +	 * No need for locking here. The quote buffer is initialized as part of
> +	 * core TDX bringup, which comes before KVM is ready for userspace.
> +	 */
> +	return !!quote_data.buf;

While this works it feels a bit like a hack, perhaps have a static 
boolean variable being set by the init code which is simply returned by 
this function.

> +}
> +EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
> +
>   #define HPAS_PER_PAGE			(PAGE_SIZE / sizeof(u64))
>   
>   static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)


^ permalink raw reply

* Re: [RFC PATCH 09/15] x86/virt/tdx: Add interface to generate a Quote
From: Nikolay Borisov @ 2026-07-01 11:46 UTC (permalink / raw)
  To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
  Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
	zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-10-yilun.xu@linux.intel.com>



On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
> 
> Use the TDX Quoting extension's TDH.QUOTE.GET SEAMCALL to generate a
> Quote. Since the interface is shared across all KVM instances,
> serialize access to the SEAMCALL buffer with a mutex.
> 
> Allocate and return a per-call buffer containing the generated Quote so
> callers don't need to size the Quote buffer themselves. The caller is
> responsible for freeing the returned buffer.
> 
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
>   arch/x86/include/asm/tdx.h  |  2 +
>   arch/x86/virt/vmx/tdx/tdx.h |  1 +
>   arch/x86/virt/vmx/tdx/tdx.c | 82 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 85 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 7b257088aa1e..bc512a00a0d0 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -177,6 +177,8 @@ struct tdx_vp {
>   };
>   
>   bool tdx_quote_enabled(void);
> +void *tdx_quote_generate(struct tdx_td *td, void *in_data, u32 in_data_len,
> +			 u32 *quote_len);
>   
>   static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
>   {
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 3849f4f9cc78..01a7d7d8ada9 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -49,6 +49,7 @@
>   #define TDH_EXT_INIT			60
>   #define TDH_EXT_MEM_ADD			61
>   #define TDH_SYS_DISABLE			69
> +#define TDH_QUOTE_GET			98
>   #define TDH_QUOTE_INIT			100
>   
>   /*
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index b305fa5aab5c..821f677e9a86 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -62,6 +62,8 @@ static LIST_HEAD(tdx_memlist);
>   static struct tdx_sys_info tdx_sysinfo __ro_after_init;
>   static bool tdx_module_initialized __ro_after_init;
>   
> +static DEFINE_MUTEX(tdx_quote_lock);
> +
>   static struct quote_data {
>   	void *buf;
>   	u64 buf_len;
> @@ -1228,6 +1230,86 @@ bool tdx_quote_enabled(void)
>   }
>   EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
>   
> +#define QUOTE_ID_MASK		GENMASK_U64(47, 32)
> +
> +static u64 tdx_quote_get(struct tdx_td *td, u64 in_data_pa, u64 in_data_len,
> +			 u64 hpa_list_pa, u64 total_len, u64 *quote_len)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = tdx_tdr_pa(td),
> +		/* Don't bother specifying the quote id */
> +		.rdx = QUOTE_ID_MASK & (u64)-1,

This is simply equal to QUOTE_ID_MASK, so why not create a special value 
meaning "ANY QUOTE" i.e

#define QUOTE_ID_MASK ....
#define ANY_QUOTE QUOTE_ID_MASK

or some such .

> +		.r8 = in_data_pa,
> +		.r9 = in_data_len,
> +		.r10 = hpa_list_pa,
> +		.r11 = total_len,
> +	};
> +	u64 r;
> +
> +	do {
> +		r = seamcall_ret(TDH_QUOTE_GET, &args);
> +	} while (r == TDX_INTERRUPTED_RESUMABLE);


nit: This pattern seems to repeat a lot, might be worth it to consider 
introducing a wrapper similar to existing sc_retry?
> +
> +	*quote_len = args.rcx;
> +
> +	return r;
> +}
> +
> +/**
> + * tdx_quote_generate() - Generate a quote for a TD
> + * @td: The TD to generate the quote for.
> + * @in_data: Input data for the quote request.
> + * @in_data_len: Size of the input data in bytes.
> + * @quote_len: Returned size of the generated quote in bytes.
> + *
> + * Use the TDX Quoting extension to generate a TD quote. Pass the input data
> + * through the shared quote buffer and return the quote.
> + *
> + * Return: Newly allocated quote buffer or %NULL on failure.
> + * The caller must free the returned buffer with kvfree().
> + */
> +void *tdx_quote_generate(struct tdx_td *td, void *in_data, u32 in_data_len,
> +			 u32 *quote_len)
> +{
> +	void *quote_dup = NULL;
> +	u64 r, out_len;
> +
> +	if (!tdx_quote_enabled())
> +		return NULL;
> +
> +	/* TDH.QUOTE.GET expects the input data to fit in a page */
> +	if (in_data_len > PAGE_SIZE)
> +		return NULL;
> +
> +	mutex_lock(&tdx_quote_lock);
> +
> +	/*
> +	 * Use the first page of the quote buffer for input data. The buffer
> +	 * must be at least one page in size. @in_data may not be page-aligned,
> +	 * but TDH.QUOTE.GET expects page-aligned addresses.
> +	 */
> +	memcpy(quote_data.buf, in_data, (size_t)in_data_len);

Perhaps you can use min(PAGE_SIZE, in_data_len) and that way you can 
eliminate the in_data_len check above and copy up-to PAGE_SIZE data, if 
the data is longer - you will copy PAGE_SIZE which will likely result in 
error on generating the quote?
> +
> +	r = tdx_quote_get(td, quote_data.hpa_list[0], (u64)in_data_len,
> +			  quote_data.hpa_list_pa, quote_data.buf_len, &out_len);
> +	if (r || !out_len || out_len > quote_data.buf_len)
> +		goto out;
> +
> +	/*
> +	 * The quote buffer is a shared resource, so use it only for the
> +	 * SEAMCALL and copy the data out as soon as possible.
> +	 */
> +	quote_dup = kvmemdup(quote_data.buf, out_len, GFP_KERNEL);
> +
> +out:
> +	mutex_unlock(&tdx_quote_lock);
> +
> +	*quote_len = (u32)out_len;
> +
> +	return quote_dup;
> +}
> +EXPORT_SYMBOL_FOR_KVM(tdx_quote_generate);
> +
>   #define HPAS_PER_PAGE			(PAGE_SIZE / sizeof(u64))
>   
>   static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)


^ permalink raw reply

* Re: [PATCH v4 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Sean Christopherson @ 2026-07-01 13:25 UTC (permalink / raw)
  To: Kiryl Shutsemau (Meta)
  Cc: tglx, mingo, bp, dave.hansen, pbonzini,
	sathyanarayanan.kuppuswamy, kai.huang, xiaoyao.li, binbin.wu,
	rick.p.edgecombe, david.laight.linux, ak, djbw, tsyrulnikov.borys,
	x86, kvm, linux-coco, linux-kernel
In-Reply-To: <daac026e677c10b68740b16ed8ad2556bd9583f8.1780584300.git.kas@kernel.org>

In the shortlog, please capture that code is being moved out of KVM.  This isn't
"adding" a helper, it's moving and renaming an existing helper.   The misleading
shortlog is quite literally why I didn't look at this for a ~month, I thought it
was entirely outside of my normal scope.

On Thu, Jun 04, 2026, Kiryl Shutsemau (Meta) wrote:
> KVM's instruction emulator has a small helper, assign_register(), that
> writes a value into a sub-register with x86 partial-register-write
> semantics: 1- and 2-byte writes leave the upper bits of the destination
> untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> the full register.
> 
> The TDX guest #VE handler needs the same logic for port I/O emulation
> to get 32-bit zero-extension right. Rather than copy-pasting the helper,
> lift it to <asm/insn-eval.h> as insn_assign_reg() so both can use it.
> 
> Rewrite the body using arithmetic instead of pointer punning so the
> helper does not depend on -fno-strict-aliasing 

Meh, building with -fno-strict-aliasing is 100% mandatory for the kernel.  IMO,
even so much as suggesting that strict aliasing is something worth shooting for
is a misleading and wrong.

https://lore.kernel.org/all/CAHk-=wh921g_+TJ33JRxRGFM2uruMdqG-SONfFOD9UOsLQJ_uw@mail.gmail.com

> or little-endian byte order,

Oh come on, this is low level x86 code used to write registers.

> and add <asm/insn.h> to the header's includes so it builds standalone in
> callers that have not pulled it in transitively.
> 
> No functional change.
> 
> Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> ---
>  arch/x86/include/asm/insn-eval.h | 25 +++++++++++++++++++++++++
>  arch/x86/kvm/emulate.c           | 26 ++++----------------------
>  2 files changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> index 4733e9064ee5..85251e718a77 100644
> --- a/arch/x86/include/asm/insn-eval.h
> +++ b/arch/x86/include/asm/insn-eval.h
> @@ -9,6 +9,7 @@
>  #include <linux/compiler.h>
>  #include <linux/bug.h>
>  #include <linux/err.h>
> +#include <asm/insn.h>
>  #include <asm/ptrace.h>
>  
>  #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> @@ -46,4 +47,28 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
>  
>  bool insn_is_nop(struct insn *insn);
>  
> +/*
> + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> + * or 2-byte write leaves the upper bits of the destination untouched; a

Careful with the "upper bits" wording.  As Sashiko pointed out, this is used for
{A,B,C,D}H sub-registers as well, in which case the *lower* bits are also left
untouched.

> + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]
> + * etc.); an 8-byte write overwrites the full register.
> + */
> +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> +{
> +	switch (bytes) {
> +	case 1:
> +		*reg = (*reg & ~0xfful)   | (val & 0xff);

Sashiko's feedback aside, I strongly prefer KVM's approach as I find it much more
intuitive.  And its far, far more consistent with respect to the 4-byte and 8-byte
cases.

> +		break;
> +	case 2:
> +		*reg = (*reg & ~0xfffful) | (val & 0xffff);
> +		break;
> +	case 4:
> +		*reg = (u32)val;
> +		break;
> +	case 8:
> +		*reg = val;
> +		break;
> +	}
> +}

^ permalink raw reply

* Re: [PATCH v13 13/22] KVM: selftests: Set first memory region as shared if guest_memfd
From: Xiaoyao Li @ 2026-07-01 13:36 UTC (permalink / raw)
  To: Ackerley Tng, Lisa Wang, Andrew Jones, Binbin Wu, Chao Gao,
	Chenyi Qiang, Dave Hansen, Erdem Aktas, Ira Weiny, Isaku Yamahata,
	Kiryl Shutsemau, linux-kselftest, Paolo Bonzini, Pratik R. Sampat,
	Reinette Chatre, Rick Edgecombe, Roger Wang, Ryan Afranji,
	Sagi Shahar, Sean Christopherson, Shuah Khan, Oliver Upton
  Cc: Jeremiah McReynolds, kvm, linux-coco, linux-kernel, x86
In-Reply-To: <CAEvNRgFLjt+-cO-XT0s8uW2xXgyd+VVZ0g0Os6V0+LBFQzx8cQ@mail.gmail.com>

On 7/1/2026 2:18 AM, Ackerley Tng wrote:
>>> I think this patch should fully buy into in-place conversions, so we
>>> need to also set GUEST_MEMFD_FLAG_MMAP:
>>>
>>> @@ -483,6 +483,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape,
>>> u32 nr_runnable_vcpus,
>>>    {
>>>    	u64 nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
>>>    						 nr_extra_pages);
>>> +	enum vm_mem_backing_src_type src_type = VM_MEM_SRC_ANONYMOUS;
>>>    	struct userspace_mem_region *slot0;
>>>    	u64 gmem_flags = 0;
>>>    	struct kvm_vm *vm;
>>> @@ -503,10 +504,16 @@ struct kvm_vm *__vm_create(struct vm_shape
>>> shape, u32 nr_runnable_vcpus,
>>>    	 */
>>>    	if (is_guest_memfd_required(shape)) {
>>>    		flags |= KVM_MEM_GUEST_MEMFD;
>>> -		gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
>>> +		gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED | GUEST_MEMFD_FLAG_MMAP;
>> GUEST_MEMFD_FLAG_INIT_SHARED is valid only when the memory attributes is
>> per-gmem.
>>
> GUEST_MEMFD_FLAG_INIT_SHARED was introduced before guest_memfd in-place
> conversions, so I think it's orthogonal to whether memory attributes is
> per-gmem.

But before gmem in-place conversion, i.e., per-gmem attribute, 
GUEST_MEMFD_FLAG_INIT_SHARED is not supported/valid for Coco VMs.
>> we need to check KVM_CAP_GUEST_MEMFD_FLAGS or kvm_has_gmem_attributes.
> I think we do want to deprecate the non-in-place-conversions setup, so
> how about inserting a TEST_REQIRE(kvm_has_gmem_attributes) here?

Well, then all the TDX and SNP selftest will be skipped on the kernel 
with gmem_in_place_conversion=false.

I don't object it. But we still need to get Sean's answer.

^ permalink raw reply

* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Sean Christopherson @ 2026-07-01 13:53 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Yan Zhao, Ackerley Tng, aik, andrew.jones, binbin.wu, brauner,
	chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
	pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
	steven.price, tabba, willy, wyihan, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
	Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
	Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
	Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
	Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
	linux-coco
In-Reply-To: <25fdb77d-20f6-4b3a-8b3a-dbba0dc47046@intel.com>

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/27/2026 3:06 AM, Sean Christopherson wrote:
> > On Fri, Jun 26, 2026, Yan Zhao wrote:
> > > My first impression of gmem_in_place_conversion=true was that it enforces gmem
> > > in-place conversion. However, it actually only enforces per-gmem private/shared
> > > attribute.
> > > My worry was that people might think it's a kernel bug if userspace can still
> > > have shared memory from other sources after they configured
> > > gmem_in_place_conversion=true.
> > Ah, I see where you're coming from.  FWIW, truly enforcing in-place conversion
> > is flat out impossible.  E.g. userspace can simply replace the memslot, at which
> > point the memory effectively reverts to shared.
> 
> would something like below enforce the in-place conversion?

No.

> Userspace can create a memslot without gmem fd, but that memslot can only
> serve as shared memory and cannot be converted. So it doesn't violate the
> in-place conversion.

But userspace can delete said memslot and replace it with a memslot pointing at
a guest_memfd instance that was created without INIT_SHARED, at which point
userspace has effected a shared=>private conversion.

^ permalink raw reply

* Re: [PATCH v14 29/44] arm64: RMI: Runtime faulting of memory
From: Lorenzo Pieralisi @ 2026-07-01 13:58 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Suzuki K Poulose, Steven Price, kvm, kvmarm, Catalin Marinas,
	Marc Zyngier, Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
	linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
	Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
	Shanker Donthineni, Alper Gun, Aneesh Kumar K . V, Emi Kisanuki,
	Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <901398bb-ed6c-4997-b3cd-ce2829b09c87@redhat.com>

On Sun, Jun 28, 2026 at 08:33:01PM +1000, Gavin Shan wrote:
> On 6/27/26 2:44 AM, Lorenzo Pieralisi wrote:
> > On Fri, Jun 26, 2026 at 09:43:03PM +1000, Gavin Shan wrote:
> > > On 6/26/26 6:47 PM, Suzuki K Poulose wrote:
> > > > On 26/06/2026 08:43, Gavin Shan wrote:
> > > > > On 6/26/26 1:58 AM, Suzuki K Poulose wrote:
> > > > > > On 25/06/2026 14:53, Gavin Shan wrote:
> > > > > > > On 6/6/26 12:35 AM, Lorenzo Pieralisi wrote:
> > > > > > > > On Fri, Jun 05, 2026 at 06:11:11PM +1000, Gavin Shan wrote:
> > > > > > > > > On 6/5/26 5:28 PM, Lorenzo Pieralisi wrote:
> > > > > > > > > > On Fri, Jun 05, 2026 at 04:23:15PM +1000, Gavin Shan wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > > > > 
> > > > > > > > > I tried to rebase Jean's latest QEMU series [1] to upstream QEMU, and found
> > > > > > > > > that memory slots backed by THP are broken. With THP disabled on the host and
> > > > > > > > > other fixes (mentioned in my prevous replies) applied on the top of this (v14)
> > > > > > > > > series, I'm able to boot a realm guest with rebased QEMU series [2], plus more
> > > > > > > > > fxies on the top.
> > > > > > > > > 
> > > > > > > > > [1] https://git.codelinaro.org/linaro/dcap/qemu.git  (branch: cca/ latest)
> > > > > > > > > [2] https://git.qemu.org/git/qemu.git                (branch: cca/ gavin)
> > > > > > > > > 
> > > > > > > > > Lorenzo, You may be saying there is someone making QEMU to support ARM/CCA?
> > > > > > > > 
> > > > > > > > Mathieu and I are working on that yes and with Steven/Suzuki to fix the THP
> > > > > > > > issues you pointed out above.
> > > > > > > > 
> > > > > > > > > If so, I'm not sure if there is a QEMU repository for me to try?
> > > > > > > > 
> > > > > > > > We should be able to submit patches by end of June - we shall let you know
> > > > > > > > whether we can make something available earlier.
> > > > > > > > 
> > > > > > > 
> > > > > > > Not sure if there are other known issues in this series. It seems the stage2
> > > > > > > page fault handling on the shared space isn't working well. In my test, the
> > > > > > > vring (struct vring_desc) of virtio-net-pci is updated by the guest, and the
> > > > > > > data isn't seen by QEMU, I'm suspecting if the host-page-frame-number is properly
> > > > > > > resolved in the s2 page fault handler for shared (unprotected) space.
> > > > > > > 
> > > > > > > - I rebased Jean's latest qemu branch to the upstream qemu;
> > > > > > > 
> > > > > > > - On the host, which is emulated by qemu/tcg, the THP (transparent huge page) is
> > > > > > >     disabled.
> > > > > > > 
> > > > > > > - On the guest, I can see the virtio vring (struct vring_desc) is updated. The
> > > > > > >     S1 page-table entry looks correct because the corresponding physical address
> > > > > > >     0x10046880000 is a sane shared (unprotected) space address.
> > > > > > > 
> > > > > > >     [   52.094143] software IO TLB: Memory encryption is active and system is using DMA bounce buffers
> > > > > > >     [   52.289746] virtqueue_add_desc_split: desc[0]@0xffff000006880000, [00000100b983f000  00000640  0002  0001]
> > > > > > >     [   52.432150] PTE 0x00e8010046880707 at address 0xffff000006880000
> > > > > > > 
> > > > > > > - On the host, the s2 page-table-entry is unmapped due to attribute transition (private -> shared).
> > > > > > >     A subsequent S2 page fault is raised against the adress and the s2 page-table-entry is built.
> > > > > > > 
> > > > > > >     [  109.259077] ====> realm_unmap_shared_range: tracked_unprot_addr=0x10046880000
> > > > > > >     [  109.260249] realm_unmap_shared_range: unmapped shared range at 0x10046880000
> > > > > > >     [  109.317786] realm_unmap_shared_range: unmapped shared range at 0x10046880000
> > > > > > >     [  109.629939] ====> kvm_handle_guest_abort: fault_ipa=0x10046880000, esr=0x92000007
> > > > > > >     [  109.630245] realm_map_non_secure: ipa=0x10046880000, pfn=0xb8b59, size=0x1000, prot=0xf
> > > > > > >     [  109.630331] realm_map_non_secure: ipa=0x10046880000, ipa_top=0x10046881000, flags=0x1e0001, range_desc=0xb8b59004
> > > > > > 
> > > > > > Are you able to correlate the order of the transitions and the Guest
> > > > > > access with RMM log ? We haven't seen this from our end. We are aware
> > > > > > of permission fault issues with Unprotected IPA when backing the memslot
> > > > > > with MAP_PRIVATE areas. But this looks different.
> > > > > > 
> > > > > > Lorenzo, have you run into this ?
> > > > > > 
> > > > > 
> > > > > It's hard to correlate the order since the logs are collected from two separate
> > > > > consoles. For the write permission, I add code to the host where the permission
> > > > > is always added for all s2 page faults in the shared space. Otherwise, qemu can
> > > > > be killed by -EFAULT or similar error.
> > > > 
> > > > This is the problem. We can't add WRITE permission by default. I believe
> > > > you may have MAP_PRIVATE mapping and it has to be mapped as READ only
> > > > and on a permission fault, we replace it with a writable page. By
> > > > overriding the WRITE permission, you let the guest write to a page
> > > > that may not be seen by the VMM.
> > > > 
> > > > We identified this as a bug in the KVM driver in this series (reported
> > > > by Lorenzo) and there is a corresponding tf-RMM change that is required
> > > > to get this working. So, please could you wait until the next series
> > > > when this will be addressed ? Or you could switch to using MAP_SHARED
> > > > for the "shared" memory in the memslot.
> > > > 
> > > 
> > > Exactly. the syntax for MAP_PRIVATE is broken if the write permission is
> > > enforced for a read fault in the shared space. In my case, the host page can
> > > be the zero page and eventually multiple s2 page-table entries (for multiple
> > > unprotected or shared pages) point to the zero page. It's why clearing the
> > > 3rd queue (Ctrl queue) also clears the first queue (Rx queue) in my case.
> > > 
> > > Yes, this issue can be avoid by using a shared memory backend in qemu, something
> > > like below. With this, I'm able to see virtio-net-pci starts to work...
> > > 
> > >      -object memory-backend-ram,id=mem0,size=2G,share=yes
> > 
> > Yes, as Suzuki said that's what we have been fixing. QEmu patches
> > will be on the mailing lists very shortly - the KVM/tf-RMM fixes
> > to make MAP_PRIVATE work will be included in the next posting.
> > 
> > Feel free to drop your QEmu command line so that I can give it
> > a shot and check whether the fixes solve the problem you hit
> > (I think so because that's precisely the kind of issue I got
> > into when I started debugging THP/MAP_PRIVATE but it is better
> > to check).
> > 
> 
> The virtio-net-pci doesn't work with the following command lines. The guest
> kernel image is built from upstream kernel (v7.1.rc7).
> 
>     qemu-system-aarch64 -enable-kvm -object rme-guest,id=rme0,             \
>     -machine virt,gic-version=3,confidential-guest-support=rme0            \
>     -cpu host,pmu=off                                                      \
>     -smp maxcpus=2,cpus=2,sockets=1,clusters=1,cores=1,threads=2           \
>     -m 2G -object memory-backend-ram,id=mem0,size=2G                       \
>     -numa node,nodeid=0,cpus=0-1,memdev=mem0                               \
>     -serial mon:stdio -monitor none -nographic -nodefaults                 \
>     -kernel /mnt/linux/arch/arm64/boot/Image                               \
>     -initrd /mnt/buildroot/output/images/rootfs.cpio.xz                    \
>     -append earlycon=pl011,mmio,0x10009000000                              \
>     -device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1                  \
>     -device pcie-root-port,bus=pcie.0,chassis=2,id=pcie.2                  \
>     -device pcie-root-port,bus=pcie.0,chassis=3,id=pcie.3                  \
>     -device pcie-root-port,bus=pcie.0,chassis=4,id=pcie.4                  \
>     -netdev tap,id=tap1,vhost=on,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown  \
>     -device virtio-net-pci,bus=pcie.2,netdev=tap1,mac=b8:3f:d2:1d:3e:c0

Tested with private RAM backend, THP left enabled (and additional KVM and
RMM patches that will be included in v15) looks like the net interface
comes up cleanly.

Probably it is best to sync up on the upcoming v15 posting that I am
testing in the background to make sure you don't spend more time chasing
it.

Thanks,
Lorenzo

> 
> The virtio-net-pci starts to work with the shareable memory-backend.
> 
>     -object memory-backend-ram,id=mem0,size=2G,share=yes
> 
> Note that THP is disabled on my host.
> 
>     root@host:~# cat /sys/kernel/mm/transparent_hugepage/enabled
>     always madvise [never]

^ permalink raw reply

* Re: [PATCH v13 13/22] KVM: selftests: Set first memory region as shared if guest_memfd
From: Sean Christopherson @ 2026-07-01 14:00 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Ackerley Tng, Lisa Wang, Andrew Jones, Binbin Wu, Chao Gao,
	Chenyi Qiang, Dave Hansen, Erdem Aktas, Ira Weiny, Isaku Yamahata,
	Kiryl Shutsemau, linux-kselftest, Paolo Bonzini, Pratik R. Sampat,
	Reinette Chatre, Rick Edgecombe, Roger Wang, Ryan Afranji,
	Sagi Shahar, Shuah Khan, Oliver Upton, Jeremiah McReynolds, kvm,
	linux-coco, linux-kernel, x86
In-Reply-To: <81caa42d-9d07-4bed-892d-c24e0b298a1d@intel.com>

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 7/1/2026 2:18 AM, Ackerley Tng wrote:
> > > > I think this patch should fully buy into in-place conversions, so we
> > > > need to also set GUEST_MEMFD_FLAG_MMAP:
> > > > 
> > > > @@ -483,6 +483,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape,
> > > > u32 nr_runnable_vcpus,
> > > >    {
> > > >    	u64 nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
> > > >    						 nr_extra_pages);
> > > > +	enum vm_mem_backing_src_type src_type = VM_MEM_SRC_ANONYMOUS;
> > > >    	struct userspace_mem_region *slot0;
> > > >    	u64 gmem_flags = 0;
> > > >    	struct kvm_vm *vm;
> > > > @@ -503,10 +504,16 @@ struct kvm_vm *__vm_create(struct vm_shape
> > > > shape, u32 nr_runnable_vcpus,
> > > >    	 */
> > > >    	if (is_guest_memfd_required(shape)) {
> > > >    		flags |= KVM_MEM_GUEST_MEMFD;
> > > > -		gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
> > > > +		gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED | GUEST_MEMFD_FLAG_MMAP;
> > > GUEST_MEMFD_FLAG_INIT_SHARED is valid only when the memory attributes is
> > > per-gmem.
> > > 
> > GUEST_MEMFD_FLAG_INIT_SHARED was introduced before guest_memfd in-place
> > conversions, so I think it's orthogonal to whether memory attributes is
> > per-gmem.
> 
> But before gmem in-place conversion, i.e., per-gmem attribute,
> GUEST_MEMFD_FLAG_INIT_SHARED is not supported/valid for Coco VMs.
> > > we need to check KVM_CAP_GUEST_MEMFD_FLAGS or kvm_has_gmem_attributes.
> > I think we do want to deprecate the non-in-place-conversions setup, so
> > how about inserting a TEST_REQIRE(kvm_has_gmem_attributes) here?

No.  Unless supporting out-of-place conversion requires orders of magnitude more
effort than just supporting in-place conversion, selftests should play nice with
both.

> Well, then all the TDX and SNP selftest will be skipped on the kernel with
> gmem_in_place_conversion=false.

Exactly.  They'll also be unusable on previous kernels.

Deprecating per-VM PRIVATE tracking does not mean dropping support.  We won't be
able to drop support for years, if ever.  And so from a testing perspective, we
absolutely need to validate both models (again, within reason).

^ permalink raw reply

* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Sean Christopherson @ 2026-07-01 14:59 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Paolo Bonzini, Kuppuswamy Sathyanarayanan, Kai Huang, Xiaoyao Li,
	Rick Edgecombe, Binbin Wu, David Laight, Andi Kleen, Dan Williams,
	Borys Tsyrulnikov, kvm, linux-coco, linux-kernel, stable,
	Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-3-kirill@shutemov.name>

On Wed, Jul 01, 2026, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> 
> KVM's instruction emulator has a small helper, assign_register(), that
> writes a value into a sub-register with x86 partial-register-write
> semantics: 1- and 2-byte writes leave the upper bits of the destination
> untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> the full register.
> 
> The TDX guest #VE handler needs the same logic for port I/O emulation
> to get 32-bit zero-extension right.  Rather than copy-pasting the
> helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
> use it.
> 
> Add <asm/insn.h> to the header's includes so it builds standalone in
> callers that have not pulled it in transitively.
> 
> No functional change.
> 
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> ---
>  arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
>  arch/x86/kvm/emulate.c           | 26 ++++----------------------
>  2 files changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> index 4733e9064ee5..0c87759816d3 100644
> --- a/arch/x86/include/asm/insn-eval.h
> +++ b/arch/x86/include/asm/insn-eval.h
> @@ -9,6 +9,7 @@
>  #include <linux/compiler.h>
>  #include <linux/bug.h>
>  #include <linux/err.h>
> +#include <asm/insn.h>
>  #include <asm/ptrace.h>
>  
>  #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> @@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
>  
>  bool insn_is_nop(struct insn *insn);
>  
> +/*
> + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> + * or 2-byte write leaves the upper bits of the destination untouched; a
> + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]

The placement of the "(matching IN[BWL], MOV[BWL] etc.)" blurb is confusing.  I
*think* you're trying to say this behavior matches that of MOVB, MOVW, and MOVL
instruction mnemonics, but the blurb is buried in the snippet that specifically
describes the 4-byte write behavior.

FWIW, I think giving examples does more harm than good, because the behavior isn't
instruction specific, it's architectural behavior that applies to all writes to
GPRs, as defined in "3.4.1.1 General-Purpose Registers in 64-Bit Mode".  E.g. for
a MOV instruction that sign-extends a 32-bit immediate to a 64-bit registers, it's
not that the instruction is exempt from the normal GPR semenatics, it's that the
instruction performs a 64-bit write to the destination even though the source is
only 32 bits.

And the B/W/L terminology isn't architectural, it's AT&T syntax.  E.g. trying
to encode "movl" with NASM yields "error: instruction expected, found `movl dword'".
Yes, the kernel uses AT&T syntax for assembly, but I think this helper should very
explicitly document that it's emulating architectural behavior.

> + * etc.); an 8-byte write overwrites the full register.
> + *
> + * @reg need not be 8-byte aligned: KVM's instruction emulator points
> + * into the middle of a register slot to address the high-byte
> + * registers (AH, CH, DH, BH).  Use narrow stores for the sub-word
> + * cases so that the access width matches @bytes.
> + */
> +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> +{
> +	switch (bytes) {
> +	case 1:
> +		*(u8 *)reg = (u8)val;
> +		break;
> +	case 2:
> +		*(u16 *)reg = (u16)val;
> +		break;
> +	case 4:
> +		*reg = (u32)val;

IMO, it's worth keeping a short comment here, because even with the explanation
above, I suspect most people will think the code is buggy.  E.g.

		/* As above, zero-extend 4-byte writes on 64-bit CPUs. */
		*reg = (u32)val;

> +		break;
> +	case 8:
> +		*reg = val;
> +		break;
> +	}
> +}

^ permalink raw reply

* Re: [PATCH v8 07/46] KVM: Rename memory attribute APIs to prepare for in-place gmem conversion
From: Sean Christopherson @ 2026-07-01 15:07 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
	Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
	Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
	Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
	Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
	linux-coco
In-Reply-To: <e5876e41-a11a-4d5e-958f-9e247c19d387@intel.com>

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 7/1/2026 1:30 AM, Sean Christopherson wrote:
> > On Tue, Jun 30, 2026, Xiaoyao Li wrote:
> > > On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > > > -bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> > > > -				     unsigned long mask, unsigned long attrs);
> > > > +bool kvm_range_has_vm_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> > > > +					unsigned long mask, unsigned long attrs);
> > > >    bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> > > >    					struct kvm_gfn_range *range);
> > > >    bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
> > > 
> > > We have
> > > 
> > >   - kvm_pre_set_memory_attributes()
> > >   - kvm_arch_pre_set_memory_attributes()
> > >   - kvm_arch_post_set_memory_attributes()
> > 
> > Yeah, that's probably for the best.
> > 
> > > left, do they need to be renamed as well?
> > > 
> > > then the interesting one is kvm_vm_set_mem_attributes(), which contains "vm"
> > > already while it means "vm ioctl". Do we need to rename it to
> > > kvm_vm_set_vm_mem_attributes()?
> > 
> > I say "no" on this last one, the fact that the function is scoped to a VM ioctl
> > is enough to communicate that it applies to per-VM attributes.
> > 
> > Actually, since it's a local helper, we could go with kvm_set_vm_mem_attributes()
> > to be consistent with the other functions.  That just leaves
> > kvm_vm_ioctl_set_mem_attributes(), which I think it appropriately scoped.
> 
> If we finally choose to rename kvm_vm_set_mem_attributes() to
> kvm_set_vm_mem_attributes(), I think the trace
> trace_kvm_vm_set_mem_attributes() needs to be renamed to keep it consistent?

Ya, good catch!

^ permalink raw reply

* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Sean Christopherson @ 2026-07-01 15:35 UTC (permalink / raw)
  To: Ackerley Tng
  Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
	jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
	rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
	wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
	aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
	Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
	Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
	Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
	Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
	kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
	linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-13-9d2959357853@google.com>

On Thu, Jun 18, 2026, Ackerley Tng wrote:
> Introduce base support for KVM_SET_MEMORY_ATTRIBUTES2 in guest_memfd, which
> just updates attributes tracked by guest_memfd.
> 
> Validate input fields in general. Guard usage of KVM_SET_MEMORY_ATTRIBUTES2
> by making sure requested attributes are supported for this instance of kvm.
> 
> A new KVM_SET_MEMORY_ATTRIBUTES2 is defined to support writes (unlike

Phrase this as a command using imperative mood.  The wording is also weird,
because "support writes" makes it sound like it allows controlling WRITE attributes,
whereas what you mean by "support writes" is "allowing KVM to write back error
information to the struct without technically violating the semantics embedded
in the ioctl".  It's doubly confusing because the macros use a different polarity:
IOW means userspace is writing, but this implicitly refers to IOW as "reads".

> KVM_SET_MEMORY_ATTRIBUTES) in addition to reads so it can provide error
> details to userspace. This will be used in a later patch.
> 
> The two ioctls use their corresponding structs with no overlap, but
> backward compatibility is baked in for future support of
> KVM_SET_MEMORY_ATTRIBUTES2 and struct kvm_memory_attributes2 in the VM
> ioctl.

I don't understand what this paragraph is trying to say with respect to backwards
compatibility.  It's a new ioctl and struct, there's no compatibility in sight.

E.g.

  Add a new ioctl (and matching struct), KVM_SET_MEMORY_ATTRIBUTES2, using
  the same base ioctl number (0xd2), but with R/W semantics for the kernel
  instead of just read semantics.  "Officially" documenting that KVM writes
  to the payload will allow KVM to support partial/incremental conversions,
  instead of all-or-nothing updates (which requires complex unwinding), by
  recording the failing offset if an error occurs.

  Opportunistically add a new struct as well, even though KVM could squeeze
  the error offset into "struct kvm_memory_attributes", as there's no cost
  to doing so in practice.  Pad the struct with a pile of extra space to try
  and avoid ending up with "struct kvm_memory_attributes3" in the future.
  Use the same layout for the fields that common to version 1 of the struct,
  e.g. to ease upgrading userspace, and to provide flexibility in KVM ever
  adds support for KVM_SET_MEMORY_ATTRIBUTES2 at VM scope.

> The process of setting memory attributes is set up such that the later half
> will not fail due to allocation. Any necessary checks are performed before
> the point of no return.

Explain *why*.  Readers can usually understand the "what" by reading the code,
but it's much harder to discern *why* things were done a certain way.  Some things
go without saying, e.g. "validate input fields", but in that case, just drop the
changelog blurb (if we _weren't_ validating input, *that* would be interesting and
worth calling out).

> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> Co-developed-by: Sean Christoperson <seanjc@google.com>
> Signed-off-by: Sean Christoperson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  include/uapi/linux/kvm.h |  13 ++++++
>  virt/kvm/Kconfig         |   1 +
>  virt/kvm/guest_memfd.c   | 116 +++++++++++++++++++++++++++++++++++++++++++++++
>  virt/kvm/kvm_main.c      |  12 +++++
>  4 files changed, 142 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 419011097fa8e..956877a6aab05 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1649,6 +1649,19 @@ struct kvm_memory_attributes {
>  	__u64 flags;
>  };
>  
> +#define KVM_SET_MEMORY_ATTRIBUTES2              _IOWR(KVMIO,  0xd2, struct kvm_memory_attributes2)
> +
> +struct kvm_memory_attributes2 {
> +	union {
> +		__u64 address;
> +		__u64 offset;
> +	};
> +	__u64 size;
> +	__u64 attributes;
> +	__u64 flags;
> +	__u64 reserved[12];
> +};
> +
>  #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
>  
>  #define KVM_CREATE_GUEST_MEMFD	_IOWR(KVMIO,  0xd4, struct kvm_create_guest_memfd)
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index 297e4399fbd49..cfa2c78ba5fb9 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -102,6 +102,7 @@ config KVM_MMU_LOCKLESS_AGING
>  
>  config KVM_GUEST_MEMFD
>         select XARRAY_MULTI
> +       select KVM_MEMORY_ATTRIBUTES
>         bool
>  
>  config HAVE_KVM_ARCH_GMEM_PREPARE
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 65ce795c090d9..0d14548c1ed22 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -541,11 +541,127 @@ bool kvm_gmem_is_private(struct kvm *kvm, gfn_t gfn)
>  }
>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_is_private);
>  
> +/*
> + * Preallocate memory for attributes to be stored on a maple tree, pointed to
> + * by mas.  Adjacent ranges with attributes identical to the new attributes
> + * will be merged.  Also sets mas's bounds up for storing attributes.
> + *
> + * This maintains the invariant that ranges with the same attributes will
> + * always be merged.
> + */
> +static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
> +				    pgoff_t start, size_t nr_pages)
> +{
> +	pgoff_t end = start + nr_pages;
> +	pgoff_t last = end - 1;
> +	void *entry;
> +
> +	/* Try extending range. entry is NULL on overflow/wrap-around. */
> +	mas_set(mas, end);
> +	entry = mas_find(mas, end);
> +	if (entry && xa_to_value(entry) == attributes)
> +		last = mas->last;
> +
> +	if (start > 0) {
> +		mas_set(mas, start - 1);
> +		entry = mas_find(mas, start - 1);
> +		if (entry && xa_to_value(entry) == attributes)
> +			start = mas->index;
> +	}
> +
> +	mas_set_range(mas, start, last);
> +	return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
> +}
> +
> +static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> +				     size_t nr_pages, uint64_t attrs)
> +{
> +	struct address_space *mapping = inode->i_mapping;
> +	struct gmem_inode *gi = GMEM_I(inode);
> +	pgoff_t end = start + nr_pages;
> +	struct maple_tree *mt;
> +	struct ma_state mas;
> +	int r;
> +
> +	mt = &gi->attributes;
> +
> +	filemap_invalidate_lock(mapping);
> +
> +	mas_init(&mas, mt, start);
> +	r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
> +	if (r)
> +		goto out;
> +
> +	/*
> +	 * From this point on guest_memfd has performed necessary
> +	 * checks and can proceed to do guest-breaking changes.
> +	 */
> +
> +	kvm_gmem_invalidate_start(inode, start, end);
> +	mas_store_prealloc(&mas, xa_mk_value(attrs));
> +	kvm_gmem_invalidate_end(inode, start, end);
> +out:
> +	filemap_invalidate_unlock(mapping);
> +	return r;
> +}
> +
> +static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
> +{
> +	struct gmem_file *f = file->private_data;
> +	struct inode *inode = file_inode(file);
> +	struct kvm_memory_attributes2 attrs;
> +	size_t nr_pages;
> +	pgoff_t index;
> +	int i;
> +
> +	if (copy_from_user(&attrs, argp, sizeof(attrs)))
> +		return -EFAULT;
> +
> +	if (attrs.flags)
> +		return -EINVAL;
> +	for (i = 0; i < ARRAY_SIZE(attrs.reserved); i++) {
> +		if (attrs.reserved[i])
> +			return -EINVAL;
> +	}
> +	if (!kvm_arch_has_private_mem(f->kvm))
> +		return -EINVAL;
> +	if (attrs.attributes & ~KVM_MEMORY_ATTRIBUTE_PRIVATE)
> +		return -EINVAL;
> +	if (attrs.size == 0 || attrs.offset + attrs.size < attrs.offset)
> +		return -EINVAL;
> +	if (!PAGE_ALIGNED(attrs.offset) || !PAGE_ALIGNED(attrs.size))
> +		return -EINVAL;
> +
> +	if (attrs.offset >= i_size_read(inode) ||
> +	    attrs.offset + attrs.size > i_size_read(inode))
> +		return -EINVAL;
> +
> +	nr_pages = attrs.size >> PAGE_SHIFT;
> +	index = attrs.offset >> PAGE_SHIFT;
> +	return __kvm_gmem_set_attributes(inode, index, nr_pages,
> +					 attrs.attributes);
> +}
> +
> +static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl,
> +			   unsigned long arg)
> +{
> +	switch (ioctl) {
> +	case KVM_SET_MEMORY_ATTRIBUTES2:
> +		if (!gmem_in_place_conversion)
> +			return -ENOTTY;
> +
> +		return kvm_gmem_set_attributes(file, (void __user *)arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
>  static struct file_operations kvm_gmem_fops = {
>  	.mmap		= kvm_gmem_mmap,
>  	.open		= generic_file_open,
>  	.release	= kvm_gmem_release,
>  	.fallocate	= kvm_gmem_fallocate,
> +	.unlocked_ioctl	= kvm_gmem_ioctl,
>  };
>  
>  static int kvm_gmem_migrate_folio(struct address_space *mapping,
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 01761f6e25d25..a08b518cdb175 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -105,6 +105,18 @@ module_param(allow_unsafe_mappings, bool, 0444);
>  bool __ro_after_init gmem_in_place_conversion = false;
>  #endif
>  
> +#define MEMORY_ATTRIBUTES_MATCH(one, two)				\

Use the same terminology as the memory region asserts, i.e.
SANITY_CHECK_MEM_ATTRIBUTES_FIELD.  MEMORY_ATTRIBUTES_MATCH() reads like a helper
that checks if the two objects have the same attributes.

And put the checks where it actually matters, i.e. in the case-statement for
KVM_SET_MEMORY_ATTRIBUTES (again, same as KVM_SET_USER_MEMORY_REGION).  Because
the only reason it matters for KVM is if we want to add VM-scoped support for
KVM_SET_MEMORY_ATTRIBUTES2 in the future, at which point we'll want to use the
same overlay shenanigans that we did for KVM_SET_USER_MEMORY_REGION2.

> +	static_assert(offsetof(struct kvm_memory_attributes, one) ==	\
> +		      offsetof(struct kvm_memory_attributes2, two));	\

And then once these are landed in function scope, use BUILD_BUG_ON() with a
do { ... } while (0).

> +	static_assert(sizeof_field(struct kvm_memory_attributes, one) ==\
> +		      sizeof_field(struct kvm_memory_attributes2, two))
> +
> +/* Ensure the common parts of the two structs are identical. */
> +MEMORY_ATTRIBUTES_MATCH(address, address);
> +MEMORY_ATTRIBUTES_MATCH(size, size);
> +MEMORY_ATTRIBUTES_MATCH(attributes, attributes);
> +MEMORY_ATTRIBUTES_MATCH(flags, flags);

Please put these asserts in the location where the overlay matters.  Actually, I
don't think we need to enforce this?

^ permalink raw reply

* Re: [PATCH v8 17/46] KVM: guest_memfd: Advertise KVM_SET_MEMORY_ATTRIBUTES2 ioctl
From: Sean Christopherson @ 2026-07-01 16:09 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
	Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
	Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
	Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
	Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
	linux-coco
In-Reply-To: <584a8f9a-1538-4f8b-b576-75ef0fa961c7@intel.com>

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > @@ -4969,6 +4973,11 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
> >   		return 1;
> >   	case KVM_CAP_GUEST_MEMFD_FLAGS:
> >   		return kvm_gmem_get_supported_flags(kvm);
> > +	case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES:
> > +		if (!gmem_in_place_conversion || !kvm_supports_private_mem(kvm))
> > +			return 0;
> > +
> > +		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
> >   #endif
> >   	default:
> >   		break;
> 
> this looks inconsistent with the
> 
> 	case KVM_SET_MEMORY_ATTRIBUTES2:
> 		if (!gmem_in_place_conversion)
> 			return -ENOTTY;
> 
> Well, the check of
> 
> 	if (!kvm_arch_has_private_mem(f->kvm))
> 		return -EINVAL;
> 
> is buried in the following kvm_gmem_set_attributes(). How about moving of
> kvm_arch_has_private_mem() check to put it along with
> gmem_in_place_conversion check in kvm_gmem_ioctl() in Patch 13?

Me confused, patch 13 already adds the kvm_arch_has_private_mem() in
kvm_gmem_set_attributes().

That said, the ordering here is wonky and misleading.  A cursory read of the series
would make one think that waiting to advertise KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES
makes it safe/ok for KVM to plumb in support for KVM_SET_MEMORY_ATTRIBUTES2 over
multiple patches.  But that's not actually true, because the ioctl becomes live
the instant the code exists, userspace doesn't need to wait for KVM to formally
advertise support.

To further confuse matters, it is actually safe/ok to iteratively add support,
because it's all effectively dead code until "Let userspace disable per-VM mem
attributes, enable per-gmem attributes".

So, I think we should go a step further than what I think Xiaoyao is suggesting,
and fully squash patch 17 into patch 13.  That way the reader doesn't have to jump
through as many mental hoops to piece together what is happening.  It'll obviously
be a bigger patch, but should be easier to review/understand overall.

Oh, and that combined patch should carve out error_offset straightaway, so that
the full uAPI can be reviewed in a single patch.

^ permalink raw reply

* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: K Prateek Nayak @ 2026-07-01 16:39 UTC (permalink / raw)
  To: Jethro Beekman, Ashish Kalra, tglx, mingo, bp, dave.hansen, x86,
	hpa, seanjc, peterz, thomas.lendacky, herbert, davem, ardb
  Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
	ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
	pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
	linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <80f3f279-d70e-44d7-a179-c52068115e46@fortanix.com>

Hello Jethro,

On 7/1/2026 3:10 PM, Jethro Beekman wrote:
> I don't believe my concern has been addressed
> 
> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/

Quoting your question:

> I think this is too broad. If I have a hypervisor that supports SNP
> virtualization, a (non-confidential) L1 guest running Linux should
> still support CPU hotplug while also running confidential L2 guests.

Ashish, Tom, correct me if I'm wrong, but I don't think KVM exposes SNP
support to L1, at least as per
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/cpuid.c?h=v7.2-rc1#n1221
and only SNP initialization disables hotplug - not the other variants.

L1, running a confidential guest (SEV/SEV-ES) should still be able to
support hotplug since it doesn't go through SNP init. Only the base
hypervisor can setup the RMP tables and go through snp_prepare().

Also bsp_determine_snp() should clear CC_ATTR_HOST_SEV_SNP if it
detects X86_FEATURE_HYPERVISOR so I don't see how this can be a
problem for hotplug in L1.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/amd.c?h=v7.2-rc1#n368

-- 
Thanks and Regards,
Prateek


^ permalink raw reply

* Re: [PATCH v8 06/46] KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined
From: Sean Christopherson @ 2026-07-01 16:55 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
	david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
	qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
	tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
	suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
	Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
	Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
	Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
	Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
	linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
	linux-coco
In-Reply-To: <739e6834-40b3-405b-ada4-d31c38d8416a@intel.com>

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > From: Ackerley Tng <ackerleytng@google.com>
> > 
> > Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based
> > on kvm_arch_has_private_mem being #defined in anticipation of decoupling
> > kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
> 
> Well, after this series, kvm_supported_mem_attributes() is renamed to
> kvm_supported_vm_mem_attributes(), and it's still under
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
> 
> > guest_memfd support for memory attributes will be unconditional to avoid
> > yet more macros (all architectures that support guest_memfd are expected to
> > use per-gmem attributes at some point), at which point enumerating support
> > KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being
> > supported _somewhere_ would result in KVM over-reporting support on arm64.
> 
> I don't understand it. This patch only changes the behavior of
> kvm_supported_mem_attributes(), the usage of which is guarded by
> CONFIG_KVM_VM_MEMORY_ATTRIBUTESq. This is config is only visible to x86 due
> to patch 03. How does it affect arm64?

Hrm, yeah, this is messed up.  Ahh, I think Ackerley shuffled things around and
"broke" stuff in the process.  In v7[1] and earlier, the diff was this:

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 091f201251159..68142bc962953 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -722,7 +722,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
 }
 #endif
 
-#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifndef kvm_arch_has_private_mem
 static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
 {
 	return false;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 306153abbafa5..abb9cfa3eb04d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2421,8 +2421,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
 #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
 static u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
+#ifdef kvm_arch_has_private_mem
 	if (!kvm || kvm_arch_has_private_mem(kvm))
 		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+#endif
 
 	return 0;
 }

which makes a *lot* more sense given the changelog (and IMO for the ordering in
general).  In v8 here, Ackerley combined part of a change[2] (that I provided
off-list) with part of this commit, to create patch 4, "KVM: Decouple
kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTESthe".

Ackerley, the cover letter says:

  + Reshuffled the earlier commits that deal with preparing KVM to stop
    seeing VM memory attributes as the only source of attributes.

but there's no explanation for *why* the reshuffling was done.  Reorganizing
code like this at v8 of a series this size is a big "no-no" unless there's a
*really* good reason to do so.  In addition to the resulting confusion, changes
like this invalidate Fuad's Reviewed-by.  And since it's obviously quite difficult
to tease out exactly what changed, it's not realistic to re-review things without
doing a deep audit of the series, which no one wants to do for a series that is/was
so close to being fully ready.  And without such an audit, I can't accept the
patches, because I can't trust that what I am accepting is what I and others have
reviewed.

So, except where there is/was a *need* to shuffle things around relative to v7,
I think we should revert back to the v7 ordering for v9.  And where there is a
need to rework things, each and every one of those needs to be explicitly
documented, because "Reshuffled the earlier commits" is grossly insufficient.

[1] https://lore.kernel.org/all/20260522-gmem-inplace-conversion-v7-3-2f0fae496530@google.com
[2] https://github.com/sean-jc/linux/commit/8a475b1bcf89f1cf776ed9ce7d6bb587aab0d421

^ permalink raw reply related

* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: David Laight @ 2026-07-01 17:00 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Kiryl Shutsemau, Dave Hansen, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, Andi Kleen,
	Dan Williams, Borys Tsyrulnikov, kvm, linux-coco, linux-kernel,
	stable, Kiryl Shutsemau (Meta)
In-Reply-To: <akUrORhAmRur-lHP@google.com>

On Wed, 1 Jul 2026 07:59:05 -0700
Sean Christopherson <seanjc@google.com> wrote:

> On Wed, Jul 01, 2026, Kiryl Shutsemau wrote:
> > From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> > 
> > KVM's instruction emulator has a small helper, assign_register(), that
> > writes a value into a sub-register with x86 partial-register-write
> > semantics: 1- and 2-byte writes leave the upper bits of the destination
> > untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> > the full register.
> > 
> > The TDX guest #VE handler needs the same logic for port I/O emulation
> > to get 32-bit zero-extension right.  Rather than copy-pasting the
> > helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
> > use it.
> > 
> > Add <asm/insn.h> to the header's includes so it builds standalone in
> > callers that have not pulled it in transitively.
> > 
> > No functional change.
> > 
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> > Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> > ---
> >  arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
> >  arch/x86/kvm/emulate.c           | 26 ++++----------------------
> >  2 files changed, 34 insertions(+), 22 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> > index 4733e9064ee5..0c87759816d3 100644
> > --- a/arch/x86/include/asm/insn-eval.h
> > +++ b/arch/x86/include/asm/insn-eval.h
> > @@ -9,6 +9,7 @@
> >  #include <linux/compiler.h>
> >  #include <linux/bug.h>
> >  #include <linux/err.h>
> > +#include <asm/insn.h>
> >  #include <asm/ptrace.h>
> >  
> >  #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> > @@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
> >  
> >  bool insn_is_nop(struct insn *insn);
> >  
> > +/*
> > + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> > + * or 2-byte write leaves the upper bits of the destination untouched; a
> > + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]  
> 
> The placement of the "(matching IN[BWL], MOV[BWL] etc.)" blurb is confusing.  I
> *think* you're trying to say this behavior matches that of MOVB, MOVW, and MOVL
> instruction mnemonics, but the blurb is buried in the snippet that specifically
> describes the 4-byte write behavior.
> 
> FWIW, I think giving examples does more harm than good, because the behavior isn't
> instruction specific, it's architectural behavior that applies to all writes to
> GPRs, as defined in "3.4.1.1 General-Purpose Registers in 64-Bit Mode".  E.g. for
> a MOV instruction that sign-extends a 32-bit immediate to a 64-bit registers, it's
> not that the instruction is exempt from the normal GPR semenatics, it's that the
> instruction performs a 64-bit write to the destination even though the source is
> only 32 bits.
> 
> And the B/W/L terminology isn't architectural, it's AT&T syntax.  E.g. trying
> to encode "movl" with NASM yields "error: instruction expected, found `movl dword'".
> Yes, the kernel uses AT&T syntax for assembly, but I think this helper should very
> explicitly document that it's emulating architectural behavior.
> 
> > + * etc.); an 8-byte write overwrites the full register.
> > + *
> > + * @reg need not be 8-byte aligned: KVM's instruction emulator points
> > + * into the middle of a register slot to address the high-byte
                 ^ it isn't really the 'middle'.

> > + * registers (AH, CH, DH, BH).  Use narrow stores for the sub-word
> > + * cases so that the access width matches @bytes.
> > + */
> > +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> > +{
> > +	switch (bytes) {
> > +	case 1:
> > +		*(u8 *)reg = (u8)val;
> > +		break;
> > +	case 2:
> > +		*(u16 *)reg = (u16)val;
> > +		break;
> > +	case 4:
> > +		*reg = (u32)val;  
> 
> IMO, it's worth keeping a short comment here, because even with the explanation
> above, I suspect most people will think the code is buggy.  E.g.
> 
> 		/* As above, zero-extend 4-byte writes on 64-bit CPUs. */
> 		*reg = (u32)val;

Or be even more specific and use '& 0xffffffff' rather than a cast.
Particularly since the casts of the RHS in the byte/short cases aren't
needed at all.

-- David

> 
> > +		break;
> > +	case 8:
> > +		*reg = val;
> > +		break;
> > +	}
> > +}  


^ permalink raw reply

* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Sean Christopherson @ 2026-07-01 17:12 UTC (permalink / raw)
  To: Binbin Wu
  Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
	x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
	Yan Zhao, Ackerley Tng
In-Reply-To: <323a9a1a-6cf9-4a68-b92a-867f497b3d34@linux.intel.com>

On Wed, Jul 01, 2026, Binbin Wu wrote:
> On 7/1/2026 5:37 AM, Sean Christopherson wrote:
> > Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
> > pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
> > consistent between TDX and SNP (for LAUNCH_UPDATE).  EOPNOTSUPP was chosen
> > to be a forward-looking error code for when guest_memfd supports in-place
> > conversion, but even when in-place conversion comes along, it's an awkward
> > error code as KVM is deliberately choosing to disallow virtual address '0',
> > which is technically a legal userspace address.  I.e. it's not so much a
> > lack of support as it is that KVM reserves address '0' to simplify KVM's
> > internal implementation.
> 
> Nit:
> Do you think it's worth calling this out in the documentation?

Yes, though that can be done separate since this series doesn't change ABI.
E.g. we can probably do it opportunistically as part of the in-place conversion
series?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox