* Re: [PATCH v2 27/31] coco/tdx-host: Implement SPDM session setup
From: Xu Yilun @ 2026-04-22 9:53 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-coco, linux-pci, dan.j.williams, x86, chao.gao, dave.jiang,
baolu.lu, yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe,
dave.hansen, kas, xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <51f2be5a-47d2-4a06-92bb-368aaed73530@suse.com>
> > +#define TDISP_FUNC_ID GENMASK(15, 0)
> > +#define TDISP_FUNC_ID_SEGMENT GENMASK(23, 16)
> > +#define TDISP_FUNC_ID_SEG_VALID BIT(24)
> > +
> > +static inline u32 tdisp_func_id(struct pci_dev *pdev)
> > +{
> > + u32 func_id;
> > +
> > + func_id = FIELD_PREP(TDISP_FUNC_ID_SEGMENT, pci_domain_nr(pdev->bus));
> > + if (func_id)
> > + func_id |= TDISP_FUNC_ID_SEG_VALID;
>
> This check implies pci_domain_nr returning 0 is considered invalid. Other
> callers in the kernel seem to not care, they just use the domain nr, so is
> this check spurious or intentional ?
This is the func_id format defined in TDISP SPEC, bit 24 is a must. It
is not the linux defined SBDF format.
>
> > + func_id |= FIELD_PREP(TDISP_FUNC_ID,
> > + PCI_DEVID(pdev->bus->number, pdev->devfn));
> > +
> > + return func_id;
> > +}
> > +
> > +struct spdm_config_info_t {
> > + u32 vmm_spdm_cap;
> > +#define SPDM_CAP_HBEAT BIT(13)
> > +#define SPDM_CAP_KEY_UPD BIT(14)
>
> nit: move those defines above the struct definition, they just break the
> reading flow as it is.
Yes.
...
> > +static void *tdx_dup_array_data(struct tdx_page_array *array,
> > + unsigned int data_size)
> > +{
> > + unsigned int npages = (data_size + PAGE_SIZE - 1) / PAGE_SIZE;
>
> nit: There's DIV_ROUND_UP
Yes.
...
> > +DEFINE_FREE(tdx_spdm_session_teardown, struct tdx_tsm_link *,
> > + if (!IS_ERR_OR_NULL(_T)) tdx_spdm_session_teardown(_T))
> > +
> > static int tdx_tsm_link_connect(struct pci_dev *pdev)
> > {
> > - return -ENXIO;
> > + struct tdx_tsm_link *tlink = to_tdx_tsm_link(pdev->tsm);
> > +
> > + struct tdx_tsm_link *tlink_spdm __free(tdx_spdm_session_teardown) =
> > + tdx_spdm_session_setup(tlink);
>
> Is the free() really needed here, either the session is correctly setup and
> tlink_spdm is returned. But if session_setup() files then what about calling
> spdm_session_disconnect() on an unestablished session?
Ah, we have more steps to add, the __free() will take function when the
following steps fail.
We may add __free() when we add more steps, but I think that makes the
diff harder to read, so I want to keep this style.
Thanks.
>
>
> > + if (IS_ERR(tlink_spdm)) {
> > + pci_err(pdev, "fail to setup spdm session\n");
> > + return PTR_ERR(tlink_spdm);
> > + }
> > +
> > + retain_and_null_ptr(tlink_spdm);
> > +
> > + return 0;
> > }
>
> <snip>
>
^ permalink raw reply
* Re: [PATCH v2 30/31] coco/tdx-host: Implement IDE stream setup/teardown
From: Xu Yilun @ 2026-04-22 9:57 UTC (permalink / raw)
To: Tian, Kevin
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <BN9PR11MB5276BB48433E1AF97E5D27298C582@BN9PR11MB5276.namprd11.prod.outlook.com>
On Thu, Apr 09, 2026 at 08:02:33AM +0000, Tian, Kevin wrote:
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Saturday, March 28, 2026 12:02 AM
> >
> > Implementation for a most straightforward Selective IDE stream setup.
> > Hard code all parameters for Stream Control Register. And no IDE Key
> > Refresh support.
> >
>
> 'more straightforward', compared to what?
Actually it is " *most* straightforward", I just mean "very".
^ permalink raw reply
* Re: [PATCH 0/3] arm64/virt: Add Arm CCA measurement register support
From: Jason Gunthorpe @ 2026-04-22 12:40 UTC (permalink / raw)
To: Sami Mujawar
Cc: Suzuki Poulose, Dan Williams,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Catalin Marinas, will@kernel.org,
thuth@redhat.com, Steven Price, gshan@redhat.com, Yeoreum Yun,
cedric.xing@intel.com, Dan Williams, Dionna Glaze,
Aneesh Kumar K . V, Alexey Kardashevskiy,
linux-coco@lists.linux.dev
In-Reply-To: <AS8PR08MB680669CBFA9654BC12179D5E842F2@AS8PR08MB6806.eurprd08.prod.outlook.com>
On Wed, Apr 22, 2026 at 08:57:00AM +0000, Sami Mujawar wrote:
> > So this is tying it to the same FW event log that TPM uses.
> >
> > I think that strengthens my point this should all be uninform. TPM
> > drivers are directly exposing the event log today, but I guess that
> > needs generalization if non-TPM drivers are going to present it as
> > well.
> >
> > How do you imagine getting and manipulating the EFI event log to use
> > with this?
>
> The event logs from UEFI will be handed off to the OS using CCEL
> ACPI table. The CCEL table spec update can be seen at
> https://github.com/tianocore/edk2/issues/11384
I ment from linux userspace. event log is well establihsed in the aCPI
side for TPM and in Linux today it is only exposed to userspace
through TPM.
This is not TPM, so how do you intend to give the event log to
userspace?
Jason
^ permalink raw reply
* Re: [PATCH v13 00/48] arm64: Support for Arm CCA in KVM
From: Steven Price @ 2026-04-22 15:38 UTC (permalink / raw)
To: Jiahao zheng
Cc: alexandru.elisei, alpergun, aneesh.kumar, catalin.marinas,
christoffer.dall, fj0570is, gankulkarni, gshan, james.morse,
joey.gouly, kvm, kvmarm, linux-arm-kernel, linux-coco,
linux-kernel, maz, oliver.upton, sdonthineni, suzuki.poulose,
tabba, vannapurve, will, yuzenghui
In-Reply-To: <20260421135145.14789-1-jahao.zheng@gmail.com_quarantine>
On 21/04/2026 14:51, Jiahao zheng wrote:
> Hi Steven,
>
> I've been testing CCA patch series and noticed Realm VM cannot boot successfully when the host is forced to run in nVHE mode (e.g., via `kvm-arm.mode=nvhe`). The kvmtool debug information will be truncated in set_guest_bank_private_gpa.
>
> Currently, in `kvm_ioctl_vcpu_run()`, running a Realm VM (REC) bypasses the standard nVHE EL2 stub. `kvm_rec_enter()` directly executes the SMC instruction to transition to the RMM. Upon returning to the EL1 host, the code falls back to `kvm_vgic_sync_hwstate()`, where the VGIC save operation is explicitly skipped for nVHE. Since the EL2 stub was bypassed, `__vgic_v3_save_state()` is never executed, and `ICH_*_EL2` states are lost.
>
> To resolve this, I have a couple of thoughts:
> 1. If Host nVHE mode is not intended to be supported for Realms:
> Since RME implies ARMv9 which mandates VHE, running a Realm with an nVHE host might just be an unsupported edge case. If so, we should explicitly reject RME initialization or REC creation when `!is_kernel_in_hyp_mode()`. This would cleanly prevent the undefined behavior.
> 2. If Host nVHE mode is intended to be supported:
> Since RMM should remain agnostic to the Non-Secure VGIC states, the burden of saving these states falls strictly on KVM. However, the EL1 host cannot access `ICH_*_EL2`. Therefore, KVM needs to add specific logic for this scenario. We would likely need to route the REC exit through a dedicated nVHE EL2 stub to invoke `__vgic_v3_save_state()` before dropping back to EL1, rather than jumping straight back to `kvm_ioctl_vcpu_run()`.
>
> I might have missed some documentation or comments regarding nVHE restrictions for CCA. If this is an oversight, it would be great to see a check added in the next iteration of the series.
Thanks for the testing. Yes indeed this is an oversight. For now option
1 is what I'm going to go for. There's nothing stopping nVHE mode being
supported but as you note any platform with RME will have VHE so it's
not an immediate priority to support.
One interesting case of nVHE is of course pKVM and for that there needs
to be some significant work to ensure that the EL2 hypervisor
understands the RMM communication and prevent any confused-deputy style
attacks. E.g. the host must not be able to map a pVM's private memory
into a realm guest.
I don't have any immediate plans to work on nVHE - my focus is getting
the basic support merged. But I know there was some interest to ensure
that pKVM and CCA would be able to co-exist on a platform so I expect it
will come in some form or another.
Thanks,
Steve
^ permalink raw reply
* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Moger, Babu @ 2026-04-22 21:42 UTC (permalink / raw)
To: Reinette Chatre, Babu Moger, corbet@lwn.net, tony.luck@intel.com,
Dave.Martin@arm.com, james.morse@arm.com, tglx@kernel.org,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com
Cc: skhan@linuxfoundation.org, x86@kernel.org, hpa@zytor.com,
peterz@infradead.org, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, kas@kernel.org, rick.p.edgecombe@intel.com,
akpm@linux-foundation.org, pmladek@suse.com,
rdunlap@infradead.org, dapeng1.mi@linux.intel.com,
kees@kernel.org, elver@google.com, paulmck@kernel.org,
lirongqing@baidu.com, safinaskar@gmail.com, fvdl@google.com,
seanjc@google.com, pawan.kumar.gupta@linux.intel.com,
xin@zytor.com, tiala@microsoft.com, chang.seok.bae@intel.com,
Lendacky, Thomas, elena.reshetova@intel.com,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-coco@lists.linux.dev, kvm@vger.kernel.org,
eranian@google.com, peternewman@google.com
In-Reply-To: <ed0d2463-edbf-42df-b345-b83da356e865@intel.com>
Hi Reinette,
On 4/21/2026 9:56 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 4/21/26 5:17 PM, Moger, Babu wrote:
>> On 4/21/2026 5:44 PM, Reinette Chatre wrote:
>>> On 4/21/26 3:04 PM, Moger, Babu wrote:
>
>>>> That said, I agree we need to support this. Without it, we won’t be able to move the group from PLZA to non-PLZA.
>>>>
>>>> # cat info/kernel_mode
>>>> inherit_ctrl_and_mon:
>>>> global_assign_ctrl_assign_mon_per_cpu:group=uninitialized
>>>> [global_assign_ctrl_assign_mon_per_cpu]:group=ctrl1/mon1/
>>>
>>> Like above where the listing is inconsistent. Is this what you mean?
>>
>> I meant the listing of "inherit_ctrl_and_mon" does not have groups while other modes have it.
>
> I think this is ok since it does not need a group or any other (for now?) property.
> What issues do you foresee here?
Nothing at this point. Will let you know if something comes up when
started working on it.
Thanks,
Babu
^ permalink raw reply
* Re: [PATCH v2 20/31] x86/virt/tdx: Add a helper to loop on TDX_INTERRUPTED_RESUMABLE
From: Huang, Kai @ 2026-04-23 0:29 UTC (permalink / raw)
To: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Edgecombe, Rick P, Xu, Yilun, Jiang, Dave,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
Duan, Zhenzhong, kas@kernel.org, Verma, Vishal L, Li, Xiaoyao,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260327160132.2946114-21-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> +static u64 __maybe_unused __seamcall_ir_resched(sc_func_t sc_func, u64 fn,
> + struct tdx_module_args *args)
> +{
> + struct tdx_module_args _args;
> + u64 r;
> +
> + while (1) {
> + _args = *(args);
> + r = sc_retry(sc_func, fn, &_args);
> + if (r != TDX_INTERRUPTED_RESUMABLE)
> + break;
> +
> + cond_resched();
> + }
> +
> + *args = _args;
> +
> + return r;
> +}
Since commit 7dadeaa6e851e ("sched: Further restrict the preemption modes")
for x86 only PREEMPT (full) and PREEMPT_LAZY are possible, even when
PREEMPT_DYNAMIC is on.
cond_resched() is useful in PREEMPT_NONE and PREEMPT_VOLUNTARY, but it is
basically a RET0 in both PREEMPT_LAZY and PREEMPT. My understanding is we
shouldn't add any more cond_resched() for x86 now (see the aforementioned
commit changelog for more info).
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Huang, Kai @ 2026-04-23 0:59 UTC (permalink / raw)
To: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Edgecombe, Rick P, Xu, Yilun, Jiang, Dave,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
Duan, Zhenzhong, kas@kernel.org, Verma, Vishal L, Li, Xiaoyao,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260327160132.2946114-11-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> +static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> +{
> + struct tdx_module_args args = {
> + .rcx = hpa_list_info_assign_raw(ext_mem),
> + };
> + u64 r;
> +
> + tdx_clflush_page_array(ext_mem);
> +
> + do {
> + r = seamcall_ret(TDH_EXT_MEM_ADD, &args);
> + cond_resched();
> + } while (r == TDX_INTERRUPTED_RESUMABLE);
Ditto here. I don't think we should introduce any more cond_resched().
Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
later, albeit in which a local '_args' is used as a copy of the original 'args',
but that has no harm for the case where we can just use the 'args' to loop.
I am wondering whether we can just use that here, or we just get rid of that
helper (then open retry by the callers of these SEAMCALL wrappers), since there
will be more cases where we need to manually set 'resume=1' in the SEAMCALL
input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
Unless you have good idea to unify them all?
E.g., we have something like below in our internal KVM code, using macros to do
'resume=1' and retry as the caller wishes. But my understanding is Dave
probably won't like macros. :-)
(you may see broken indent/text due to text wrapper and sorry for that.)
/*
* ...
*
* The retry_func and update_args allow the SEAMCALL to be retried in a loop if
* it can still return other error code when there's no race from both KVM and
* vCPUs and can be "retried" until it succeeds.
*/
#define tdh_do_no_vcpus_retry(tdh_func, kvm, retry_func, update_args, args...)\
({ \
struct kvm_tdx *__kvm_tdx = to_kvm_tdx(kvm); \
u64 __err; \
\
lockdep_assert_held_write(&kvm->mmu_lock); \
\
__err = retry_func(tdh_func, update_args, args); \
if (unlikely(tdx_operand_busy(__err))) { \
WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, true); \
kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE); \
\
__err = retry_func(tdh_func, update_args, args); \
\
WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, false); \
} \
__err; \
})
#define tdh_intr_retry(tdh_func, update_args, args...) \
({ \
u64 ____err; \
\
do { \
____err = tdh_func(args); \
\
if ((____err & TDX_SEAMCALL_STATUS_MASK) != \
TDX_INTERRUPTED_RESUMABLE)
\
break; \
\
update_args; \
} while (1); \
____err; \
})
#define tdh_no_retry(tdh_func, update_args, args...) tdh_func(args)
#define tdh_do_no_vcpus(tdh_func, kvm, args...) \
tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_no_retry, ;, args)
#define tdh_do_no_vcpus_intr_retry(tdh_func, kvm, update_args, args...) \
tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_intr_retry, update_args, args)
^ permalink raw reply
* Re: [PATCH v2 05/31] x86/virt/tdx: Extend tdx_page_array to support IOMMU_MT
From: Xu Yilun @ 2026-04-23 11:15 UTC (permalink / raw)
To: Dan Williams
Cc: Edgecombe, Rick P, Gao, Chao, Xu, Yilun, x86@kernel.org,
kas@kernel.org, baolu.lu@linux.intel.com,
dave.hansen@linux.intel.com, Li, Xiaoyao, Jiang, Dave,
linux-pci@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Duan, Zhenzhong, Verma, Vishal L,
kvm@vger.kernel.org
In-Reply-To: <69e7f166319a5_fe083100f7@djbw-dev.notmuch>
> A couple recommendations come to mind:
>
> * s/tdx_page_array_free/tdx_page_array_destroy/
>
> ...since "destroy" mirrors create and matches other cases where only
> metadata is managed.
>
> * Create a new tdx_page_array_repopulate() helper to make it clear which
> paths depend on being able to repopulate and move the WARN_ON_ONCE() out of
> the common path that does not repopulate. "repopulate" can have
> "realloc" semantics where it allocates on first use, but otherwise
> "populate" gets to not care about the corner cases. Make the WARN case
> fail repopulate.
Agree. I end up add a function like that:
/**
* tdx_page_array_repopulate() - repopulate a tdx_page_array
* @array: The array descriptor to reallocate for.
* @pages: Pointer to struct page array for tdx_page_array populating
* @nr_pages: Size of @pages array.
*
* Re-populate the tdx_page_array. If @array is %NULL, it behaves exactly like
* tdx_page_array_create().
*
* Return: Re-populated tdx_page_array or NULL on failure.
*/
static struct tdx_page_array *
tdx_page_array_repopulate(struct tdx_page_array *array, struct page **pages,
unsigned int nr_pages)
{
struct tdx_page_array *tmp = array;
int ret;
if (tmp) {
/* Don't pass in something partially initialized */
if (!tmp->root || !tmp->pages || !tmp->nr_pages)
return NULL;
/*
* When re-populating, the old pages are no longer tracked.
* Theoretically they require cache flushing before reclaiming
* for other kernel usage, similar to tdx_page_array_destroy().
* Since there is no use case to repopulate and then reclaim
* old pages yet, just warn to prompt future improvement.
*/
if (WARN_ON_ONCE(tmp->need_phymem_page_wbinvd))
return NULL;
} else {
tmp = tdx_page_array_alloc();
if (!tmp)
return NULL;
}
ret = tdx_page_array_populate(tmp, pages, nr_pages);
if (ret) {
/* Only destroy newly allocated object */
if (!array)
tdx_page_array_destroy(tmp);
return NULL;
}
return tmp;
}
^ permalink raw reply
* Re: [PATCH v2 06/31] x86/virt/tdx: Read global metadata for TDX Module Extensions/Connect
From: Xu Yilun @ 2026-04-23 11:58 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, x86, chao.gao, dave.jiang, baolu.lu,
yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe, dave.hansen, kas,
xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <69e7f808e9aa5_fe0831004@djbw-dev.notmuch>
On Tue, Apr 21, 2026 at 03:19:52PM -0700, Dan Williams wrote:
> Xu Yilun wrote:
> > Add reading of the global metadata for TDX Module Extensions & TDX
> > Connect. Add them in a batch as TDX Connect is currently the only user
> > of TDX Module Extensions and no way to initialize TDX Module Extensions
> > without firstly enabling TDX Connect.
> >
> > TDX Module Extensions & TDX Connect are optional features enumerated by
> > TDX_FEATURES0. Check the TDX_FEATURES0 before reading these metadata to
> > avoid failing the whole TDX initialization.
>
> I think it is important to distinguish "optional" module features vs
> required Linux features. Linux requires all features that a module
> advertises to succeed at core TDX init time.
Agree. But I want to reduce the scope to only about metadata reading in
this patch. So:
TDX Module Extensions is an optional features enumerated by
TDX_FEATURES0. But in the implementation, Linux requires that all
features that a Module advertises must have a complete, valid set of
metadata, and the check 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 whole
TDX initialization to fail, otherwise skip.
>
> Otherwise, this looks ok / consistent with other metadata reading. It
> sets the precedent that if TDX Connect is advertised it must succeed all
> core initialization.
^ permalink raw reply
* Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
From: Xu Yilun @ 2026-04-23 15:49 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, dan.j.williams, x86, chao.gao, dave.jiang,
baolu.lu, yilun.xu, zhenzhong.duan, kvm, rick.p.edgecombe,
dave.hansen, kas, xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <69e8223f4c8e6_fe08310058@djbw-dev.notmuch>
On Tue, Apr 21, 2026 at 06:19:59PM -0700, Dan Williams wrote:
> Xu Yilun wrote:
> > TDX Module supports optional TDX features (e.g. TDX Connect & TDX Module
> > Extensions) that won't be enabled by default.
>
> So this is another place where "optional" is misleading. For simplicity
> there is no mechanism to fallback from TDX Connect operation if present,
> at least in the core. The only optional aspects would be the mechanism
> that could be unloaded through the tdx_host driver.
I see. I think I should use 'extra' instead of 'optional' in all places.
I'll rephase the comments.
>
> .../me notices that other comments on this patch say the same, but do
> read on, another important detail about ktime_get_real_seconds() below.
>
> > It extends TDH.SYS.CONFIG for host to choose to enable them on bootup.
> >
> > Call TDH.SYS.CONFIG with a new bitmap input parameter to specify which
> > features to enable. The bitmap uses the same definitions as
> > TDX_FEATURES0. But note not all bits in TDX_FEATURES0 are valid for
> > configuration, e.g. TDX Module Extensions is a service that supports TDX
> > Connect, it is implicitly enabled when TDX Connect is enabled. Setting
> > TDX_FEATURES0_EXT in the bitmap has no effect.
> >
> > TDX Module advances the version of TDH.SYS.CONFIG for the change, so
> > use the latest version (v1) for optional feature enabling. But
> > supporting existing Modules which only support v0 is still necessary
> > until they are deprecated, enumerate via TDX_FEATURES0 to decide which
> > version to use.
>
> I would say this differently, it will always be the case that new
> kernels are needed to enable new features, but it is unlikely that
> TDH.SYS.CONFIG ever needs to change again. The v0 -> v1 transition means
> that feature bits are to be used here on out. So there is little value
> in worrying about deprecating v0 to save a couple lines of code in 5-7
> years when these original TDX platforms sunset.
OK. I'll use your rationale.
>
> > TDX Module updates global metadata when optional features are enabled.
> > Host should update the cached tdx_sysinfo to reflect these changes.
> >
> > Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > ---
> > arch/x86/virt/vmx/tdx/tdx.h | 3 ++-
> > arch/x86/virt/vmx/tdx/tdx.c | 16 +++++++++++++++-
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> > index e5a9331df451..870bb75da3ba 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.h
> > +++ b/arch/x86/virt/vmx/tdx/tdx.h
> > @@ -58,7 +58,8 @@
> > #define TDH_PHYMEM_CACHE_WB 40
> > #define TDH_PHYMEM_PAGE_WBINVD 41
> > #define TDH_VP_WR 43
> > -#define TDH_SYS_CONFIG 45
> > +#define TDH_SYS_CONFIG_V0 45
> > +#define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
> >
> > /* TDX page types */
> > #define PT_NDA 0x0
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 130214933c2f..0c5d6bdd810f 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1353,6 +1353,7 @@ static int construct_tdmrs(struct list_head *tmb_list,
> > static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
> > {
> > struct tdx_module_args args = {};
> > + u64 seamcall_fn = TDH_SYS_CONFIG_V0;
> > u64 *tdmr_pa_array;
> > size_t array_sz;
> > int i, ret;
> > @@ -1377,7 +1378,15 @@ static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
> > args.rcx = __pa(tdmr_pa_array);
> > args.rdx = tdmr_list->nr_consumed_tdmrs;
> > args.r8 = global_keyid;
> > - ret = seamcall_prerr(TDH_SYS_CONFIG, &args);
> > +
> > + if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_TDXCONNECT) {
> > + args.r9 |= TDX_FEATURES0_TDXCONNECT;
> > + args.r11 = ktime_get_real_seconds();
>
> Mainline has reason to not entertain this module requirement. The fact
> that passing zero is an error is useful to detect unsupported modules.
Ah, it is "useful"... :)
> An updated module would accept zero as indicating "VMM requests module
> disable all policy and mechanisms related to untrusted wall clock time".
> Specifically, there are several problems with this:
>
> 1/ No other TSM implementation requires the VMM to pass in an untrusted time
> 2/ The wall time may change and may require hooks to keep the module time
> up to date, but see point 1/, this would be a TDX special flower hook.
> 3/ Presumably this allows the module or the guest to do certificate expiration
> checks, but that is the responsibility of the relying party. The
> relying party may have reason to accept an "expired" cert as determined
> by VMM wall clock, and the guest presumably already has mechanisms to
> determine untrusted wall clock time from the VMM if it wants. Guests
> do not need TDX ABI for that.
>
> So I think Linux wants to pass 0 here and wait for modules that accept
> that as the start of TDX Connect support. As you said, given there are
> no released modules with TDX Connect there is time to make that first
> release drop this requirement.
I see. Actually the Module is about to remove the r11 RTC.
https://cdrdv2.intel.com/v1/dl/getContent/871617
I'll remove this r11.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xu Yilun @ 2026-04-23 16:41 UTC (permalink / raw)
To: Huang, Kai
Cc: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, x86@kernel.org, Gao, Chao,
Edgecombe, Rick P, Xu, Yilun, Jiang, Dave,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
Duan, Zhenzhong, kas@kernel.org, Verma, Vishal L, Li, Xiaoyao,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <89bccca9a409e02667278fd83f0b7b9064557dab.camel@intel.com>
On Thu, Apr 23, 2026 at 12:59:55AM +0000, Huang, Kai wrote:
> On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > +static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> > +{
> > + struct tdx_module_args args = {
> > + .rcx = hpa_list_info_assign_raw(ext_mem),
> > + };
> > + u64 r;
> > +
> > + tdx_clflush_page_array(ext_mem);
> > +
> > + do {
> > + r = seamcall_ret(TDH_EXT_MEM_ADD, &args);
> > + cond_resched();
> > + } while (r == TDX_INTERRUPTED_RESUMABLE);
>
>
> Ditto here. I don't think we should introduce any more cond_resched().
Good to me.
>
> Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> later, albeit in which a local '_args' is used as a copy of the original 'args',
> but that has no harm for the case where we can just use the 'args' to loop.
No we can't. TDH_EXT_MEM_ADD is designed to use output parameter RCX
to override/update input parameter RCX, so the caller doesn't have to
do manual parameter update on retry call. Using seamcall_ir_resched()
makes each retry use the original RCX, not the updated one.
>
> I am wondering whether we can just use that here, or we just get rid of that
> helper (then open retry by the callers of these SEAMCALL wrappers), since there
> will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
I'd like to know why some SEAMCALLs needs resume flag but others don't.
If there is chance we don't introduce too much variants for the same thing,
that's most friendly to OS. And "no resume flag" is my best preference.
For now, I can see only one SEAMCALL with resume flag in mainline,
tdh_phymem_cache_wb(). I'd rather we treat it as an exception and no
resume flag any more if possible.
Then we don't have to make all following efforts, they are complex...
>
> Unless you have good idea to unify them all?
>
> E.g., we have something like below in our internal KVM code, using macros to do
> 'resume=1' and retry as the caller wishes. But my understanding is Dave
> probably won't like macros. :-)
>
> (you may see broken indent/text due to text wrapper and sorry for that.)
>
> /*
> * ...
> *
> * The retry_func and update_args allow the SEAMCALL to be retried in a loop if
> * it can still return other error code when there's no race from both KVM and
> * vCPUs and can be "retried" until it succeeds.
> */
> #define tdh_do_no_vcpus_retry(tdh_func, kvm, retry_func, update_args, args...)\
> ({ \
> struct kvm_tdx *__kvm_tdx = to_kvm_tdx(kvm); \
> u64 __err; \
> \
> lockdep_assert_held_write(&kvm->mmu_lock); \
> \
> __err = retry_func(tdh_func, update_args, args); \
> if (unlikely(tdx_operand_busy(__err))) { \
> WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, true); \
> kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE); \
> \
> __err = retry_func(tdh_func, update_args, args); \
> \
> WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, false); \
> } \
> __err; \
> })
>
> #define tdh_intr_retry(tdh_func, update_args, args...) \
> ({ \
> u64 ____err; \
> \
> do { \
> ____err = tdh_func(args); \
> \
> if ((____err & TDX_SEAMCALL_STATUS_MASK) != \
> TDX_INTERRUPTED_RESUMABLE)
> \
> break; \
> \
> update_args; \
> } while (1); \
> ____err; \
> })
>
> #define tdh_no_retry(tdh_func, update_args, args...) tdh_func(args)
>
> #define tdh_do_no_vcpus(tdh_func, kvm, args...) \
> tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_no_retry, ;, args)
>
> #define tdh_do_no_vcpus_intr_retry(tdh_func, kvm, update_args, args...) \
> tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_intr_retry, update_args, args)
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Edgecombe, Rick P @ 2026-04-23 17:05 UTC (permalink / raw)
To: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, Huang, Kai, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, Jiang, Dave, baolu.lu@linux.intel.com,
kas@kernel.org, dave.hansen@linux.intel.com, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <89bccca9a409e02667278fd83f0b7b9064557dab.camel@intel.com>
On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> Ditto here. I don't think we should introduce any more cond_resched().
>
> Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> later, albeit in which a local '_args' is used as a copy of the original 'args',
> but that has no harm for the case where we can just use the 'args' to loop.
>
> I am wondering whether we can just use that here, or we just get rid of that
> helper (then open retry by the callers of these SEAMCALL wrappers), since there
> will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
I kind of like the latter option to open code more of this stuff. The stacks of
seamcall wrapper macros is already too much.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Huang, Kai @ 2026-04-23 21:55 UTC (permalink / raw)
To: yilun.xu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
kas@kernel.org, linux-kernel@vger.kernel.org, Xu, Yilun,
Jiang, Dave, Verma, Vishal L, Duan, Zhenzhong, Gao, Chao,
Edgecombe, Rick P, linux-pci@vger.kernel.org, x86@kernel.org,
dan.j.williams@intel.com
In-Reply-To: <aepLsi/cHnw3Cfgs@yilunxu-OptiPlex-7050>
>
> >
> > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > but that has no harm for the case where we can just use the 'args' to loop.
>
> No we can't. TDH_EXT_MEM_ADD is designed to use output parameter RCX
> to override/update input parameter RCX, so the caller doesn't have to
> do manual parameter update on retry call. Using seamcall_ir_resched()
> makes each retry use the original RCX, not the updated one.
OK I wish there's a comment saying there's additional output besides error code
via RAX and we just need to feed the output as input again when retrying the
SEAMCALL.
>
> >
> > I am wondering whether we can just use that here, or we just get rid of that
> > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
>
> I'd like to know why some SEAMCALLs needs resume flag but others don't.
> If there is chance we don't introduce too much variants for the same thing,
> that's most friendly to OS. And "no resume flag" is my best preference.
I don't know either.
>
> For now, I can see only one SEAMCALL with resume flag in mainline,
> tdh_phymem_cache_wb(). I'd rather we treat it as an exception and no
> resume flag any more if possible.
Right, but there will be more, and setting 'resumed=1' is even different from
how tdh_phymem_cache_wb() does.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Huang, Kai @ 2026-04-23 22:29 UTC (permalink / raw)
To: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, Edgecombe, Rick P,
yilun.xu@linux.intel.com, x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com,
baolu.lu@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
linux-kernel@vger.kernel.org, Verma, Vishal L, Jiang, Dave,
kvm@vger.kernel.org, Duan, Zhenzhong
In-Reply-To: <3b0bc0dea544b10bd2fb0304a96d2671f263829b.camel@intel.com>
On Thu, 2026-04-23 at 17:05 +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> > Ditto here. I don't think we should introduce any more cond_resched().
> >
> > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > but that has no harm for the case where we can just use the 'args' to loop.
> >
> > I am wondering whether we can just use that here, or we just get rid of that
> > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
>
> I kind of like the latter option to open code more of this stuff. The stacks of
> seamcall wrapper macros is already too much.
Agreed.
And SEAMCALL *users* can actually come up with their own version of wrapper(s)
to do the retry. E.g., currently seamcall_ir_resched() is only used for IOMMU
SEAMCALLs, and we can put this wrapper in the IOMMU code or coco/tdx-host.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xu Yilun @ 2026-04-24 3:07 UTC (permalink / raw)
To: Huang, Kai
Cc: dan.j.williams@intel.com, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, Edgecombe, Rick P, x86@kernel.org,
Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com,
baolu.lu@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
linux-kernel@vger.kernel.org, Verma, Vishal L, Jiang, Dave,
kvm@vger.kernel.org, Duan, Zhenzhong
In-Reply-To: <48171f4ab772da546beccec3c7a95fe442524b42.camel@intel.com>
On Thu, Apr 23, 2026 at 10:29:31PM +0000, Huang, Kai wrote:
> On Thu, 2026-04-23 at 17:05 +0000, Edgecombe, Rick P wrote:
> > On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> > > Ditto here. I don't think we should introduce any more cond_resched().
> > >
> > > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > > but that has no harm for the case where we can just use the 'args' to loop.
> > >
> > > I am wondering whether we can just use that here, or we just get rid of that
> > > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
> >
> > I kind of like the latter option to open code more of this stuff. The stacks of
> > seamcall wrapper macros is already too much.
>
> Agreed.
>
> And SEAMCALL *users* can actually come up with their own version of wrapper(s)
> to do the retry. E.g., currently seamcall_ir_resched() is only used for IOMMU
> SEAMCALLs, and we can put this wrapper in the IOMMU code or coco/tdx-host.
After we have introduced TDX Module Extension, irq preemptable
EXT-SEAMCALLs become a common concept. It is irq preemptable so that the
secure world remembers and resumes the context, no need for host to
remind via resume lag.
Today there are 3 EXT-SEAMCALLs, TDH_SPDM_CONNECT/DISCONNECT/MNG,
irq preemption handling is a general requirement for them, and I think
it is still true for any further EXT-SEAMCALLs.
So I think a general helper for EXT-SEAMCALLs makes sense.
TDH.IOMMU.SETUP, however, is another case. It is not a EXT-SEAMCALL but
happened to follow the same irq-retry handling process. To avoid code
duplication we have:
/*
* seamcall_ret_ir_exec() aliases seamcall_ret_ir_resched() for
* documentation purposes. It documents the TDX Module extension
* seamcalls that are long running / hard-irq preemptible flows that
* generate events. The calls using seamcall_ret_ir_resched() are long
* running flows, that periodically yield.
*/
#define seamcall_ret_ir_exec seamcall_ret_ir_resched
TDH.IOMMU.SETUP uses seamcall_ret_ir_resched(), and EXT-SEAMCALLs use
seamcall_ret_ir_exec().
How do you think?
^ permalink raw reply
* RE: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
From: Tian, Kevin @ 2026-04-24 6:49 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aehkFicutMwa2LP+@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 2:01 PM
>
> > Here we need more words to explain the strategy here.
> >
> > The comment says "When IOMMU is *enabled*...", but the code here
> > just checks the static capability. It's probably a design choice that you
> > don't want to add complexity on recycling DIDs when TDX connect
> > is actually enabled, but it's worth a note here.
>
> Yes, that's the rationale. I'll add it to comments.
btw halving the DID space permanently on any platforms supporting
TDX connect doesn't sound a good design. It may break usages which
already uses more than 50% of the DID space but have no business
to do with TDX connect.
It makes more sense to cut it down in-fly when tdx connect is initialized.
If the higher half DIDs have been used then fail TDX connect. otherwise
adjust the max domain id.
>
> >
> > btw in patch23 commit msg:
> >
> > "
> > There is no dedicated way to enumerate which IOMMU devices support
> > trusted operations. The host has to call TDH.IOMMU.SETUP on all IOMMU
> > devices and tell their trusted capability by the return value.
> > "
> >
> > which implies that ecap_tdxc() alone doesn't really report the capability?
>
> Ah, good catch. Let me explain:
>
> ecap_tdxc does report the capability. This bit is special cause both
> trusted part & untrusted part access it.
>
> For IOMMU driver (which now handles the untrusted part), it can directly
> query to this bit and decide what to do.
>
> But for tdx-host driver which handles the trusted part, it shouldn't
> speculate into the IOMMU for capability enumeration. TDX Module has more
> concerns about trusted capability, including the related I/O stack
I guess "more concerns" means that there are more conditions for
TDX module to look at beyond ecap_tdxc(), so it's not appropriate
for tdx-host driver to check ecap alone.
> capabilities e.g. SPDM/IDE cap... So in patch23 I actually mean we
> don't have an enumeration SEAMCALL for trusted capability, I will
> refactor that message:
>
> There is no dedicated *SEAMCALL* to enumerate which IOMMU devices
> support
> trusted operations...
>
> >
> > anyway all of those need a better explanation here...
^ permalink raw reply
* RE: [PATCH v2 22/31] iommu/vt-d: Export a helper to do function for each dmar_drhd_unit
From: Tian, Kevin @ 2026-04-24 6:50 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aehrup2/IWdq62mm@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 2:34 PM
>
> On Thu, Apr 09, 2026 at 07:49:46AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Saturday, March 28, 2026 12:01 AM
> > >
> > > @@ -86,6 +86,8 @@ extern struct list_head dmar_drhd_units;
> > > dmar_rcu_check()) \
> > > if (i=drhd->iommu, 0) {} else
> > >
> > > +int do_for_each_drhd_unit(int (*fn)(struct dmar_drhd_unit *));
> > > +
> > > static inline bool dmar_rcu_check(void)
> >
> > It's a bit weird to insert it here. Move it to follow for_each_iommu().
>
> Sorry, it is following for_each_iommu(), is it?
>
yeah, I misread it.
^ permalink raw reply
* RE: [PATCH v2 23/31] coco/tdx-host: Setup all trusted IOMMUs on TDX Connect init
From: Tian, Kevin @ 2026-04-24 6:54 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aeiUartvxzsvq2Ye@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 5:27 PM
>
> On Thu, Apr 09, 2026 at 07:51:56AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Saturday, March 28, 2026 12:01 AM
> > >
> > > Setup all trusted IOMMUs on TDX Connect initialization and clear all on
> > > TDX Connect removal.
> > >
> > > Trusted IOMMU setup is the pre-condition for all following TDX Connect
> > > operations such as SPDM/IDE setup. It is more of a platform
> > > configuration than a standalone IOMMU configuration, so put the
> > > implementation in tdx-host driver.
> > >
> >
> > not sure what above tries to tell. why is it a platform configuration
> > when you have seamcalls on each IOMMU?
>
> This is to say the TDH.IOMMU.SETUP relates to PCIe SPDM/IDE, it is not
> just about IOMMU. By identifying the
>
> for_each_iommu(iommu)
> tdh.iommu.setup(iommu)
>
> as a platform configuration, it justifies why we trigger this
> configuration at tdx-host driver probe, rather than in some
> IOMMU/IOMMUFD API.
iommu drivers also involve PCI, e.g. call pci_enable_ats(), etc.
so having relation to PCIe SPDM/IDE is not an argument of
platform vs. IOMMU.
Actually I'm OK to put that logic in tdx-host. Just the explanation
here doesn't make much sense...
^ permalink raw reply
* RE: [PATCH v2 20/31] x86/virt/tdx: Add a helper to loop on TDX_INTERRUPTED_RESUMABLE
From: Tian, Kevin @ 2026-04-24 6:57 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aehk/2DEizc1eG+e@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 2:05 PM
>
> On Thu, Apr 09, 2026 at 07:21:48AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Saturday, March 28, 2026 12:01 AM
> > >
> > > +static u64 __maybe_unused __seamcall_ir_resched(sc_func_t sc_func,
> u64
> > > fn,
> > > + struct tdx_module_args *args)
> > > +{
> >
> > 'ir' sounds redundant with the trailing 'resched'?
>
> Mm.. I want to 'ir' to reflect the loop-retry is dedicated for
> INTERRUPTED_RESUMABLE in TDX context. When you say not big deal, I
> assume I can keep the naming?
>
that's ok
^ permalink raw reply
* RE: [PATCH v2 24/31] coco/tdx-host: Add a helper to exchange SPDM messages through DOE
From: Tian, Kevin @ 2026-04-24 7:01 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aeiXuludvkZedOg5@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 5:41 PM
>
> On Thu, Apr 09, 2026 at 07:56:06AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Saturday, March 28, 2026 12:01 AM
> > > +
> > > +static int __maybe_unused tdx_spdm_msg_exchange(struct
> tdx_tsm_link
> > > *tlink,
> > > + void *request, size_t
> > > request_sz,
> > > + void *response, size_t
> > > response_sz)
> > > +{
> > > + struct pci_dev *pdev = tlink->pci.base_tsm.pdev;
> >
> > call it pci_spdm_msg_exchange() and pass in struct pci_dev directly.
>
> I don't think so. There is kernel managed spdm transfer support WIP,
> which is another topic. We don't want to mix the namespace with that
> one.
pci_spdm_raw_msg_exchange() then, since you said currently only
one user i.e. tdx?
If the kernel managed spdm doesn't support the raw format, then there
won't be conflict.
if it supports (i.e. the 2nd user), then this should be moved to pci core.
>
> And we also don't name it tsm_spdm_msg_exchange, cause TSM firmwares
> output different blobs for vendor TSM drivers to transfer. E.g. TDX
> Module outputs buffers with DOE header & SPDM header, other vendors
> (AMD IIRC) outputs buffers with only SPDM header. So this function is
> TDX specific.
>
TDX is just an user of that. All the logic here is about handling the
raw format, nothing specific to tdx.
^ permalink raw reply
* RE: [PATCH v2 30/31] coco/tdx-host: Implement IDE stream setup/teardown
From: Tian, Kevin @ 2026-04-24 7:05 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco@lists.linux.dev, linux-pci@vger.kernel.org,
Williams, Dan J, x86@kernel.org, Gao, Chao, Jiang, Dave,
baolu.lu@linux.intel.com, Xu, Yilun, Duan, Zhenzhong,
kvm@vger.kernel.org, Edgecombe, Rick P,
dave.hansen@linux.intel.com, kas@kernel.org, Li, Xiaoyao,
Verma, Vishal L, linux-kernel@vger.kernel.org
In-Reply-To: <aeibi7JzfZ2Kx0NU@yilunxu-OptiPlex-7050>
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, April 22, 2026 5:58 PM
>
> On Thu, Apr 09, 2026 at 08:02:33AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Saturday, March 28, 2026 12:02 AM
> > >
> > > Implementation for a most straightforward Selective IDE stream setup.
> > > Hard code all parameters for Stream Control Register. And no IDE Key
> > > Refresh support.
> > >
> >
> > 'more straightforward', compared to what?
a typo.
>
> Actually it is " *most* straightforward", I just mean "very".
When you say "most straightforward", then I want to know what are
other options to compare. If you think that the thought practice
leading to the 'most' definition is important, then please elaborate.
otherwise I'd just remove that sentence.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Huang, Kai @ 2026-04-24 8:09 UTC (permalink / raw)
To: yilun.xu@linux.intel.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, Xu, Yilun,
Verma, Vishal L, Jiang, Dave, Duan, Zhenzhong, Gao, Chao,
Edgecombe, Rick P, linux-pci@vger.kernel.org, x86@kernel.org,
dan.j.williams@intel.com
In-Reply-To: <aereVy8zVb2be2hh@yilunxu-OptiPlex-7050>
On Fri, 2026-04-24 at 11:07 +0800, Xu Yilun wrote:
> On Thu, Apr 23, 2026 at 10:29:31PM +0000, Huang, Kai wrote:
> > On Thu, 2026-04-23 at 17:05 +0000, Edgecombe, Rick P wrote:
> > > On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> > > > Ditto here. I don't think we should introduce any more cond_resched().
> > > >
> > > > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > > > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > > > but that has no harm for the case where we can just use the 'args' to loop.
> > > >
> > > > I am wondering whether we can just use that here, or we just get rid of that
> > > > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > > > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > > > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
> > >
> > > I kind of like the latter option to open code more of this stuff. The stacks of
> > > seamcall wrapper macros is already too much.
> >
> > Agreed.
> >
> > And SEAMCALL *users* can actually come up with their own version of wrapper(s)
> > to do the retry. E.g., currently seamcall_ir_resched() is only used for IOMMU
> > SEAMCALLs, and we can put this wrapper in the IOMMU code or coco/tdx-host.
>
> After we have introduced TDX Module Extension, irq preemptable
> EXT-SEAMCALLs become a common concept.
>
It has been a concept since before the EXT-SEAMCALLs actually. For instance,
TDX live migration using blocking export doesn't need any opt-in via module
extension (only the non-blocking way needs), but the SEAMCALLs to export/import
TD/vCPU/memory are all interruptible.
In fact, they had the "latency requirement" behind the INTERRUPT_RESUMABLE in
mind at the very beginning. It's just at that point all SEAMCALLs were not that
heavy.
> It is irq preemptable so that the
> secure world remembers and resumes the context, no need for host to
> remind via resume lag.
The fact is the aforementioned live migration related export/import SEAMCALLs
(there are 8 at least, but maybe more) all requires the explicit setting of
'resume=1' (plus using the SEAMCALL output as input for retry). I don't know
the story behind this, though. There might be some tricky thing here for the
module to remember and manage (e.g., migration has a concept of "migration
stream", and the resume is per-stream).
>
> Today there are 3 EXT-SEAMCALLs, TDH_SPDM_CONNECT/DISCONNECT/MNG,
> irq preemption handling is a general requirement for them, and I think
> it is still true for any further EXT-SEAMCALLs.
>
> So I think a general helper for EXT-SEAMCALLs makes sense.
Yes conceptually I agree, but not need to distinguish EXT-SEAMCALLs or not IMHO.
The problem is there isn't a common rule to follow.
E.g., let's say "the module can remember thus no resume flag is needed", how
about the SEAMCALL inputs? Can the "output" args be directly used as input for
retry, or the original input should always be used?
Not to mention there's existing SEAMCALLs which require explicitly setting
'resume=1'.
I believe we can use some smart hack to implement a common one to cover all
cases above, but I am not sure whether it's worth to do (maybe we can have a try
to see how does it look like, though, I think).
Given the SEAMCALLs for TDX Connect seem to follow one rule to retry, and live
migration SEAMCALLs follow another rule, it seems for now the simplest way is to
introduce the needed retry helper in the layer of SEAMCALL *user* (TDX Connect
and migration).
> TDH.IOMMU.SETUP, however, is another case. It is not a EXT-SEAMCALL but
> happened to follow the same irq-retry handling process. To avoid code
> duplication we have:
>
> /*
> * seamcall_ret_ir_exec() aliases seamcall_ret_ir_resched() for
> * documentation purposes. It documents the TDX Module extension
> * seamcalls that are long running / hard-irq preemptible flows that
> * generate events. The calls using seamcall_ret_ir_resched() are long
> * running flows, that periodically yield.
> */
> #define seamcall_ret_ir_exec seamcall_ret_ir_resched
>
> TDH.IOMMU.SETUP uses seamcall_ret_ir_resched(), and EXT-SEAMCALLs use
> seamcall_ret_ir_exec().
>
> How do you think?
Sorry I don't quite get. What does "exec" postfix mean?
From patch 25, they are all in TDX core, so I don't quite get why we need to
distinguish EXT-SEAMCALLs vs normal ones. IMHO it's an additional layer which
doesn't actually help address any problem.
Btw, we should really get rid of the "resched()" postfix from the function name
since cond_resched() is no longer needed and possibility of rescheduling is
implied pretty much all places in the kernel code now (except some special code
such as code in IRQ context).
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Huang, Kai @ 2026-04-24 9:10 UTC (permalink / raw)
To: yilun.xu@linux.intel.com
Cc: kvm@vger.kernel.org, Li, Xiaoyao, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
kas@kernel.org, linux-kernel@vger.kernel.org, Xu, Yilun,
Verma, Vishal L, Jiang, Dave, Duan, Zhenzhong, Gao, Chao,
Edgecombe, Rick P, linux-pci@vger.kernel.org,
dan.j.williams@intel.com, x86@kernel.org
In-Reply-To: <668a903ba2dccff2d641ac15b74deacc95ef19a6.camel@intel.com>
On Fri, 2026-04-24 at 08:09 +0000, Huang, Kai wrote:
> I believe we can use some smart hack to implement a common one to cover all
> cases above, but I am not sure whether it's worth to do (maybe we can have a try
> to see how does it look like, though, I think).
So the problem is use what as input when retry, and one way is to provide a
callback to allow the user to provide a specific function to update the 'args'
before retrying.
Something like below? I am not sure I like it though, because as Rick said
there's too much SEAMCALL wrapper/macros already.
typedef void (*args_update_func_t)(struct tdx_module_args *args,
struct tdx_module_args *ori);
static __always_inline u64 sc_retry_intr(sc_func_t func, u64 fn,
struct tdx_module_args *args,
args_update_func_t update_args)
{
struct tdx_module_args _args = *args;
u64 ret;
do {
ret = sc_retry(func, fn, &_args);
if (ret != TDX_INTERRUPT_RESUMABLE)
break;
update_args(&_args, args);
} while (1);
*args = _args;
return ret;
}
#define seamcall_ret_intr(_fn, _args, _args_update_f) \
sc_retry_intr(__seamcall_ret, (_fn), (_args), (_args_update_f))
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Aneesh Kumar K.V @ 2026-04-24 10:15 UTC (permalink / raw)
To: Dan Williams, Lukas Wunner, Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, yilun.xu, bhelgaas,
alistair23, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <69ba57199b38b_7ee31005c@dwillia2-mobl4.notmuch>
Dan Williams <dan.j.williams@intel.com> writes:
> The ARM CCA spec does reference the EAT nonce which is 64-bytes. So it
> may be the case that PCI_TSM_MAX_NONCE_SIZE != SPDM_NONCE_SZ depending
> on what evidence can be collected over this interface, but I am not
> finding any spec references for 256, Aneesh?
That was probably me getting confused about 256 bits in an earlier
version of the code. Also, the RmiVdevMeasureParams structure layout
adds 256 bytes of padding between the different fields.
-aneesh
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Xu Yilun @ 2026-04-24 10:41 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org, Xu, Yilun,
Verma, Vishal L, Jiang, Dave, Duan, Zhenzhong, Gao, Chao,
Edgecombe, Rick P, linux-pci@vger.kernel.org, x86@kernel.org,
dan.j.williams@intel.com
In-Reply-To: <668a903ba2dccff2d641ac15b74deacc95ef19a6.camel@intel.com>
On Fri, Apr 24, 2026 at 08:09:16AM +0000, Huang, Kai wrote:
> On Fri, 2026-04-24 at 11:07 +0800, Xu Yilun wrote:
> > On Thu, Apr 23, 2026 at 10:29:31PM +0000, Huang, Kai wrote:
> > > On Thu, 2026-04-23 at 17:05 +0000, Edgecombe, Rick P wrote:
> > > > On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> > > > > Ditto here. I don't think we should introduce any more cond_resched().
> > > > >
> > > > > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > > > > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > > > > but that has no harm for the case where we can just use the 'args' to loop.
> > > > >
> > > > > I am wondering whether we can just use that here, or we just get rid of that
> > > > > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > > > > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > > > > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
> > > >
> > > > I kind of like the latter option to open code more of this stuff. The stacks of
> > > > seamcall wrapper macros is already too much.
> > >
> > > Agreed.
> > >
> > > And SEAMCALL *users* can actually come up with their own version of wrapper(s)
> > > to do the retry. E.g., currently seamcall_ir_resched() is only used for IOMMU
> > > SEAMCALLs, and we can put this wrapper in the IOMMU code or coco/tdx-host.
> >
> > After we have introduced TDX Module Extension, irq preemptable
> > EXT-SEAMCALLs become a common concept.
> >
>
> It has been a concept since before the EXT-SEAMCALLs actually. For instance,
No, they look similar but different. EXT-SEAMCALLs are truly irq
preemptable and resumable to its context. Other SEAMCALLs just
periodically yield and don't have a generic way to save/resume their
context. Sometime you need to pass in resume flag on 2nd time, which
means the secure world forget where they were and can't really resume
all by itself.
What I mean is, EXT-SEAMCALLs should never need to play tricks on
input parameters. Just input what is originally inputted, the secure
world doesn't need hint to resume itself. So the int-retry process
should be common.
> TDX live migration using blocking export doesn't need any opt-in via module
> extension (only the non-blocking way needs), but the SEAMCALLs to export/import
> TD/vCPU/memory are all interruptible.
>
> In fact, they had the "latency requirement" behind the INTERRUPT_RESUMABLE in
> mind at the very beginning. It's just at that point all SEAMCALLs were not that
> heavy.
>
> > It is irq preemptable so that the
> > secure world remembers and resumes the context, no need for host to
> > remind via resume lag.
>
> The fact is the aforementioned live migration related export/import SEAMCALLs
> (there are 8 at least, but maybe more) all requires the explicit setting of
> 'resume=1' (plus using the SEAMCALL output as input for retry). I don't know
Yes, so they are not truly interrupt resumable and should be specially
treated.
> the story behind this, though. There might be some tricky thing here for the
> module to remember and manage (e.g., migration has a concept of "migration
> stream", and the resume is per-stream).
>
> >
> > Today there are 3 EXT-SEAMCALLs, TDH_SPDM_CONNECT/DISCONNECT/MNG,
> > irq preemption handling is a general requirement for them, and I think
> > it is still true for any further EXT-SEAMCALLs.
> >
> > So I think a general helper for EXT-SEAMCALLs makes sense.
>
> Yes conceptually I agree, but not need to distinguish EXT-SEAMCALLs or not IMHO.
>
> The problem is there isn't a common rule to follow.
>
> E.g., let's say "the module can remember thus no resume flag is needed", how
> about the SEAMCALL inputs? Can the "output" args be directly used as input for
> retry, or the original input should always be used?
Since EXT-SEAMCALLs don't depend on input tricks to resume, there could
be a common rule, now it is defined as "the original input should always
be used".
>
> Not to mention there's existing SEAMCALLs which require explicitly setting
> 'resume=1'.
>
> I believe we can use some smart hack to implement a common one to cover all
> cases above, but I am not sure whether it's worth to do (maybe we can have a try
> to see how does it look like, though, I think).
>
> Given the SEAMCALLs for TDX Connect seem to follow one rule to retry, and live
> migration SEAMCALLs follow another rule, it seems for now the simplest way is to
> introduce the needed retry helper in the layer of SEAMCALL *user* (TDX Connect
> and migration).
>
> > TDH.IOMMU.SETUP, however, is another case. It is not a EXT-SEAMCALL but
> > happened to follow the same irq-retry handling process. To avoid code
> > duplication we have:
> >
> > /*
> > * seamcall_ret_ir_exec() aliases seamcall_ret_ir_resched() for
> > * documentation purposes. It documents the TDX Module extension
> > * seamcalls that are long running / hard-irq preemptible flows that
> > * generate events. The calls using seamcall_ret_ir_resched() are long
> > * running flows, that periodically yield.
> > */
> > #define seamcall_ret_ir_exec seamcall_ret_ir_resched
> >
> > TDH.IOMMU.SETUP uses seamcall_ret_ir_resched(), and EXT-SEAMCALLs use
> > seamcall_ret_ir_exec().
> >
> > How do you think?
>
> Sorry I don't quite get. What does "exec" postfix mean?
It is 'execution', means EXT-SEAMCALLs can resume their execution. But
since you have concern, maybe some better name?
>
> From patch 25, they are all in TDX core, so I don't quite get why we need to
> distinguish EXT-SEAMCALLs vs normal ones. IMHO it's an additional layer which
EXT-SEAMCALLs have generic way to resume, while others don't. So we need
a helper for EXT-SEAMCALLs. For other SEAMCALLs that happens to process
the same way, we are avoiding code duplication, but should clearly
distinguish the purpose so make another name as documentation.
But if any concern, we could delete the int-retry support for normal
SEAMCALLs, they are not generic as you said.
> doesn't actually help address any problem.
>
> Btw, we should really get rid of the "resched()" postfix from the function name
> since cond_resched() is no longer needed and possibility of rescheduling is
> implied pretty much all places in the kernel code now (except some special code
> such as code in IRQ context).
Yes, thanks to remind me again.
^ 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