* Re: [PATCH 02/15] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xiaoyao Li @ 2026-05-27 8:18 UTC (permalink / raw)
To: Xu Yilun
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <ahaeCGLPEfmYNFtC@yilunxu-OptiPlex-7050>
On 5/27/2026 3:32 PM, Xu Yilun wrote:
> On Wed, May 27, 2026 at 02:38:27PM +0800, Xiaoyao Li wrote:
>> On 5/27/2026 11:47 AM, Xu Yilun wrote:
>>>>> +static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
>>>>> +{
>>>>> + u64 *entries = page_to_virt(root);
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < nr_pages; i++)
>>>>> + clflush_cache_range(__va(entries[i]), PAGE_SIZE);
>>>>
>>>> Is the page flush only needed when CLFLUSH_BEFORE_ALLOC is true?
>>>>
>>>> If so, it inherits the same decision to always flush as what
>>>
>>> Yes it is basically the same as tdx_clflush_page().
>>>
>>>> tdx_clflush_page() did. Then, any chance we can use tdx_clflush_page() here
>>>
>>> But I don't think we should convert hpa/page/va back and forth just for
>>> re-using one line of code.
>>
>> Because we want/need to flush page as late as possible so that the page
>> flush needs to happen right before SEAMCALL?
>
> I think so. Let the flushing be part of the tdh call semantic.
>
>>
>> How about we pass in the struct page * and number into tdx_ext_mem_add() and
>> construct the root page inside it?
>
> I assume you don't suggest allocate root page inside the call, then we
> need 3 parameters for the HPA_LIST_INFO:
>
> struct page *, unsigned int nr_pages, struct page *root
>
> which I think too much.
yeah, sort of.
> I think your concern is to try not to introduce another tdx_clflush_
> variant, but I believe this will happen, pfn based memory description is
> on the way:
>
> https://lore.kernel.org/all/20260430014929.24210-1-yan.y.zhao@intel.com/
I don't object the variant of tdx_clflush_hpa_list(), but suggest if
tdx_clflush_page() can be used instead of raw clflush_cache_range()
Maybe we can try to put tdx_clflush_hpa_list() along with
tdx_clflush_page() and tdx_clflush_pfn()? This way, I think we can save
the separate comment.
^ permalink raw reply
* Re: [PATCH 02/15] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xu Yilun @ 2026-05-27 7:32 UTC (permalink / raw)
To: Xiaoyao Li
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <9073ac91-3aa4-41e2-bb81-8878409498e5@intel.com>
On Wed, May 27, 2026 at 02:38:27PM +0800, Xiaoyao Li wrote:
> On 5/27/2026 11:47 AM, Xu Yilun wrote:
> > > > +static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
> > > > +{
> > > > + u64 *entries = page_to_virt(root);
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < nr_pages; i++)
> > > > + clflush_cache_range(__va(entries[i]), PAGE_SIZE);
> > >
> > > Is the page flush only needed when CLFLUSH_BEFORE_ALLOC is true?
> > >
> > > If so, it inherits the same decision to always flush as what
> >
> > Yes it is basically the same as tdx_clflush_page().
> >
> > > tdx_clflush_page() did. Then, any chance we can use tdx_clflush_page() here
> >
> > But I don't think we should convert hpa/page/va back and forth just for
> > re-using one line of code.
>
> Because we want/need to flush page as late as possible so that the page
> flush needs to happen right before SEAMCALL?
I think so. Let the flushing be part of the tdh call semantic.
>
> How about we pass in the struct page * and number into tdx_ext_mem_add() and
> construct the root page inside it?
I assume you don't suggest allocate root page inside the call, then we
need 3 parameters for the HPA_LIST_INFO:
struct page *, unsigned int nr_pages, struct page *root
which I think too much.
I think your concern is to try not to introduce another tdx_clflush_
variant, but I believe this will happen, pfn based memory description is
on the way:
https://lore.kernel.org/all/20260430014929.24210-1-yan.y.zhao@intel.com/
^ permalink raw reply
* Re: [RFC PATCH 14/15] x86/virt/tdx: Embed version info in SEAMCALL leaf function definitions
From: Xiaoyao Li @ 2026-05-27 7:44 UTC (permalink / raw)
To: Xu Yilun
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <ahaTH+O8YGxEXSpz@yilunxu-OptiPlex-7050>
On 5/27/2026 2:45 PM, Xu Yilun wrote:
>>> /*
>>> * TDX module SEAMCALL leaf functions
>>> */
>>> @@ -31,7 +44,7 @@
>>> #define TDH_VP_CREATE 10
>>> #define TDH_MNG_KEY_FREEID 20
>>> #define TDH_MNG_INIT 21
>>> -#define TDH_VP_INIT 22
>>> +#define TDH_VP_INIT SEAMCALL_LEAF_VER(22, 1)
>>
>> how about
>>
>> #define TDH_VP_INIT 22
>> #define TDH_VP_INIT_V1 SEAMCALL_LEAF_VER(TDH_VP_INIT, 1)
>>
>> and use TDH_VP_INIT_V1 below?
>
> I'm trying to avoid a _Vx postfix if unnecessary. Don't make callers
> have to choose between versions. The main MACRO should always point to
> the latest version since later versions are backward compatible.
I don't agree.
The later versions are backwards compatible, but the later versions
might not be supported by the loaded TDX module.
Usually the callers will have to choose between versions due to the TDX
module being used varies, just like the case in the next patch.
We can make TDH_VP_INIT represent the v1 as this patch because Linux
mandates v1 when the code was merged. So it can be made the default.
> The next patch is an exception. I've found there is no public TDX Module
> release available for TDH.SYS.CONFIG v1. I expect people just use the
> un-versioned MACRO for development, but have to keep the explicitly
> versioned _V0 macro for compatibility for now.
^ permalink raw reply
* Re: [PATCH 01/15] x86/virt/tdx: Read global metadata for TDX Module Extensions
From: Xu Yilun @ 2026-05-27 7:11 UTC (permalink / raw)
To: Sohil Mehta
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, yilun.xu, baolu.lu, zhenzhong.duan, xiaoyao.li
In-Reply-To: <b25e03ad-0bf4-482f-86ec-eebc6ac03d95@intel.com>
On Tue, May 26, 2026 at 11:05:48PM -0700, Sohil Mehta wrote:
> On 5/21/2026 8:41 PM, Xu Yilun wrote:
> > Add reading of the global metadata for TDX Module Extensions.
> >
> > TDX Module Extensions is an add-on feature enumerated by TDX_FEATURES0.
> > But for the Module's integrity, Linux requires that all features that a
> > Module advertises must have a complete, valid set of metadata, and the
> > validation must succeed at core TDX initialization time.
> >
> > Check TDX_FEATURES0 before reading these metadata. If a feature is
> > advertised, a failure in reading associated metadata causes the entire
> > TDX initialization to fail, otherwise skip.
> >
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > ---
> > arch/x86/include/asm/tdx_global_metadata.h | 6 ++++++
> > arch/x86/virt/vmx/tdx/tdx.h | 1 +
> > arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
>
> The top comments in tdx_global_metadata.h and tdx_global_metadata.c say
> that these files are autogenerated. I believe the script lives outside
> the tree. Is there a plan to merge the script?
No, the plan of auto-generating is deprecated. Now we switch to manual
update.
>
> The generated code is optimized for space instead of readability. Also,
> I see odd uncommented assignments u64 => u8/u16 all over the file. I am
> assuming the upper bits are expected to be zero.
>
> The patch is hard to review without the script. Can you post a link to
Yes, it is. A new plan is to refactor the file in future.
> the updated script that led to this patch?
>
>
> > 3 files changed, 23 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> > index 40689c8dc67e..533afe50a3f1 100644
> > --- a/arch/x86/include/asm/tdx_global_metadata.h
> > +++ b/arch/x86/include/asm/tdx_global_metadata.h
> > @@ -40,12 +40,18 @@ struct tdx_sys_info_td_conf {
> > u64 cpuid_config_values[128][2];
> > };
> >
> > +struct tdx_sys_info_ext {
> > + u16 memory_pool_required_pages;
> > + u8 ext_required;
>
> The name ext_required seems like a boolean. It is also used like a
> boolean later.
> if (!tdx_sysinfo.ext.ext_required)
> return 0;
>
> But, IIUC, is it actually a mask that lists any feature that needs
No it is just a bool about Extentions needs to be initialized or not.
> extensions to work correctly? If so, it would be good to give it a name
> that reflects its usage. Maybe:
> features_requiring_ext or something better
>
> As Xiaoyao mentioned, the struct requires a better explanation in the
> commit log.
Will do. I also plan to change the patch organization: instead of the
old auto-generated patch splitting style, I will switch to a human-readable
style and fold these metadata readings directly into the patches that
actually use them (e.g., DPAMT and TDX Runtime Update).
^ permalink raw reply
* Re: [RFC PATCH 14/15] x86/virt/tdx: Embed version info in SEAMCALL leaf function definitions
From: Xu Yilun @ 2026-05-27 6:45 UTC (permalink / raw)
To: Xiaoyao Li
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <90a4835f-bc11-4415-a7b6-84347f40861b@intel.com>
> > /*
> > * TDX module SEAMCALL leaf functions
> > */
> > @@ -31,7 +44,7 @@
> > #define TDH_VP_CREATE 10
> > #define TDH_MNG_KEY_FREEID 20
> > #define TDH_MNG_INIT 21
> > -#define TDH_VP_INIT 22
> > +#define TDH_VP_INIT SEAMCALL_LEAF_VER(22, 1)
>
> how about
>
> #define TDH_VP_INIT 22
> #define TDH_VP_INIT_V1 SEAMCALL_LEAF_VER(TDH_VP_INIT, 1)
>
> and use TDH_VP_INIT_V1 below?
I'm trying to avoid a _Vx postfix if unnecessary. Don't make callers
have to choose between versions. The main MACRO should always point to
the latest version since later versions are backward compatible.
The next patch is an exception. I've found there is no public TDX Module
release available for TDH.SYS.CONFIG v1. I expect people just use the
un-versioned MACRO for development, but have to keep the explicitly
versioned _V0 macro for compatibility for now.
^ permalink raw reply
* RE: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Tian, Kevin @ 2026-05-27 6:56 UTC (permalink / raw)
To: Dan Williams (nvidia), Alexey Kardashevskiy,
Aneesh Kumar K.V (Arm), linux-coco@lists.linux.dev,
iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org
Cc: Bjorn Helgaas, Jason Gunthorpe, Joerg Roedel, Jonathan Cameron,
Nicolin Chen, Samuel Ortiz, Steven Price, Suzuki K Poulose,
Will Deacon, Xu Yilun, Shameer Kolothum, Paolo Bonzini,
Tony Krowiak, Halil Pasic, Jason Herne, Harald Freudenberger,
Holger Dengler, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Alex Williamson,
Matthew Rosato, Farhan Ali, Eric Farman,
linux-s390@vger.kernel.org
In-Reply-To: <6a168c8ea7d10_2129b2100e@djbw-dev.notmuch>
> From: Dan Williams (nvidia) <djbw@kernel.org>
> Sent: Wednesday, May 27, 2026 2:18 PM
>
> Alexey Kardashevskiy wrote:
> >
> >
> > On 26/5/26 01:48, Aneesh Kumar K.V (Arm) wrote:
> > > +static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> > > +{
> > > + if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> > > + return false;
> > > +
> > > + switch (scope) {
> > > + case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> > > + case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> > > + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> > > + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
> >
> > This scope thing still needs clarification.
> >
> > I have 3 types of requests to fit here, all go via VM -> KVM -> QEMU ->
> IOMMUFD -> TSM.
> >
> > 1) bind/unbind TDI <- moves to CONFIG_LOCKED, this is "OP";
> > 2) start/stop TDI <- moves to RUN, this is "GR"? Right now I route it via "OP";
> > 3) enable/disable MMIO/DMA <- no TDI state change, this is "GR" but
> which scope is it here?
>
> The scope parameter was meant to enumerate a security model for classes
> of commands that are otherwise opaque to the kernel. However, none of
> the commands we are targeting are opaque (private specification with
> unknown effect). It now turns out there is no role for @scope for
> security.
yeah, I haven't succeeded on figuring out that role for now. It sounds an
unnecessary abstraction asking vendor specific code to translate its
command into opaque then in the end we go back to the vendor code
to decide the security scope of that opaque.
[...]
> ...or just observe that per CC arch commands are needed to setup the VM
> so per CC arch commands are needed to marshal device assignment support
> requests.
>
> In that case pci_tsm_req_scope becomes tsm_req_type and is just:
>
> TSM_REQ_TYPE_CCA
> TSM_REQ_TYPE_SEV
> TSM_REQ_TYPE_TDX
>
> I am leaning towards the latter at this point.
+1
^ permalink raw reply
* Re: [PATCH 02/15] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xiaoyao Li @ 2026-05-27 6:38 UTC (permalink / raw)
To: Xu Yilun
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <ahZpUA62NRdNrkvZ@yilunxu-OptiPlex-7050>
On 5/27/2026 11:47 AM, Xu Yilun wrote:
>>> +static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
>>> +{
>>> + u64 *entries = page_to_virt(root);
>>> + int i;
>>> +
>>> + for (i = 0; i < nr_pages; i++)
>>> + clflush_cache_range(__va(entries[i]), PAGE_SIZE);
>>
>> Is the page flush only needed when CLFLUSH_BEFORE_ALLOC is true?
>>
>> If so, it inherits the same decision to always flush as what
>
> Yes it is basically the same as tdx_clflush_page().
>
>> tdx_clflush_page() did. Then, any chance we can use tdx_clflush_page() here
>
> But I don't think we should convert hpa/page/va back and forth just for
> re-using one line of code.
Because we want/need to flush page as late as possible so that the page
flush needs to happen right before SEAMCALL?
How about we pass in the struct page * and number into tdx_ext_mem_add()
and construct the root page inside it?
>> so that we have a single central place of the comment to explain the kernel
>> design decision.
>
> How about I add a comment here to connect this wrapper to
> tdx_clflush_page():
>
> /*
> * Unconditionally flush the pages regardless of CLFLUSH_BEFORE_ALLOC. Inherit
> * the same decision as tdx_clflush_page().
> */
> static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
> ...
It works either. I don't have strong preference. Let's see if anyone
else say something about it.
^ permalink raw reply
* Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Dan Williams (nvidia) @ 2026-05-27 6:17 UTC (permalink / raw)
To: Alexey Kardashevskiy, Aneesh Kumar K.V (Arm), linux-coco, iommu,
linux-kernel, kvm
Cc: Bjorn Helgaas, Dan Williams, Jason Gunthorpe, Joerg Roedel,
Jonathan Cameron, Kevin Tian, Nicolin Chen, Samuel Ortiz,
Steven Price, Suzuki K Poulose, Will Deacon, Xu Yilun,
Shameer Kolothum, Paolo Bonzini, Tony Krowiak, Halil Pasic,
Jason Herne, Harald Freudenberger, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Alex Williamson, Matthew Rosato, Farhan Ali,
Eric Farman, linux-s390
In-Reply-To: <becd865d-09a4-4ac3-b719-4a0deae2692a@amd.com>
Alexey Kardashevskiy wrote:
>
>
> On 26/5/26 01:48, Aneesh Kumar K.V (Arm) wrote:
> > Add IOMMU_VDEVICE_TSM_REQUEST for issuing TSM guest request/response
> > transactions against an iommufd vdevice.
> >
> > The ioctl takes a vdevice_id plus request/response user buffers and length
> > fields, and forwards the request through tsm_guest_req() to the PCI TSM
> > backend. This provides the host-side passthrough path used by CoCo guests
> > for TSM device attestation and acceptance flows after the device has been
> > bound to TSM.
> >
> > Also add the supporting tsm_guest_req() helper and associated TSM core
> > interface definitions.
> >
> > Based on changes from: Alexey Kardashevskiy <aik@amd.com>
> >
> > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> > ---
> > drivers/iommu/iommufd/iommufd_private.h | 6 ++
> > drivers/iommu/iommufd/main.c | 3 +
> > drivers/iommu/iommufd/tsm.c | 68 +++++++++++++++++++++
> > drivers/virt/coco/tsm-core.c | 39 ++++++++++++
> > include/linux/pci-tsm.h | 9 +--
> > include/linux/tsm.h | 25 ++++++++
> > include/uapi/linux/iommufd.h | 80 +++++++++++++++++++++++++
> > 7 files changed, 226 insertions(+), 4 deletions(-)
[..]
> > diff --git a/drivers/iommu/iommufd/tsm.c b/drivers/iommu/iommufd/tsm.c
> > index 09ee668dbed9..342fbdb6a6b9 100644
> > --- a/drivers/iommu/iommufd/tsm.c
> > +++ b/drivers/iommu/iommufd/tsm.c
> > @@ -60,3 +60,71 @@ int iommufd_vdevice_tsm_op_ioctl(struct iommufd_ucmd *ucmd)
> > iommufd_put_object(ucmd->ictx, &vdev->obj);
> > return rc;
> > }
> > +
> > +static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> > +{
> > + if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> > + return false;
> > +
> > + switch (scope) {
> > + case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> > + case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> > + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> > + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
>
> This scope thing still needs clarification.
>
> I have 3 types of requests to fit here, all go via VM -> KVM -> QEMU -> IOMMUFD -> TSM.
>
> 1) bind/unbind TDI <- moves to CONFIG_LOCKED, this is "OP";
> 2) start/stop TDI <- moves to RUN, this is "GR"? Right now I route it via "OP";
> 3) enable/disable MMIO/DMA <- no TDI state change, this is "GR" but which scope is it here?
The scope parameter was meant to enumerate a security model for classes
of commands that are otherwise opaque to the kernel. However, none of
the commands we are targeting are opaque (private specification with
unknown effect). It now turns out there is no role for @scope for
security.
Now a command family that iommufd can validate seems useful. As it
stands this implementation aliases command codes across TSMs. Do we
proceed with creating an actual shared command uapi for the truly shared
commands:
TSM_REQ_TYPE_DEFAULT: Commands every arch needs
TSM_REQ_READ_OBJECT
TSM_REQ_REGEN_OBJECT
TSM_REQ_OBJECT_INFO
TSM_REQ_VALIDATE_MMIO
TSM_REQ_SET_TDI_STATE
TSM_REQ_TYPE_SEV: Commands only SEV needs
TSM_REQ_SEV_ENABLE_DMA
TSM_REQ_SEV_DISABLE_DMA
...or just observe that per CC arch commands are needed to setup the VM
so per CC arch commands are needed to marshal device assignment support
requests.
In that case pci_tsm_req_scope becomes tsm_req_type and is just:
TSM_REQ_TYPE_CCA
TSM_REQ_TYPE_SEV
TSM_REQ_TYPE_TDX
I am leaning towards the latter at this point.
^ permalink raw reply
* Re: [PATCH 01/15] x86/virt/tdx: Read global metadata for TDX Module Extensions
From: Sohil Mehta @ 2026-05-27 6:05 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, yilun.xu, baolu.lu, zhenzhong.duan,
xiaoyao.li
In-Reply-To: <20260522034128.3144354-2-yilun.xu@linux.intel.com>
On 5/21/2026 8:41 PM, Xu Yilun wrote:
> Add reading of the global metadata for TDX Module Extensions.
>
> TDX Module Extensions is an add-on feature enumerated by TDX_FEATURES0.
> But for the Module's integrity, Linux requires that all features that a
> Module advertises must have a complete, valid set of metadata, and the
> validation must succeed at core TDX initialization time.
>
> Check TDX_FEATURES0 before reading these metadata. If a feature is
> advertised, a failure in reading associated metadata causes the entire
> TDX initialization to fail, otherwise skip.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx_global_metadata.h | 6 ++++++
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
The top comments in tdx_global_metadata.h and tdx_global_metadata.c say
that these files are autogenerated. I believe the script lives outside
the tree. Is there a plan to merge the script?
The generated code is optimized for space instead of readability. Also,
I see odd uncommented assignments u64 => u8/u16 all over the file. I am
assuming the upper bits are expected to be zero.
The patch is hard to review without the script. Can you post a link to
the updated script that led to this patch?
> 3 files changed, 23 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 40689c8dc67e..533afe50a3f1 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -40,12 +40,18 @@ struct tdx_sys_info_td_conf {
> u64 cpuid_config_values[128][2];
> };
>
> +struct tdx_sys_info_ext {
> + u16 memory_pool_required_pages;
> + u8 ext_required;
The name ext_required seems like a boolean. It is also used like a
boolean later.
if (!tdx_sysinfo.ext.ext_required)
return 0;
But, IIUC, is it actually a mask that lists any feature that needs
extensions to work correctly? If so, it would be good to give it a name
that reflects its usage. Maybe:
features_requiring_ext or something better
As Xiaoyao mentioned, the struct requires a better explanation in the
commit log.
> +};
...
> static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> {
> int ret = 0;
> @@ -116,5 +129,8 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>
> + if (sysinfo->features.tdx_features0 & TDX_FEATURES0_EXT)
Other metadata reads aren't gated on feature checking. Is this check
manually added or autogenerated. If manually added, it should have a
code comment clarifying that.
> + ret = ret ?: get_tdx_sys_info_ext(&sysinfo->ext);
> +
> return ret;
> }
^ permalink raw reply
* Re: [PATCH 00/15] Enable TDX Module Extensions and DICE-based TDX Quoting
From: Sohil Mehta @ 2026-05-27 5:23 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, yilun.xu, baolu.lu, zhenzhong.duan,
xiaoyao.li
In-Reply-To: <20260522034128.3144354-1-yilun.xu@linux.intel.com>
Hello,
On 5/21/2026 8:41 PM, Xu Yilun wrote:
> The first 4 patches will eventually need an ack by an x86 maintainer, so
> please review with that in mind.
>
I am looking at this from an x86 reviewer perspective with limited prior
TDX knowledge.
> == Overview ==
>
> TDX Module introduces the "TDX Module Extensions" to support long
> running / hard-irq preemptible flows inside. This makes TDX Module
> capable of handling complex tasks through "Extension SEAMCALLs".
Can we explain a bit more about why these extensions are needed or what
would happen if the kernel didn't enable them? I ran the series through
an LLM for my curiosity. I think something on the below lines might be a
good addition for the cover letter itself.
(Please verify)
The TDX module's normal SEAMCALLs are designed to be short,
non-preemptible operations. However, some newer features (like
DICE-based TDX Quoting) require complex, potentially long-running
computations that can't complete within the tight constraints of a
single non-preemptible SEAMCALL.
The "TDX Module Extensions" solve this by introducing "Extension
SEAMCALLs" — a new class of SEAMCALLs that are:
* Long-running — they may take significant time to complete (e.g.,
cryptographic operations for attestation/quoting).
* Hard-IRQ preemptible — they can be interrupted by hardware interrupts
and later resumed, so they don't monopolize the CPU or cause
unacceptable interrupt latency.
Without this mechanism, complex operations like generating DICE
attestation quotes would either block interrupts for too long
(unacceptable for a host kernel) or wouldn't be possible inside the TDX
module at all. The Extensions give the TDX module a way to handle these
heavyweight tasks while remaining cooperative with the host's
interrupt/scheduling model.
>
> TDX Module allows some add-on features to use the Extension.
s/Module/module throughout the series.
The existing kernel code predominantly uses the lower case TDX "module".
> The first feature to use Extensions is DICE-based TDX Quoting [1].
> DICE is an industry-standard, certificate-backed attestation
> framework that layers evidence through a chain of certificates.
>
> This series adds infrastructure to enable the Extensions and then
> implement DICE-based TDX Quoting.
>
> The Extensions consumes relatively large amount of memory (~50MB). So it
> is designed to be off by default. It must be enabled after basic TDX
> Module initialization and when add-on features require it. To enable
> the Extensions, host first adds extra memory to TDX Module via a
> SEAMCALL (TDH.EXT.MEM.ADD), then uses another SEAMCALL (TDH.EXT.INIT) to
> initialize Extensions, and then some add-on features, e.g. DICE, could
> use Extension SEAMCALLs for work. Note that host can never get the added
> memory back.
>
> Theoretically, the Extensions doesn't need to be enabled right after
> basic TDX initialization. It could be enabled right before the first
> Extension SEAMCALL is issued. That would save or postpone memory usage.
> But it isn't worth the complexity, the needs for the Extensions are vast
> but the savings are little for a typical TDX capable system (about
> 0.001% of memory). So the Linux decision is to just enable it along with
> the basic TDX.
>
I think enabling it by default on TDX platforms (with the module
extension) might make sense. But the explanation here is slightly
confusing.
You said earlier that "The Extensions consumes relatively large amount
of memory (~50MB)" so they must be off by default. Later you say that
"..the saving are little .."
Are you saying that the dynamic enabling of the extensions is not worth
it or the dynamic allocation of the memory needed to support them?
In addition, could you briefly describe the complexity we are trading off?
> This series has 2 distinct parts:
>
> Patches 1-4: TDX Module Extensions enabling
> Patches 5-15: DICE-based TDX Quoting, primarily Peter's work.
>
^ permalink raw reply
* Re: [PATCH 04/15] x86/virt/tdx: Enable the Extensions right after basic TDX Module init
From: Xu Yilun @ 2026-05-27 4:02 UTC (permalink / raw)
To: Tony Lindgren
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <ahPlgBgvIwimzmzH@tlindgre-MOBL1>
On Mon, May 25, 2026 at 09:00:32AM +0300, Tony Lindgren wrote:
> On Fri, May 22, 2026 at 11:41:17AM +0800, Xu Yilun wrote:
> > The detailed initialization flow for TDX Module Extensions has been
> > fully implemented. Enable the flow after basic TDX Module
> > initialization.
> >
> > Theoretically, the Extensions doesn't need to be enabled right after
> > basic TDX initialization. It could be enabled right before the first
> > Extension SEAMCALL is issued. That would save or postpone memory usage.
> > But it isn't worth the complexity, the needs for the Extensions are vast
> > but the savings are little for a typical TDX capable system (about
> > 0.001% of memory). So the Linux decision is to just enable it along with
> > the basic TDX.
> >
> > Note that the Extensions initialization flow will still not start if no
> > add-on features require Extensions. The enabling of add-on features will
> > be in later patches. Until then, the system hasn't consumed extra memory.
>
> Looking at patch 15/15, we need to reload the TDX module metadata at least
> for the attestation. We need to do that early, so to me it seems that
> everything can be just tagged __init from the start.
I'm good to it. The Extension initialization will not start without
add-on features anyway. Let me move the patch as the first one to avoid
tag churn.
^ permalink raw reply
* Re: [PATCH 02/15] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xu Yilun @ 2026-05-27 3:47 UTC (permalink / raw)
To: Xiaoyao Li
Cc: kas, djbw, rick.p.edgecombe, x86, peter.fang, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <7139c55b-b949-415d-ab82-fca1b1cc3880@intel.com>
> > +static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
> > +{
> > + u64 *entries = page_to_virt(root);
> > + int i;
> > +
> > + for (i = 0; i < nr_pages; i++)
> > + clflush_cache_range(__va(entries[i]), PAGE_SIZE);
>
> Is the page flush only needed when CLFLUSH_BEFORE_ALLOC is true?
>
> If so, it inherits the same decision to always flush as what
Yes it is basically the same as tdx_clflush_page().
> tdx_clflush_page() did. Then, any chance we can use tdx_clflush_page() here
But I don't think we should convert hpa/page/va back and forth just for
re-using one line of code.
> so that we have a single central place of the comment to explain the kernel
> design decision.
How about I add a comment here to connect this wrapper to
tdx_clflush_page():
/*
* Unconditionally flush the pages regardless of CLFLUSH_BEFORE_ALLOC. Inherit
* the same decision as tdx_clflush_page().
*/
static void tdx_clflush_hpa_list(struct page *root, unsigned int nr_pages)
...
^ permalink raw reply
* Re: [RFC PATCH 15/15] x86/virt/tdx: Enable TDX Quoting extension
From: Xiaoyao Li @ 2026-05-27 1:30 UTC (permalink / raw)
To: Xu Yilun
Cc: Tony Lindgren, kas, djbw, rick.p.edgecombe, x86, peter.fang,
linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan
In-Reply-To: <ahXAL41ZmIDHmgfu@yilunxu-OptiPlex-7050>
On 5/26/2026 11:45 PM, Xu Yilun wrote:
> On Mon, May 25, 2026 at 06:51:27PM +0800, Xiaoyao Li wrote:
>> On 5/25/2026 1:17 PM, Tony Lindgren wrote:
>>> On Fri, May 22, 2026 at 11:41:28AM +0800, Xu Yilun wrote:
>>>> From: Peter Fang <peter.fang@intel.com>
>>>>
>>>> TDX Module updates global metadata when add-on features are enabled.
>>>> Host should update the cached tdx_sysinfo to reflect these changes.
>>>
>>> This should be made clearer IMO. How about mention that get_tdx_sys_info()
>>> needs to get called again to reload the TDX module global metadata?
>>
>> Ah ha! This patch answers my comment to patch 1:
>> https://lore.kernel.org/all/956fa1e6-2920-4b2e-8037-d4b9d812ae53@intel.com/
>>
>> sysinfo_ext->memory_pool_required_pages and sysinfo_ext->ext_required will
>> be updated after extensions are enabled by TDH.SYS.CONFIG.
>>
>> Patch 06 in this series already reads the tdx_sys_info_quote out of
>> get_tdx_sys_info(), which mean get_tdx_sys_info() doesn't ensure all the
>> global metadata will be update again.
>>
>> So how about move the read of memory_pool_required_pages and ext_required
>> out of get_tdx_sys_info() and put them after TDH.SYS.CONFIG, so that we
>> don't need call get_tdx_sys_info() again?
>
> Yes, I'm good to it. I hesitated to move them out in case we need some
> central control on global data. But now I see there is already a
> precedent:
>
> https://lore.kernel.org/kvm/20260520133909.409394-22-chao.gao@intel.com/
>
> Once we've agreed on moving add-on data reading out of get_tdx_sys_info(),
> we don't have to read them after TDH.SYS.CONFIG, read them when really
> needed. How about the following, that makes the Extension part in this
> series self-contained.
Actually below is what I meant after TDH.SYS.CONFIG.
And I think we can re-order the patches of enabling TDX extensions by
moving the patch 04 as the first one.
> ----8<----
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 86e5b7ad19b3..b729c1f5ab9e 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1536,6 +1536,10 @@ static __init int init_tdx_ext(void)
> if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> return 0;
>
> + ret = get_tdx_sys_info_ext(&tdx_sysinfo.ext);
> + if (ret)
> + return ret;
> +
> /* No feature requires TDX Module Extensions. */
> if (!tdx_sysinfo.ext.ext_required)
> return 0;
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index f9cc2dd02caf..e7d9e0c4b604 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -140,8 +140,5 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>
> - if (sysinfo->features.tdx_features0 & TDX_FEATURES0_EXT)
> - ret = ret ?: get_tdx_sys_info_ext(&sysinfo->ext);
> -
> return ret;
> }
^ permalink raw reply
* Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl
From: Alexey Kardashevskiy @ 2026-05-27 0:16 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-coco, iommu, linux-kernel, kvm
Cc: Bjorn Helgaas, Dan Williams, Jason Gunthorpe, Joerg Roedel,
Jonathan Cameron, Kevin Tian, Nicolin Chen, Samuel Ortiz,
Steven Price, Suzuki K Poulose, Will Deacon, Xu Yilun,
Shameer Kolothum, Paolo Bonzini, Tony Krowiak, Halil Pasic,
Jason Herne, Harald Freudenberger, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Alex Williamson, Matthew Rosato, Farhan Ali,
Eric Farman, linux-s390
In-Reply-To: <20260525154816.1029642-6-aneesh.kumar@kernel.org>
On 26/5/26 01:48, Aneesh Kumar K.V (Arm) wrote:
> Add IOMMU_VDEVICE_TSM_REQUEST for issuing TSM guest request/response
> transactions against an iommufd vdevice.
>
> The ioctl takes a vdevice_id plus request/response user buffers and length
> fields, and forwards the request through tsm_guest_req() to the PCI TSM
> backend. This provides the host-side passthrough path used by CoCo guests
> for TSM device attestation and acceptance flows after the device has been
> bound to TSM.
>
> Also add the supporting tsm_guest_req() helper and associated TSM core
> interface definitions.
>
> Based on changes from: Alexey Kardashevskiy <aik@amd.com>
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> drivers/iommu/iommufd/iommufd_private.h | 6 ++
> drivers/iommu/iommufd/main.c | 3 +
> drivers/iommu/iommufd/tsm.c | 68 +++++++++++++++++++++
> drivers/virt/coco/tsm-core.c | 39 ++++++++++++
> include/linux/pci-tsm.h | 9 +--
> include/linux/tsm.h | 25 ++++++++
> include/uapi/linux/iommufd.h | 80 +++++++++++++++++++++++++
> 7 files changed, 226 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index 8eea0c2c332b..0080895e9e92 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -701,11 +701,17 @@ int iommufd_hw_queue_alloc_ioctl(struct iommufd_ucmd *ucmd);
> void iommufd_hw_queue_destroy(struct iommufd_object *obj);
> #ifdef CONFIG_TSM
> int iommufd_vdevice_tsm_op_ioctl(struct iommufd_ucmd *ucmd);
> +int iommufd_vdevice_tsm_req_ioctl(struct iommufd_ucmd *ucmd);
> #else
> static inline int iommufd_vdevice_tsm_op_ioctl(struct iommufd_ucmd *ucmd)
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline int iommufd_vdevice_tsm_req_ioctl(struct iommufd_ucmd *ucmd)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
>
> static inline struct iommufd_vdevice *
> diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
> index d73e6b391c6f..5f49b546ec92 100644
> --- a/drivers/iommu/iommufd/main.c
> +++ b/drivers/iommu/iommufd/main.c
> @@ -433,6 +433,7 @@ union ucmd_buffer {
> struct iommu_vfio_ioas vfio_ioas;
> struct iommu_viommu_alloc viommu;
> struct iommu_vdevice_tsm_op tsm_op;
> + struct iommu_vdevice_tsm_req tsm_req;
> #ifdef CONFIG_IOMMUFD_TEST
> struct iommu_test_cmd test;
> #endif
> @@ -496,6 +497,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = {
> struct iommu_viommu_alloc, out_viommu_id),
> IOCTL_OP(IOMMU_VDEVICE_TSM_OP, iommufd_vdevice_tsm_op_ioctl,
> struct iommu_vdevice_tsm_op, vdevice_id),
> + IOCTL_OP(IOMMU_VDEVICE_TSM_REQ, iommufd_vdevice_tsm_req_ioctl,
> + struct iommu_vdevice_tsm_req, resp_uptr),
> #ifdef CONFIG_IOMMUFD_TEST
> IOCTL_OP(IOMMU_TEST_CMD, iommufd_test, struct iommu_test_cmd, last),
> #endif
> diff --git a/drivers/iommu/iommufd/tsm.c b/drivers/iommu/iommufd/tsm.c
> index 09ee668dbed9..342fbdb6a6b9 100644
> --- a/drivers/iommu/iommufd/tsm.c
> +++ b/drivers/iommu/iommufd/tsm.c
> @@ -60,3 +60,71 @@ int iommufd_vdevice_tsm_op_ioctl(struct iommufd_ucmd *ucmd)
> iommufd_put_object(ucmd->ictx, &vdev->obj);
> return rc;
> }
> +
> +static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> +{
> + if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> + return false;
> +
> + switch (scope) {
> + case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> + case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
This scope thing still needs clarification.
I have 3 types of requests to fit here, all go via VM -> KVM -> QEMU -> IOMMUFD -> TSM.
1) bind/unbind TDI <- moves to CONFIG_LOCKED, this is "OP";
2) start/stop TDI <- moves to RUN, this is "GR"? Right now I route it via "OP";
3) enable/disable MMIO/DMA <- no TDI state change, this is "GR" but which scope is it here?
thanks,
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +/**
> + * iommufd_vdevice_tsm_req_ioctl - Forward TSM requests
> + * @ucmd: user command data for IOMMU_VDEVICE_TSM_REQ
> + *
> + * Resolve @iommu_vdevice_tsm_req::vdevice_id to a vdevice and pass the
> + * request/response buffers to the TSM core.
> + *
> + * Return:
> + * -errno on error.
> + * positive residue if response/request bytes were left unconsumed.
> + * if response buffer is provided, residue indicates the number of bytes
> + * not used in response buffer
> + * if there is no response buffer, residue indicates the number of bytes
> + * not consumed in req buffer
> + * 0 otherwise.
> + */
> +int iommufd_vdevice_tsm_req_ioctl(struct iommufd_ucmd *ucmd)
> +{
> + int rc;
> + struct iommufd_vdevice *vdev;
> + struct iommu_vdevice_tsm_req *cmd = ucmd->cmd;
> + struct tsm_guest_req_info info = {
> + .scope = cmd->scope,
> + .req = {
> + .user = u64_to_user_ptr(cmd->req_uptr),
> + .is_kernel = false,
> + },
> + .req_len = cmd->req_len,
> + .resp = {
> + .user = u64_to_user_ptr(cmd->resp_uptr),
> + .is_kernel = false,
> + },
> + .resp_len = cmd->resp_len,
> + };
> +
> + if (cmd->__reserved)
> + return -EOPNOTSUPP;
> +
> + if (!iommufd_vdevice_tsm_req_scope_valid(cmd->scope))
> + return -EINVAL;
> +
> + vdev = iommufd_get_vdevice(ucmd->ictx, cmd->vdevice_id);
> + if (IS_ERR(vdev))
> + return PTR_ERR(vdev);
> +
> + rc = tsm_guest_req(vdev->idev->dev, &info);
> +
> + /* No inline response, hence we don't need to copy the response */
> + iommufd_put_object(ucmd->ictx, &vdev->obj);
> + return rc;
> +}
> diff --git a/drivers/virt/coco/tsm-core.c b/drivers/virt/coco/tsm-core.c
> index 3870d08ffe0d..c24886851f9e 100644
> --- a/drivers/virt/coco/tsm-core.c
> +++ b/drivers/virt/coco/tsm-core.c
> @@ -8,6 +8,7 @@
> #include <linux/module.h>
> #include <linux/cleanup.h>
> #include <linux/pci-tsm.h>
> +#include <uapi/linux/iommufd.h>
>
> static void tsm_release(struct device *);
> static const struct class tsm_class = {
> @@ -127,6 +128,44 @@ int tsm_unbind(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(tsm_unbind);
>
> +static int tsm_pci_req_scope(u32 scope, enum pci_tsm_req_scope *pci_scope)
> +{
> + switch (scope) {
> + case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> + *pci_scope = PCI_TSM_REQ_INFO;
> + return 0;
> + case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> + *pci_scope = PCI_TSM_REQ_STATE_CHANGE;
> + return 0;
> + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> + *pci_scope = PCI_TSM_REQ_DEBUG_READ;
> + return 0;
> + case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
> + *pci_scope = PCI_TSM_REQ_DEBUG_WRITE;
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +ssize_t tsm_guest_req(struct device *dev, struct tsm_guest_req_info *info)
> +{
> + int ret;
> + enum pci_tsm_req_scope pci_scope;
> +
> + if (!dev_is_pci(dev))
> + return -EINVAL;
> +
> + ret = tsm_pci_req_scope(info->scope, &pci_scope);
> + if (ret)
> + return ret;
> +
> + return pci_tsm_guest_req(to_pci_dev(dev), pci_scope, info->req,
> + info->req_len, info->resp, info->resp_len,
> + NULL);
> +}
> +EXPORT_SYMBOL_GPL(tsm_guest_req);
> +
> static void tsm_release(struct device *dev)
> {
> struct tsm_dev *tsm_dev = container_of(dev, typeof(*tsm_dev), dev);
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index a6435aba03f9..ec2236a7a279 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -4,6 +4,7 @@
> #include <linux/mutex.h>
> #include <linux/pci.h>
> #include <linux/sockptr.h>
> +#include <uapi/linux/iommufd.h>
>
> struct pci_tsm;
> struct tsm_dev;
> @@ -173,7 +174,7 @@ enum pci_tsm_req_scope {
> * typical TDISP collateral information like Device Interface Reports.
> * No device secrets are permitted, and no device state is changed.
> */
> - PCI_TSM_REQ_INFO = 0,
> + PCI_TSM_REQ_INFO = IOMMU_VDEVICE_TSM_REQ_PCI_INFO,
> /**
> * @PCI_TSM_REQ_STATE_CHANGE: Request to change the TDISP state from
> * UNLOCKED->LOCKED, LOCKED->RUN, or other architecture specific state
> @@ -181,14 +182,14 @@ enum pci_tsm_req_scope {
> * to TDISP) device / host state, configuration, or data change is
> * permitted.
> */
> - PCI_TSM_REQ_STATE_CHANGE = 1,
> + PCI_TSM_REQ_STATE_CHANGE = IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE,
> /**
> * @PCI_TSM_REQ_DEBUG_READ: Read-only request for debug information
> *
> * A method to facilitate TVM information retrieval outside of typical
> * TDISP operational requirements. No device secrets are permitted.
> */
> - PCI_TSM_REQ_DEBUG_READ = 2,
> + PCI_TSM_REQ_DEBUG_READ = IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ,
> /**
> * @PCI_TSM_REQ_DEBUG_WRITE: Device state changes for debug purposes
> *
> @@ -196,7 +197,7 @@ enum pci_tsm_req_scope {
> * the TDISP operational model. If allowed, requires CAP_SYS_RAW_IO, and
> * will taint the kernel.
> */
> - PCI_TSM_REQ_DEBUG_WRITE = 3,
> + PCI_TSM_REQ_DEBUG_WRITE = IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE,
> };
>
> #ifdef CONFIG_PCI_TSM
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 7b6df827321b..6101a2a1db61 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -6,6 +6,7 @@
> #include <linux/types.h>
> #include <linux/uuid.h>
> #include <linux/device.h>
> +#include <linux/sockptr.h>
>
> #define TSM_REPORT_INBLOB_MAX 64
> #define TSM_REPORT_OUTBLOB_MAX SZ_16M
> @@ -128,6 +129,23 @@ struct kvm;
> #ifdef CONFIG_TSM
> int tsm_bind(struct device *dev, struct kvm *kvm, u64 tdi_id);
> int tsm_unbind(struct device *dev);
> +
> +/**
> + * struct tsm_guest_req_info - parameter for tsm_guest_req()
> + * @scope: iommufd allocated scope for tsm guest request
> + * @req: request data buffer filled by guest
> + * @req_len: the size of @req filled by guest
> + * @resp: response data buffer filled by host
> + * @resp_len: the size of @resp buffer filled by guest
> + */
> +struct tsm_guest_req_info {
> + u32 scope;
> + sockptr_t req;
> + size_t req_len;
> + sockptr_t resp;
> + size_t resp_len;
> +};
> +ssize_t tsm_guest_req(struct device *dev, struct tsm_guest_req_info *info);
> #else
> static inline int tsm_bind(struct device *dev, struct kvm *kvm, u64 tdi_id)
> {
> @@ -138,6 +156,13 @@ static inline int tsm_unbind(struct device *dev)
> {
> return 0;
> }
> +
> +struct tsm_guest_req_info;
> +static inline ssize_t tsm_guest_req(struct device *dev,
> + struct tsm_guest_req_info *info)
> +{
> + return -EINVAL;
> +}
> #endif
>
> #endif /* __TSM_H */
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index 66398efa31d1..7953e99a9671 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -58,6 +58,7 @@ enum {
> IOMMUFD_CMD_VEVENTQ_ALLOC = 0x93,
> IOMMUFD_CMD_HW_QUEUE_ALLOC = 0x94,
> IOMMUFD_CMD_VDEVICE_TSM_OP = 0x95,
> + IOMMUFD_CMD_VDEVICE_TSM_REQ = 0x96,
> };
>
> /**
> @@ -1373,4 +1374,83 @@ struct iommu_hw_queue_alloc {
> __aligned_u64 length;
> };
> #define IOMMU_HW_QUEUE_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_HW_QUEUE_ALLOC)
> +
> +/*
> + * TSM request scope values are allocated by iommufd. Each device-bus transport
> + * gets a range from this number space.
> + */
> +#define IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_BASE 0
> +
> +enum iommu_vdevice_tsm_req_scope {
> + /*
> + * Read-only, without side effects, request for typical TDISP
> + * collateral information like Device Interface Reports. No device
> + * secrets are permitted, and no device state is changed.
> + */
> + IOMMU_VDEVICE_TSM_REQ_PCI_INFO =
> + IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_BASE,
> + /*
> + * Request to change the TDISP state from UNLOCKED->LOCKED,
> + * LOCKED->RUN, or other architecture specific state changes to
> + * support those transitions for a TDI. No other device or host state,
> + * configuration, or data change is permitted.
> + */
> + IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE =
> + IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_BASE + 1,
> + /*
> + * Read-only request for debug information outside of typical TDISP
> + * operational requirements. No device secrets are permitted.
> + */
> + IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ =
> + IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_BASE + 2,
> + /*
> + * Device state changes for debug purposes. The request may affect the
> + * operational state of the device outside of the TDISP operational
> + * model. If allowed, this requires CAP_SYS_RAW_IO and taints the
> + * kernel.
> + */
> + IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE =
> + IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_BASE + 3,
> + IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST =
> + IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE,
> +};
> +
> +/**
> + * struct iommu_vdevice_tsm_req - ioctl(IOMMU_VDEVICE_TSM_REQ)
> + * @size: sizeof(struct iommu_vdevice_tsm_req)
> + * @vdevice_id: vDevice ID the guest request is for
> + * @scope: One of enum iommu_vdevice_tsm_req_scope
> + * @req_len: Size in bytes of the input payload at @req_uptr
> + * @resp_len: Size in bytes of the output buffer at @resp_uptr
> + * @__reserved: Must be 0
> + * @req_uptr: Userspace pointer to the guest-provided request payload
> + * @resp_uptr: Userspace pointer to the guest response buffer
> + *
> + * Forward a TSM request to the TSM bound vDevice. This is intended for
> + * guest TSM/TDISP message transport where the host kernel only marshals
> + * bytes between userspace and the TSM implementation.
> + *
> + * Requests outside the iommufd allocated scope values are rejected. Lower
> + * layers may reject scope values that are valid in the global iommufd
> + * namespace, but not permitted for a specific bus.
> + *
> + * The request payload is read from @req_uptr/@req_len. If a response is
> + * expected, userspace provides @resp_uptr/@resp_len as writable storage for
> + * response bytes returned by the TSM path.
> + *
> + * The ioctl is only suitable for commands and results that the host kernel
> + * has no use, the host is only facilitating guest to TSM communication.
> + */
> +struct iommu_vdevice_tsm_req {
> + __u32 size;
> + __u32 vdevice_id;
> + __u32 scope;
> + __u32 req_len;
> + __u32 resp_len;
> + __u32 __reserved;
> + __aligned_u64 req_uptr;
> + __aligned_u64 resp_uptr;
> +};
> +
> +#define IOMMU_VDEVICE_TSM_REQ _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VDEVICE_TSM_REQ)
> #endif
--
Alexey
^ permalink raw reply
* [Invitation] bi-weekly guest_memfd upstream call on 2026-05-28
From: Ackerley Tng @ 2026-05-26 23:49 UTC (permalink / raw)
To: linux-coco, linux-mm, kvm; +Cc: david
Hi,
Our next guest_memfd upstream call is scheduled for Thursday, 2026-05-28
at 8:00 - 9:00am (GMT-07:00) Pacific Time - Vancouver.
We'll be using the following Google meet:
http://meet.google.com/wxp-wtju-jzw
In this meeting, we'll Tarun talk about guest_memfd preservation for
LiveUpdate.
We also have one question regarding virtio-mem and CoCo forwarded from
David.
The meeting notes can be found at [1], where we also link recordings and
collect current guest_memfd upstream proposals. If you want an google
calendar invitation that also covers all future meetings, just write
Ackerley or David a mail.
To put something to discuss onto the agenda, reply to this mail or add
them to the "Topics/questions for next meeting(s)" section in the
meeting notes as a comment.
[1] https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?usp=sharing
Ackerley
^ permalink raw reply
* Re: [PATCH v14 28/44] arm64: RMI: Create the realm descriptor
From: Wei-Lin Chang @ 2026-05-26 22:47 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: 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, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-29-steven.price@arm.com>
Hi,
On Wed, May 13, 2026 at 02:17:36PM +0100, Steven Price wrote:
> Creating a realm involves first creating a realm descriptor (RD). This
> involves passing the configuration information to the RMM. Do this as
> part of realm_ensure_created() so that the realm is created when it is
> first needed.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * The RMM no longer uses AUX granules, so no need to ask it how many it
> needs.
> * Adapted to other changes.
> Changes since v12:
> * Since RMM page size is now equal to the host's page size various
> calculations are simplified.
> * Switch to using range based APIs to delegate/undelegate.
> * VMID handling is now handled entirely by the RMM.
> ---
> arch/arm64/kvm/rmi.c | 88 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 86 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index fb96bcaa73ed..cae29fd3353c 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -418,6 +418,77 @@ static void realm_unmap_shared_range(struct kvm *kvm,
> start, end);
> }
>
> +static int realm_create_rd(struct kvm *kvm)
> +{
> + struct realm *realm = &kvm->arch.realm;
> + struct realm_params *params = realm->params;
> + void *rd = NULL;
> + phys_addr_t rd_phys, params_phys;
> + size_t pgd_size = kvm_pgtable_stage2_pgd_size(kvm->arch.mmu.vtcr);
> + int r;
> +
> + realm->ia_bits = VTCR_EL2_IPA(kvm->arch.mmu.vtcr);
> +
> + if (WARN_ON(realm->rd || !realm->params))
> + return -EEXIST;
> +
> + rd = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
> + if (!rd)
> + return -ENOMEM;
> +
> + rd_phys = virt_to_phys(rd);
> + if (rmi_delegate_page(rd_phys)) {
> + r = -ENXIO;
> + goto free_rd;
> + }
> +
> + if (rmi_delegate_range(kvm->arch.mmu.pgd_phys, pgd_size)) {
> + r = -ENXIO;
> + goto out_undelegate_tables;
> + }
> +
> + params->s2sz = VTCR_EL2_IPA(kvm->arch.mmu.vtcr);
> + params->rtt_level_start = get_start_level(realm);
> + params->rtt_num_start = pgd_size / PAGE_SIZE;
> + params->rtt_base = kvm->arch.mmu.pgd_phys;
> +
> + if (kvm->arch.arm_pmu) {
> + params->pmu_num_ctrs = kvm->arch.nr_pmu_counters;
> + params->flags |= RMI_REALM_PARAM_FLAG_PMU;
> + }
> +
> + if (kvm_lpa2_is_enabled())
> + params->flags |= RMI_REALM_PARAM_FLAG_LPA2;
> +
> + params_phys = virt_to_phys(params);
> +
> + if (rmi_realm_create(rd_phys, params_phys)) {
> + r = -ENXIO;
> + goto out_undelegate_tables;
> + }
> +
> + realm->rd = rd;
> + kvm_set_realm_state(kvm, REALM_STATE_NEW);
> + /* The realm is up, free the parameters. */
> + free_page((unsigned long)realm->params);
> + realm->params = NULL;
> +
> + return 0;
> +
> +out_undelegate_tables:
> + if (WARN_ON(rmi_undelegate_range(kvm->arch.mmu.pgd_phys, pgd_size))) {
> + /* Leak the pages if they cannot be returned */
> + kvm->arch.mmu.pgt = NULL;
> + }
> + if (WARN_ON(rmi_undelegate_page(rd_phys))) {
> + /* Leak the page if it isn't returned */
> + return r;
> + }
> +free_rd:
> + free_page((unsigned long)rd);
> + return r;
> +}
> +
> static void realm_unmap_private_range(struct kvm *kvm,
> unsigned long start,
> unsigned long end,
> @@ -647,8 +718,21 @@ static int realm_init_ipa_state(struct kvm *kvm,
>
> static int realm_ensure_created(struct kvm *kvm)
> {
> - /* Provided in later patch */
> - return -ENXIO;
> + int ret;
> +
> + switch (kvm_realm_state(kvm)) {
> + case REALM_STATE_NONE:
> + break;
> + case REALM_STATE_NEW:
> + return 0;
> + case REALM_STATE_DEAD:
> + return -ENXIO;
> + default:
> + return -EBUSY;
> + }
> +
> + ret = realm_create_rd(kvm);
> + return ret;
> }
I think ret can be simplified out.
Thanks,
Wei-Lin Chang
>
> static int set_ripas_of_protected_regions(struct kvm *kvm)
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v14 19/44] arm64: RMI: Allocate/free RECs to match vCPUs
From: Wei-Lin Chang @ 2026-05-26 22:39 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: 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, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-20-steven.price@arm.com>
Hi,
On Wed, May 13, 2026 at 02:17:27PM +0100, Steven Price wrote:
> The RMM maintains a data structure known as the Realm Execution Context
> (or REC). It is similar to struct kvm_vcpu and tracks the state of the
> virtual CPUs. KVM must delegate memory and request the structures are
> created when vCPUs are created, and suitably tear down on destruction.
>
> RECs may require additional pages (e.g. for storing larger register
> state for SVE). The RMM can request extra pages for this purpose using
> the Stateful RMI Operations (SRO) functionality to request pages during
> REC creation. These pages are then passed back to the host from the RMM
> ('reclaimed') when the REC is destroyed. The kernel tracking object
> (struct rmi_sro_state) is stored in the realm_rec structure to avoid
> memory allocation during the destruction path.
>
> Note that only some of register state for the REC can be set by KVM, the
> rest is defined by the RMM (zeroed). The register state then cannot be
> changed by KVM after the REC is created (except when the guest
> explicitly requests this e.g. by performing a PSCI call).
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * Support SRO for REC creation/destruction instead of auxiliary
> granules.
> Changes since v12:
> * Use the new range-based delegation RMI.
> Changes since v11:
> * Remove the KVM_ARM_VCPU_REC feature. User space no longer needs to
> configure each VCPU separately, RECs are created on the first VCPU
> run of the guest.
> Changes since v9:
> * Size the aux_pages array according to the PAGE_SIZE of the host.
> Changes since v7:
> * Add comment explaining the aux_pages array.
> * Rename "undeleted_failed" variable to "should_free" to avoid a
> confusing double negative.
> Changes since v6:
> * Avoid reporting the KVM_ARM_VCPU_REC feature if the guest isn't a
> realm guest.
> * Support host page size being larger than RMM's granule size when
> allocating/freeing aux granules.
> Changes since v5:
> * Separate the concept of vcpu_is_rec() and
> kvm_arm_vcpu_rec_finalized() by using the KVM_ARM_VCPU_REC feature as
> the indication that the VCPU is a REC.
> Changes since v2:
> * Free rec->run earlier in kvm_destroy_realm() and adapt to previous patches.
> ---
> arch/arm64/include/asm/kvm_emulate.h | 2 +-
> arch/arm64/include/asm/kvm_host.h | 3 +
> arch/arm64/include/asm/kvm_rmi.h | 17 +++++
> arch/arm64/kvm/arm.c | 6 ++
> arch/arm64/kvm/reset.c | 1 +
> arch/arm64/kvm/rmi.c | 105 +++++++++++++++++++++++++++
> 6 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 82fd777bd9bb..2e69fe494716 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -714,7 +714,7 @@ static inline bool kvm_realm_is_created(struct kvm *kvm)
>
> static inline bool vcpu_is_rec(const struct kvm_vcpu *vcpu)
> {
> - return false;
> + return kvm_is_realm(vcpu->kvm);
> }
>
> #endif /* __ARM64_KVM_EMULATE_H__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 3512696ed506..39b5de03d0fe 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -969,6 +969,9 @@ struct kvm_vcpu_arch {
>
> /* Hyp-readable copy of kvm_vcpu::pid */
> pid_t pid;
> +
> + /* Realm meta data */
> + struct realm_rec rec;
> };
>
> /*
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 8bd743093ccf..d99bf4fc3c39 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -59,6 +59,22 @@ struct realm {
> unsigned int ia_bits;
> };
>
> +/**
> + * struct realm_rec - Additional per VCPU data for a Realm
> + *
> + * @mpidr: MPIDR (Multiprocessor Affinity Register) value to identify this VCPU
> + * @rec_page: Kernel VA of the RMM's private page for this REC
> + * @aux_pages: Additional pages private to the RMM for this REC
> + * @run: Kernel VA of the RmiRecRun structure shared with the RMM
> + * @sro: A preallocated SRO state context
> + */
> +struct realm_rec {
> + unsigned long mpidr;
> + void *rec_page;
> + struct rec_run *run;
> + struct rmi_sro_state *sro;
> +};
> +
> void kvm_init_rmi(void);
> u32 kvm_realm_ipa_limit(void);
>
> @@ -66,6 +82,7 @@ int kvm_init_realm(struct kvm *kvm);
> int kvm_activate_realm(struct kvm *kvm);
> void kvm_destroy_realm(struct kvm *kvm);
> void kvm_realm_destroy_rtts(struct kvm *kvm);
> +void kvm_destroy_rec(struct kvm_vcpu *vcpu);
>
> static inline bool kvm_realm_is_private_address(struct realm *realm,
> unsigned long addr)
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index eb2b61fe1f0a..93d34762db91 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -586,6 +586,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> /* Force users to call KVM_ARM_VCPU_INIT */
> vcpu_clear_flag(vcpu, VCPU_INITIALIZED);
>
> + vcpu->arch.rec.mpidr = INVALID_HWID;
> +
> vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
>
> /* Set up the timer */
> @@ -1651,6 +1653,10 @@ static int kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu,
> if (test_bit(KVM_ARM_VCPU_HAS_EL2, &features))
> return -EINVAL;
>
> + /* Realms are incompatible with AArch32 */
> + if (vcpu_is_rec(vcpu))
> + return -EINVAL;
> +
> return 0;
> }
>
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index b963fd975aac..c18cdca7d125 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -161,6 +161,7 @@ void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
> free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> kfree(vcpu->arch.vncr_tlb);
> kfree(vcpu->arch.ccsidr);
> + kvm_destroy_rec(vcpu);
> }
>
> static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index 849111817af7..353a5ca45e78 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -173,9 +173,108 @@ static int realm_ensure_created(struct kvm *kvm)
> return -ENXIO;
> }
>
> +static int kvm_create_rec(struct kvm_vcpu *vcpu)
> +{
> + struct user_pt_regs *vcpu_regs = vcpu_gp_regs(vcpu);
> + unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
> + struct realm *realm = &vcpu->kvm->arch.realm;
> + struct realm_rec *rec = &vcpu->arch.rec;
> + unsigned long rec_page_phys;
> + struct rec_params *params;
> + int r, i;
> +
> + if (rec->run)
> + return -EBUSY;
> +
> + /*
> + * The RMM will report PSCI v1.0 to Realms and the KVM_ARM_VCPU_PSCI_0_2
> + * flag covers v0.2 and onwards.
> + */
> + if (!vcpu_has_feature(vcpu, KVM_ARM_VCPU_PSCI_0_2))
> + return -EINVAL;
> +
> + BUILD_BUG_ON(sizeof(*params) > PAGE_SIZE);
> + BUILD_BUG_ON(sizeof(*rec->run) > PAGE_SIZE);
> +
> + params = (struct rec_params *)get_zeroed_page(GFP_KERNEL);
> + rec->rec_page = (void *)__get_free_page(GFP_KERNEL);
> + rec->run = (void *)get_zeroed_page(GFP_KERNEL);
Should this be cast to (struct rec_run *) ?
> + rec->sro = kmalloc_obj(*rec->sro);
> + if (!params || !rec->rec_page || !rec->run || !rec->sro) {
> + r = -ENOMEM;
> + goto out_free_pages;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(params->gprs); i++)
> + params->gprs[i] = vcpu_regs->regs[i];
> +
> + params->pc = vcpu_regs->pc;
> +
> + if (vcpu->vcpu_id == 0)
> + params->flags |= REC_PARAMS_FLAG_RUNNABLE;
> +
> + rec_page_phys = virt_to_phys(rec->rec_page);
> +
> + if (rmi_delegate_page(rec_page_phys)) {
> + r = -ENXIO;
> + goto out_free_pages;
> + }
> +
> + params->mpidr = mpidr;
> +
> + if (rmi_rec_create(virt_to_phys(realm->rd), rec_page_phys,
> + virt_to_phys(params), rec->sro)) {
> + r = -ENXIO;
> + goto out_undelegate_rmm_rec;
> + }
> +
> + rec->mpidr = mpidr;
> +
> + free_page((unsigned long)params);
> + return 0;
> +
> +out_undelegate_rmm_rec:
> + if (WARN_ON(rmi_undelegate_page(rec_page_phys)))
> + rec->rec_page = NULL;
> +out_free_pages:
> + free_page((unsigned long)rec->run);
> + free_page((unsigned long)rec->rec_page);
> + free_page((unsigned long)params);
> + kfree(rec->sro);
> + rec->run = NULL;
> + return r;
> +}
> +
[...]
Thanks,
Wei-Lin Chang
^ permalink raw reply
* Re: [PATCH v14 17/44] arm64: RMI: RTT tear down
From: Wei-Lin Chang @ 2026-05-26 22:32 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: 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, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-18-steven.price@arm.com>
Hi,
On Wed, May 13, 2026 at 02:17:25PM +0100, Steven Price wrote:
> The RMM owns the stage 2 page tables for a realm, and KVM must request
> that the RMM creates/destroys entries as necessary. The physical pages
> to store the page tables are delegated to the realm as required, and can
> be undelegated when no longer used.
>
> Creating new RTTs is the easy part, tearing down is a little more
> tricky. The result of realm_rtt_destroy() can be used to effectively
> walk the tree and destroy the entries (undelegating pages that were
> given to the realm).
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * Avoid the double call of kvm_free_stage2_pgd() by splitting the work
> across that and a new function kvm_realm_uninit_stage2() which is
> only called for realm guests.
> Changes since v12:
> * Simplify some functions now we know RMM page size is the same as the
> host's.
> Changes since v11:
> * Moved some code from earlier in the series to this one so that it's
> added when it's first used.
> Changes since v10:
> * RME->RMI rename.
> * Some code to handle freeing stage 2 PGD moved into this patch where
> it belongs.
> Changes since v9:
> * Add a comment clarifying that root level RTTs are not destroyed until
> after the RD is destroyed.
> Changes since v8:
> * Introduce free_rtt() wrapper which calls free_delegated_granule()
> followed by kvm_account_pgtable_pages(). This makes it clear where an
> RTT is being freed rather than just a delegated granule.
> Changes since v6:
> * Move rme_rtt_level_mapsize() and supporting defines from kvm_rme.h
> into rme.c as they are only used in that file.
> Changes since v5:
> * Rename some RME_xxx defines to do with page sizes as RMM_xxx - they are
> a property of the RMM specification not the RME architecture.
> Changes since v2:
> * Moved {alloc,free}_delegated_page() and ensure_spare_page() to a
> later patch when they are actually used.
> * Some simplifications now rmi_xxx() functions allow NULL as an output
> parameter.
> * Improved comments and code layout.
> ---
> arch/arm64/include/asm/kvm_rmi.h | 7 ++
> arch/arm64/kvm/mmu.c | 21 ++++-
> arch/arm64/kvm/rmi.c | 148 +++++++++++++++++++++++++++++++
> 3 files changed, 174 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 9de34983ee52..06ba0d4745c6 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -64,5 +64,12 @@ u32 kvm_realm_ipa_limit(void);
>
> int kvm_init_realm(struct kvm *kvm);
> void kvm_destroy_realm(struct kvm *kvm);
> +void kvm_realm_destroy_rtts(struct kvm *kvm);
> +
> +static inline bool kvm_realm_is_private_address(struct realm *realm,
> + unsigned long addr)
> +{
> + return !(addr & BIT(realm->ia_bits - 1));
> +}
>
> #endif /* __ASM_KVM_RMI_H */
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba8286472286..eb56d4e7f21a 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1024,9 +1024,26 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu, unsigned long t
> return err;
> }
>
> +static void kvm_realm_uninit_stage2(struct kvm_s2_mmu *mmu)
> +{
> + struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> + struct realm *realm = &kvm->arch.realm;
> +
> + if (kvm_realm_state(kvm) != REALM_STATE_ACTIVE)
> + return;
> +
> + write_lock(&kvm->mmu_lock);
> + kvm_stage2_unmap_range(mmu, 0, BIT(realm->ia_bits - 1), true);
> + write_unlock(&kvm->mmu_lock);
> + kvm_realm_destroy_rtts(kvm);
> +}
> +
> void kvm_uninit_stage2_mmu(struct kvm *kvm)
> {
> - kvm_free_stage2_pgd(&kvm->arch.mmu);
> + if (kvm_is_realm(kvm))
> + kvm_realm_uninit_stage2(&kvm->arch.mmu);
> + else
> + kvm_free_stage2_pgd(&kvm->arch.mmu);
> kvm_mmu_free_memory_cache(&kvm->arch.mmu.split_page_cache);
> }
>
> @@ -1103,7 +1120,7 @@ void stage2_unmap_vm(struct kvm *kvm)
> void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
> {
> struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> - struct kvm_pgtable *pgt = NULL;
> + struct kvm_pgtable *pgt;
>
> write_lock(&kvm->mmu_lock);
> pgt = mmu->pgt;
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index f51ec667445e..5b00ccca4af3 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -11,6 +11,14 @@
> #include <asm/rmi_cmds.h>
> #include <asm/virt.h>
>
> +static inline unsigned long rmi_rtt_level_mapsize(int level)
> +{
> + if (WARN_ON(level > KVM_PGTABLE_LAST_LEVEL))
> + return PAGE_SIZE;
> +
> + return (1UL << ARM64_HW_PGTABLE_LEVEL_SHIFT(level));
> +}
> +
> static bool rmi_has_feature(unsigned long feature)
> {
> return !!u64_get_bits(rmm_feat_reg0, feature);
> @@ -21,6 +29,144 @@ u32 kvm_realm_ipa_limit(void)
> return u64_get_bits(rmm_feat_reg0, RMI_FEATURE_REGISTER_0_S2SZ);
> }
>
> +static int get_start_level(struct realm *realm)
> +{
> + return 4 - stage2_pgtable_levels(realm->ia_bits);
> +}
> +
> +static void free_rtt(phys_addr_t phys)
> +{
> + if (free_delegated_page(phys))
> + return;
> +
> + kvm_account_pgtable_pages(phys_to_virt(phys), -1);
> +}
> +
> +/*
> + * realm_rtt_destroy - Destroy an RTT at @level for @addr.
> + *
> + * Returns - Result of the RMI_RTT_DESTROY call, and:
> + * @rtt_granule: RTT granule, if the RTT was destroyed.
> + * @next_addr: IPA corresponding to the next possible valid entry we
> + * can target
> + */
> +static int realm_rtt_destroy(struct realm *realm, unsigned long addr,
> + int level, phys_addr_t *rtt_granule,
> + unsigned long *next_addr)
> +{
> + unsigned long out_rtt;
> + int ret;
> +
> + ret = rmi_rtt_destroy(virt_to_phys(realm->rd), addr, level,
> + &out_rtt, next_addr);
> +
> + *rtt_granule = out_rtt;
> +
> + return ret;
> +}
Looks like out_rtt can be simplified out.
[...]
Thanks,
Wei-Lin Chang
^ permalink raw reply
* Re: [PATCH v14 17/44] arm64: RMI: RTT tear down
From: Wei-Lin Chang @ 2026-05-26 22:27 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: 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, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-18-steven.price@arm.com>
Hi,
On Wed, May 13, 2026 at 02:17:25PM +0100, Steven Price wrote:
> The RMM owns the stage 2 page tables for a realm, and KVM must request
> that the RMM creates/destroys entries as necessary. The physical pages
> to store the page tables are delegated to the realm as required, and can
> be undelegated when no longer used.
>
> Creating new RTTs is the easy part, tearing down is a little more
> tricky. The result of realm_rtt_destroy() can be used to effectively
> walk the tree and destroy the entries (undelegating pages that were
> given to the realm).
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * Avoid the double call of kvm_free_stage2_pgd() by splitting the work
> across that and a new function kvm_realm_uninit_stage2() which is
> only called for realm guests.
> Changes since v12:
> * Simplify some functions now we know RMM page size is the same as the
> host's.
> Changes since v11:
> * Moved some code from earlier in the series to this one so that it's
> added when it's first used.
> Changes since v10:
> * RME->RMI rename.
> * Some code to handle freeing stage 2 PGD moved into this patch where
> it belongs.
> Changes since v9:
> * Add a comment clarifying that root level RTTs are not destroyed until
> after the RD is destroyed.
> Changes since v8:
> * Introduce free_rtt() wrapper which calls free_delegated_granule()
> followed by kvm_account_pgtable_pages(). This makes it clear where an
> RTT is being freed rather than just a delegated granule.
> Changes since v6:
> * Move rme_rtt_level_mapsize() and supporting defines from kvm_rme.h
> into rme.c as they are only used in that file.
> Changes since v5:
> * Rename some RME_xxx defines to do with page sizes as RMM_xxx - they are
> a property of the RMM specification not the RME architecture.
> Changes since v2:
> * Moved {alloc,free}_delegated_page() and ensure_spare_page() to a
> later patch when they are actually used.
> * Some simplifications now rmi_xxx() functions allow NULL as an output
> parameter.
> * Improved comments and code layout.
> ---
> arch/arm64/include/asm/kvm_rmi.h | 7 ++
> arch/arm64/kvm/mmu.c | 21 ++++-
> arch/arm64/kvm/rmi.c | 148 +++++++++++++++++++++++++++++++
> 3 files changed, 174 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 9de34983ee52..06ba0d4745c6 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -64,5 +64,12 @@ u32 kvm_realm_ipa_limit(void);
>
> int kvm_init_realm(struct kvm *kvm);
> void kvm_destroy_realm(struct kvm *kvm);
> +void kvm_realm_destroy_rtts(struct kvm *kvm);
> +
> +static inline bool kvm_realm_is_private_address(struct realm *realm,
> + unsigned long addr)
> +{
> + return !(addr & BIT(realm->ia_bits - 1));
> +}
>
> #endif /* __ASM_KVM_RMI_H */
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba8286472286..eb56d4e7f21a 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1024,9 +1024,26 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu, unsigned long t
> return err;
> }
>
> +static void kvm_realm_uninit_stage2(struct kvm_s2_mmu *mmu)
> +{
> + struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> + struct realm *realm = &kvm->arch.realm;
> +
> + if (kvm_realm_state(kvm) != REALM_STATE_ACTIVE)
> + return;
> +
> + write_lock(&kvm->mmu_lock);
> + kvm_stage2_unmap_range(mmu, 0, BIT(realm->ia_bits - 1), true);
> + write_unlock(&kvm->mmu_lock);
> + kvm_realm_destroy_rtts(kvm);
> +}
> +
> void kvm_uninit_stage2_mmu(struct kvm *kvm)
> {
> - kvm_free_stage2_pgd(&kvm->arch.mmu);
> + if (kvm_is_realm(kvm))
> + kvm_realm_uninit_stage2(&kvm->arch.mmu);
> + else
> + kvm_free_stage2_pgd(&kvm->arch.mmu);
> kvm_mmu_free_memory_cache(&kvm->arch.mmu.split_page_cache);
> }
>
> @@ -1103,7 +1120,7 @@ void stage2_unmap_vm(struct kvm *kvm)
> void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu)
> {
> struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> - struct kvm_pgtable *pgt = NULL;
> + struct kvm_pgtable *pgt;
Is this included by accident?
>
> write_lock(&kvm->mmu_lock);
> pgt = mmu->pgt;
[...]
Thanks,
Wei-Lin Chang
^ permalink raw reply
* Re: [PATCH v14 13/44] arm64: RMI: Define the user ABI
From: Wei-Lin Chang @ 2026-05-26 22:17 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: 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, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-14-steven.price@arm.com>
On Wed, May 13, 2026 at 02:17:21PM +0100, Steven Price wrote:
> There is one CAP which identified the presence of CCA, and one ioctl.
> The ioctl is used to populate memory during creation of the realm as
> this requires the RMM to copy data from an unprotected address to the
> protected memory - CCA does not support memory conversion where the
> memory contents is preserved as this is incompatible with memory
> encryption.
Nit:
I believe spelling out the CAP and ioctl names can improve the commit
message. Also "memory conversion" is a little vague, maybe
... CCA does not support shared <-> private memory conversion where ...
would make this clearer?
Thanks,
Wei-Lin Chang
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * KVM_ARM_VCPU_RMI_PSCI_COMPLETE removed.
> * KVM_ARM_RMI_POPULATE documentation updated to reflect that the
> structure is written by the kernel.
> * CAP number bumped.
> Changes since v12:
> * Change KVM_ARM_RMI_POPULATE to update the structure with the amount
> that has been progressed rather than return the number of bytes
> populated.
> * Describe the flag KVM_ARM_RMI_POPULATE_FLAGS_MEASURE.
> * CAP number is bumped.
> * NOTE: The PSCI ioctl may be removed in a future spec release.
> Changes since v11:
> * Completely reworked to be more implicit. Rather than having explicit
> CAP operations to progress the realm construction these operations
> are done when needed (on populating and on first vCPU run).
> * Populate and PSCI complete are promoted to proper ioctls.
> Changes since v10:
> * Rename symbols from RME to RMI.
> Changes since v9:
> * Improvements to documentation.
> * Bump the magic number for KVM_CAP_ARM_RME to avoid conflicts.
> Changes since v8:
> * Minor improvements to documentation following review.
> * Bump the magic numbers to avoid conflicts.
> Changes since v7:
> * Add documentation of new ioctls
> * Bump the magic numbers to avoid conflicts
> Changes since v6:
> * Rename some of the symbols to make their usage clearer and avoid
> repetition.
> Changes from v5:
> * Actually expose the new VCPU capability (KVM_ARM_VCPU_REC) by bumping
> KVM_VCPU_MAX_FEATURES - note this also exposes KVM_ARM_VCPU_HAS_EL2!
> ---
> Documentation/virt/kvm/api.rst | 40 ++++++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 13 +++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 52bbbb553ce1..ca68aae7faa2 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6553,6 +6553,37 @@ KVM_S390_KEYOP_SSKE
> Sets the storage key for the guest address ``guest_addr`` to the key
> specified in ``key``, returning the previous value in ``key``.
>
> +4.145 KVM_ARM_RMI_POPULATE
> +--------------------------
> +
> +:Capability: KVM_CAP_ARM_RMI
> +:Architectures: arm64
> +:Type: vm ioctl
> +:Parameters: struct kvm_arm_rmi_populate (in/out)
> +:Returns: 0 on success, < 0 on error
> +
> +::
> +
> + struct kvm_arm_rmi_populate {
> + __u64 base;
> + __u64 size;
> + __u64 source_uaddr;
> + __u32 flags;
> + __u32 reserved;
> + };
> +
> +Populate a region of protected address space by copying the data from the
> +(non-protected) user space pointer provided into a protected region (backed by
> +guestmem_fd). It implicitly sets the destination region to RIPAS RAM. This is
> +only valid before any VCPUs have been run. The ioctl might not populate the
> +entire region and in this case the kernel updates the fields `base`, `size` and
> +`source_uaddr`. User space may have to repeatedly call it until `size` is 0 to
> +populate the entire region.
> +
> +`flags` can be set to `KVM_ARM_RMI_POPULATE_FLAGS_MEASURE` to request that the
> +populated data is hashed and added to the guest's Realm Initial Measurement
> +(RIM).
> +
> .. _kvm_run:
>
> 5. The kvm_run structure
> @@ -8904,6 +8935,15 @@ helpful if user space wants to emulate instructions which are not
> This capability can be enabled dynamically even if VCPUs were already
> created and are running.
>
> +7.47 KVM_CAP_ARM_RMI
> +--------------------
> +
> +:Architectures: arm64
> +:Target: VM
> +:Parameters: None
> +
> +This capability indicates that support for CCA realms is available.
> +
> 8. Other capabilities.
> ======================
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6c8afa2047bf..b8cff0938041 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -996,6 +996,7 @@ struct kvm_enable_cap {
> #define KVM_CAP_S390_USER_OPEREXEC 246
> #define KVM_CAP_S390_KEYOP 247
> #define KVM_CAP_S390_VSIE_ESAMODE 248
> +#define KVM_CAP_ARM_RMI 249
>
> struct kvm_irq_routing_irqchip {
> __u32 irqchip;
> @@ -1669,4 +1670,16 @@ struct kvm_pre_fault_memory {
> __u64 padding[5];
> };
>
> +/* Available with KVM_CAP_ARM_RMI, only for VMs with KVM_VM_TYPE_ARM_REALM */
> +#define KVM_ARM_RMI_POPULATE _IOWR(KVMIO, 0xd7, struct kvm_arm_rmi_populate)
> +#define KVM_ARM_RMI_POPULATE_FLAGS_MEASURE (1 << 0)
> +
> +struct kvm_arm_rmi_populate {
> + __u64 base;
> + __u64 size;
> + __u64 source_uaddr;
> + __u32 flags;
> + __u32 reserved;
> +};
> +
> #endif /* __LINUX_KVM_H */
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v2 0/4] struct page to PFN conversion for TDX guest private memory
From: Dave Hansen @ 2026-05-26 20:00 UTC (permalink / raw)
To: Sean Christopherson, Yan Zhao
Cc: dave.hansen, pbonzini, tglx, mingo, bp, kas, x86, linux-kernel,
kvm, linux-coco, kai.huang, rick.p.edgecombe, yilun.xu,
vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <ahX2dcHAQgwuiJBC@google.com>
On 5/26/26 12:37, Sean Christopherson wrote:
>> v2 is based on v7.1.0-rc1 + Sean's 4 cleanup patches (see details in
>> section "Base" below). The purpose is to get Dave's Ack, so Sean can take
>> it from the KVM x86 tree. The full stack of v2 is available at [14].
> Dave, any concerns?
These look fine to me. They make the code marginally cleaner and the
changelogs are much better at describing the problem now.
Going to Linus via the KVM route is fine with me:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v2 0/4] struct page to PFN conversion for TDX guest private memory
From: Sean Christopherson @ 2026-05-26 19:37 UTC (permalink / raw)
To: Yan Zhao
Cc: dave.hansen, pbonzini, tglx, mingo, bp, kas, x86, linux-kernel,
kvm, linux-coco, kai.huang, rick.p.edgecombe, yilun.xu,
vannapurve, ackerleytng, sagis, binbin.wu, xiaoyao.li,
isaku.yamahata
In-Reply-To: <20260430014852.24183-1-yan.y.zhao@intel.com>
On Thu, Apr 30, 2026, Yan Zhao wrote:
> Hi
>
> This is v2 of the struct page to PFN conversion series, which converts TDX
> guest private memory mapping/unmapping APIs from taking struct page to
> taking PFN as input.
>
> v2 is based on v7.1.0-rc1 + Sean's 4 cleanup patches (see details in
> section "Base" below). The purpose is to get Dave's Ack, so Sean can take
> it from the KVM x86 tree. The full stack of v2 is available at [14].
Dave, any concerns?
I'd like to get these into the KVM x86 tree sooner than later, so that we at
least have a fighting chance of landing the S-EPT cleanup (prep work for D-PAMT)
in 7.2.
^ permalink raw reply
* Re: [PATCH v2 0/5] guest_memfd fixes for bind and populate
From: Sean Christopherson @ 2026-05-26 16:55 UTC (permalink / raw)
To: Ackerley Tng
Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe,
Vishal Annapurve, Yan Zhao, Michael Roth, Isaku Yamahata,
Chao Peng, Xiaoyao Li, Zongyao Chen, kvm, linux-kernel,
linux-coco, Yu Zhang, Fuad Tabba
In-Reply-To: <20260522-fix-sev-gmem-post-populate-v2-0-3f196bfad5a1@google.com>
On Fri, May 22, 2026, Ackerley Tng wrote:
> This series is a group of fixes for the bind and populate flows for
> guest_memfd, and fixes some issues reported by Sashiko after reviewing the
> guest_memfd in-place conversions series [1] and another fixup series Sean
> posted [3].
In the future, please don't bundle unrelated changes. The SNP specific changes
are related and should be a series, but the signed integer thing and the lack of
error handling on xa_store_range() are completely unrelated, because the fact
that Sashiko kept complaining about pre-existing issues.
I totally understand why you bundled these together, but that obviously didn't
stop Sashiko from complaining about pre-existing issues, over and over.
Unnecessarily bundling can lead to exactly what's happening here: the three SNP
changes are ready to go, but the two unrelated guest_memfd changes need new
versions. Which isn't hard to deal with, but it's extra friction that is easily
avoided.
I'll apply the SNP changes, and send a new version of the signed vs. unsigned
issue. Please send a new version of the xa_store_range() error handling (or
prove that I'm wrong).
Thanks!
^ permalink raw reply
* Re: [PATCH v2 5/5] KVM: SNP: Mark source page dirty in sev_gmem_post_populate
From: Sean Christopherson @ 2026-05-26 16:47 UTC (permalink / raw)
To: Ackerley Tng
Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe,
Vishal Annapurve, Yan Zhao, Michael Roth, Isaku Yamahata,
Chao Peng, Xiaoyao Li, Zongyao Chen, kvm, linux-kernel,
linux-coco, Yu Zhang, Fuad Tabba
In-Reply-To: <20260522-fix-sev-gmem-post-populate-v2-5-3f196bfad5a1@google.com>
On Fri, May 22, 2026, Ackerley Tng wrote:
> Mark the folio as dirty after copying data into the source page in
> sev_gmem_post_populate. After the memcpy, failing to mark the page dirty
> can lead to the memory management subsystem discarding the changes if the
> page is reclaimed or otherwise processed by the swap subsystem.
>
> Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
> arch/x86/kvm/svm/sev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index dbf75326a40f4..1a361f08c7a3d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2395,6 +2395,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> void *dst_vaddr = kmap_local_pfn(pfn);
>
> memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);
> + folio_mark_dirty(page_folio(src_page));
I'd rather use set_page_dirty(). I'll fixup when applying, unless someon objects.
> kunmap_local(dst_vaddr);
> kunmap_local(src_vaddr);
>
> --
> 2.54.0.794.g4f17f83d09-goog
>
^ permalink raw reply
* Re: [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put()
From: Edgecombe, Rick P @ 2026-05-26 16:42 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, kirill.shutemov@linux.intel.com,
x86@kernel.org
In-Reply-To: <ahVghgNAe4JrmlQH@intel.com>
On Tue, 2026-05-26 at 16:57 +0800, Chao Gao wrote:
> > - scoped_guard(spinlock, &pamt_lock) {
>
> This converts the scoped_guard() added by the previous patch to
> explicit lock/unlock and goto. It would reduce code churn if the
> previous patch used that form directly.
Yea, it's a good point. I actually debated doing it, but decided not to because
the scoped version is cleaner for the non-optimized version. But for
reviewability, never doing the scoped version is probably better.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox