* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-08 2:07 UTC (permalink / raw)
To: 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, pbonzini@redhat.com,
linux-kernel@vger.kernel.org, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
tglx@kernel.org, Mehta, Sohil, bp@alien8.de, Gao, Chao,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com, binbin.wu@linux.intel.com
In-Reply-To: <efbff823-7b4e-4349-bdc4-72a244699d76@intel.com>
On Tue, 2026-07-07 at 17:49 -0700, Sohil Mehta wrote:
> On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > The TDX Physical Address Metadata Table (PAMT) holds data about the
> > physical memory used by TDX, and must be allocated by the kernel during
> > TDX module initialization.
> >
> > The exact size of the required PAMT memory is determined by the TDX module
> > and may vary between TDX module versions. Currently it is approximately
> > 0.4% of the system memory. This is a significant commitment, especially if
> > it is not known upfront whether the machine will run any TDX guests.
> >
> > Each memory region that the TDX module might use needs three separate PAMT
> > allocations. One for each supported page size (1GB, 2MB, 4KB). The
> > TDX module supports a new feature designed to reduce PAMT overhead called
> > Dynamic PAMT. At a high level, Dynamic PAMT still has the 1GB and 2MB
> > levels allocated on TDX module initialization, but the 4KB level is
> > allocated dynamically during runtime.
>
> The last statement is slightly confusing to me. Is it trying to say that
> the "dynamic" part is only applicable to 4KB allocations?
Yea. Can you explain more about what is confusing? I guess this is more compact:
Under Dynamic PAMT the 4KB level is allocated dynamically during runtime, while
the 1GB and 2MB levels remain allocated on TDX module initialization.
>
>
> >
> > However, in the details, Dynamic PAMT still needs some smaller per 4KB
> > page scoped data (currently it is 1 bit per page). The TDX module exposes
> > the number of bits as a separate piece of metadata than the 4KB static
> > allocation for regular PAMT. Although the size is enumerated differently,
> > it is handed to the TDX module in the same way the 4KB page size PAMT
> > allocation is for regular, non-dynamic PAMT.
> >
> > Begin to implement Dynamic PAMT in the kernel by reading the bits-per-page
> > needed for Dynamic PAMT. Calculate the size needed for the bitmap,
> > and use it instead of the 4KB size determined for normal PAMT, in the case
> > of Dynamic PAMT.
> >
> > Unlike the existing metadata reading code, this code is not generated by a
> > script.
>
>
> It might be useful to say that this file was auto-generated in the past
> but going forward it is going to be manually updated.
>
> > So adjust the comment to be more generic. Also, start to adopt a
> > more normal kernel code style without the tenary statements and if
>
> s/a more/
> s/tenary/ternary
How about this?
The existing metadata reading code was generated by a script, but the current
plan is to stop generating this code, as the script has continued to need
adjustments. So add manually written code and adjust the comment about it being
autogenerated to be more generic. Start to adopt a more normal kernel code style
without the ternary statements and if conditionals assignments that the auto
generated code has.
>
>
> > conditionals assignments that the auto generated code has.
> >
> > Assisted-by: Sashiko:claude-opus-4-6
> > Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>
> The review tags goes after the SOBs.
Yep, I have been fixing those across the series.
>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > ---
>
> > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> > index 503f9a3f46d61..82dc27aecf297 100644
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -149,6 +149,11 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> > const char *tdx_dump_mce_info(struct mce *m);
> > const struct tdx_sys_info *tdx_get_sysinfo(void);
> >
> > +static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> > +{
> > + return false; /* To be enabled when kernel is ready */
>
> I would avoid the tail comment even if it is temporary.
Yep, Yan commented the same thing.
>
> > +}
> > +
> > int tdx_guest_keyid_alloc(void);
> > u32 tdx_get_nr_guest_keyids(void);
> > void tdx_guest_keyid_free(unsigned int keyid);
>
>
>
> > @@ -33,6 +33,18 @@ static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinf
> > return ret;
> > }
> >
> > +static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> > +{
> > + int ret;
> > + u64 val;
> > +
> > + ret = read_sys_metadata_field(0x9100000100000013, &val);
>
> Should this be a #define now that the file is being manually updated? Or
> is the plan to do it all together? A #define would make it easier to
> read this patch.
I think switching to defines would overlap too much into the ultimate metadata
reading solution that gets discussed. Chao is currently working on a series for
this, so I'd think to leave that part for later.
>
> > + if (!ret)
> > + sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;
> > +
> > + return ret;
> > +}
> > +
> > static __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> > {
> > int ret = 0;
> > @@ -116,5 +128,12 @@ 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);
> >
> > + /*
> > + * Don't treat a module that doesn't support Dynamic PAMT
> > + * as a failure. Only read the metadata optionally.
> > + */
> > + if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> > + ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>
> There is a need for the comment because it combines two checks:
>
> 1) Did any of the previous stages fail?
> 2) Does the TDX module support Dynamic PAMT?
>
> Should these be separated for readability and to follow the typical
> kernel style?
>
> if (ret)
> return ret;
>
> if (tdx_supports_dynamic_pamt(sysinfo))
> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>
> return ret;
>
> I think you can avoid the comment altogether in that case.
I don't think it removes the need for a comment. The point is if
tdx_supports_dynamic_pamt() is not supported. The comment should be more about
"why", than what the code does, right?
How about:
if (ret)
return ret;
/*
* The kernel supports using TDX without Dynamic PAMT, so
* avoid reporting failure if it's not supported.
*/
if (tdx_supports_dynamic_pamt(sysinfo))
ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
return ret;
>
> > +
> > return ret;
> > }
>
^ permalink raw reply
* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-08 2:10 UTC (permalink / raw)
To: 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, pbonzini@redhat.com,
linux-kernel@vger.kernel.org, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
tglx@kernel.org, Mehta, Sohil, bp@alien8.de, Gao, Chao,
x86@kernel.org
Cc: kirill.shutemov@linux.intel.com, binbin.wu@linux.intel.com
In-Reply-To: <a724bce3e92e8a8dfb04798bc21860ba9be786d9.camel@intel.com>
On Tue, 2026-07-07 at 19:07 -0700, Rick Edgecombe wrote:
> > I think you can avoid the comment altogether in that case.
>
> I don't think it removes the need for a comment. The point is if
> tdx_supports_dynamic_pamt() is not supported. The comment should be more about
> "why", than what the code does, right?
>
> How about:
>
> if (ret)
> return ret;
>
> /*
> * The kernel supports using TDX without Dynamic PAMT, so
> * avoid reporting failure if it's not supported.
> */
> if (tdx_supports_dynamic_pamt(sysinfo))
> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>
> return ret;
Actually this really stands out from the autogenerated code now, how about:
/*
* The kernel supports using TDX without Dynamic PAMT, so
* avoid reporting failure if it's not supported.
*/
if (!ret && tdx_supports_dynamic_pamt(sysinfo))
ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
It why-ifys the comment more, but only gently moves from the auto-generated
patterns.
^ permalink raw reply
* Re: [PATCH 15/15] PCI/TSM: Add relative MMIO offset support?
From: Alexey Kardashevskiy @ 2026-07-08 2:25 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Xu Yilun, Aneesh Kumar K.V
In-Reply-To: <20260705220819.2472765-16-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> The RMM specification, DEN0137-2.0-bet2 section A9.6.2 "Realm validation of
> device memory mappings" documents the expectation that the
> MMIO_REPORTING_OFFSET chosen for TDISP Interface Reports is always BAR
> aligned.
>
> Ideally this change is not needed and all implementations share the same
> expectation.
>
> If this semantic is already shipping in production and/or the PCI-SIG
> clarifies that an implementation can hold this assumption then Linux will
> need to ask the TSM drivers for this hint.
We are changing it on SEV-TIO such that the host os calculates the offset to allow TDISP_OFFSET_BAR_ALIGN and passes it to the PSP during TDI_BIND (==interface start), others did not need it in the first place so I guess we can drop this one. Thanks,
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> include/linux/pci-tsm.h | 15 ++++++++++++++-
> drivers/pci/tsm/core.c | 14 ++++++++++----
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index 6d5fadd79360..be9f78ca2c1a 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -296,7 +296,20 @@ struct pci_tsm_devsec *to_pci_tsm_devsec(struct pci_tsm *tsm);
> int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio);
>
> -struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev);
> +/**
> + * enum tdisp_offset_scheme - MMIO_REPORTING_OFFSET assumptions
> + * @TDISP_OFFSET_BAR_ALIGN: mask by bar size to recover offset
> + * @TDISP_OFFSET_RELATIVE: first mmio report per bar is bar-offset-0
> + *
> + * A TSM driver may know that the default TDISP_OFFSET_BAR_ALIGN
> + * assumption is being violated.
> + */
> +enum tdisp_offset_scheme {
> + TDISP_OFFSET_BAR_ALIGN,
> + TDISP_OFFSET_RELATIVE,
> +};
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev,
> + enum tdisp_offset_scheme scheme);
> int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
> index 9ac216ad896d..19ad35f2da4a 100644
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c
> @@ -643,13 +643,15 @@ struct pci_tsm_devif_report {
> /**
> * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
> * @pdev: device owner of MMIO ranges
> + * @scheme: allow the low level TSM driver to hint the offset calc scheme
> *
> * Return: the encrypted MMIO range descriptor on success, NULL on failure
> *
> * Assumes that this is called within the live lifetime of a PCI device's
> * association with a low level TSM.
> */
> -struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev,
> + enum tdisp_offset_scheme scheme)
> {
> struct device_evidence *evidence = pdev->tsm->evidence;
> u64 reporting_bar_base, last_reporting_end;
> @@ -712,10 +714,14 @@ struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> last_bar = bar;
>
> /*
> - * Determine the obfuscated base of the BAR. BAR
> - * offsets are never obfuscated.
> + * Either the first range per bar always maps
> + * the start of the BAR, or the reporting_offset
> + * is BAR size aligned.
> */
> - reporting_bar_base = tsm_offset & ~mask;
> + if (scheme == TDISP_OFFSET_RELATIVE)
> + reporting_bar_base = tsm_offset;
> + else
> + reporting_bar_base = tsm_offset & ~mask;
> } else if (tsm_offset < last_reporting_end) {
> pci_dbg(pdev, "Reporting ranges within BAR not in ascending order\n");
> return NULL;
--
Alexey
^ permalink raw reply
* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Sohil Mehta @ 2026-07-08 3:35 UTC (permalink / raw)
To: Edgecombe, Rick P, 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,
pbonzini@redhat.com, linux-kernel@vger.kernel.org,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
Annapurve, Vishal, tglx@kernel.org, bp@alien8.de, Gao, Chao,
x86@kernel.org
Cc: binbin.wu@linux.intel.com
In-Reply-To: <818e0de368588836810718106e3b592323aaa0f1.camel@intel.com>
On 7/7/2026 7:10 PM, Edgecombe, Rick P wrote:
> On Tue, 2026-07-07 at 19:07 -0700, Rick Edgecombe wrote:
>>> I think you can avoid the comment altogether in that case.
>>
>> I don't think it removes the need for a comment. The point is if
>> tdx_supports_dynamic_pamt() is not supported. The comment should be more about
>> "why", than what the code does, right?
>>
>> How about:
>>
>> if (ret)
>> return ret;
>>
>> /*
>> * The kernel supports using TDX without Dynamic PAMT, so
>> * avoid reporting failure if it's not supported.
>> */
>> if (tdx_supports_dynamic_pamt(sysinfo))
>> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>>
>> return ret;
>
> Actually this really stands out from the autogenerated code now, how about:
>
I thought that's a good thing. Eventually, we want to make this file
conform to the standard kernel style, right? The autogenerated pattern
in this file to "check any previous failure" before doing the current
step is odd.
> /*
> * The kernel supports using TDX without Dynamic PAMT, so
> * avoid reporting failure if it's not supported.
> */
> if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>
> It why-ifys the comment more, but only gently moves from the auto-generated
> patterns.
Sure, that's fine if you want to continue the style in this file.
BTW, do you also want to clarify why we fail the init if
get_tdx_sys_info_tdmr_dpamt() fails?
Is it because:
If reading the DPAMT metadata fails, something is really wrong with the
TDX module. It is better to fail the initialization in that case.
^ permalink raw reply
* Re: [PATCH v6 03/11] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Sohil Mehta @ 2026-07-08 3:50 UTC (permalink / raw)
To: Rick Edgecombe, bp, dave.hansen, hpa, kas, kvm, linux-coco,
linux-doc, linux-kernel, mingo, nik.borisov, pbonzini, seanjc,
tglx, vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
Cc: Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-4-rick.p.edgecombe@intel.com>
How about?
x86/virt/tdx: Add tdx_{alloc,free}_control_page() helpers
On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Add helpers to use when allocating or preparing pages that are handed to
> the TDX-Module for use as control/S-EPT pages, and thus need Dynamic PAMT
> adjustments.
>
> The TDX module tracks some state for each page of physical memory that it
> might use. It calls this state the PAMT. It includes separate state for
> each page size a physical page could be utilized at within the TDX module
> (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> allocated dynamically. So for pages that TDX will use as 2MB physically
> contiguous pages, Dynamic PAMT backing is not needed.
I lost the continuation in the last sentence. Why does it only talk
about 2MB if only 4KB is dynamically allocated. What about 1GB?
(Probably due to my lack of TDX knowledge)
Similarly, why do these functions only refer to 2MB only and not 1GB?
pamt_2mb_arg(), tdh_phymem_pamt_add(), tdh_phymem_pamt_remove().
>
> KVM will need to hand pages to the TDX module that it will use at 4KB
> granularity. So these pages will need Dynamic PAMT backing added before
> they are used by the TDX module, and removed afterwards.
>
...
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 82dc27aecf297..74e75db5728c7 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -37,6 +37,7 @@
>
> #include <uapi/asm/mce.h>
> #include <asm/tdx_global_metadata.h>
> +#include <linux/mm.h>
> #include <linux/pgtable.h>
>
> /*
> @@ -160,6 +161,12 @@ void tdx_guest_keyid_free(unsigned int keyid);
>
> void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
>
> +/* Number PAMT pages to be provided to TDX module per 2MB region of PA */
^^^ of PAMT pages
> +#define TDX_DPAMT_ENTRY_PAGE_CNT 2
> +
> +struct page *tdx_alloc_control_page(void);
> +void tdx_free_control_page(struct page *page);
> +
> struct tdx_td {
> /* TD root structure: */
> struct page *tdr_page;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 9ebd192cb5c17..9e0812d87ab06 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1919,6 +1919,165 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn)
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
>
> +static int alloc_pamt_array(struct page **pamt_pages)
> +{
> + int i, j;
> +
> + for (i = 0; i < TDX_DPAMT_ENTRY_PAGE_CNT; i++) {
> + pamt_pages[i] = alloc_page(GFP_KERNEL_ACCOUNT);
> + if (!pamt_pages[i])
> + goto err;
> + }
> +
> + return 0;
> +err:
> + for (j = 0; j < i; j++)
> + __free_page(pamt_pages[j]);
> + return -ENOMEM;
Add a blank line before the return to separate from the for loop?
> +}
> +
> +static void free_pamt_array(struct page **pamt_pages)
> +{
> + for (int i = 0; i < TDX_DPAMT_ENTRY_PAGE_CNT; i++) {
> + /*
> + * Reset pages unconditionally to cover cases
> + * where they were passed to the TDX module.
> + */
> + tdx_quirk_reset_paddr(page_to_phys(pamt_pages[i]), PAGE_SIZE);
> +
> + __free_page(pamt_pages[i]);
> + }
> +}
> +
> +/*
> + * Calculate the arg needed for operating on the DPAMT backing for
> + * a given 4KB page.
> + */
> +static u64 pamt_2mb_arg(kvm_pfn_t pfn)
> +{
> + unsigned long hpa_2mb = ALIGN_DOWN(pfn << PAGE_SHIFT, PMD_SIZE);
> +
> + return hpa_2mb | TDX_PS_2M;
> +}
> +
> +/* Add PAMT backing for the given page. */
> +static u64 tdh_phymem_pamt_add(kvm_pfn_t pfn, struct page **pamt_pages)
> +{
> + struct tdx_module_args args = {
> + .rcx = pamt_2mb_arg(pfn),
> + .rdx = page_to_phys(pamt_pages[0]),
> + .r8 = page_to_phys(pamt_pages[1]),
> + };
> +
> + return seamcall(TDH_PHYMEM_PAMT_ADD, &args);
> +}
> +
> +/* Remove PAMT backing for the given page. */
> +static u64 tdh_phymem_pamt_remove(kvm_pfn_t pfn, struct page **pamt_pages)
> +{
> + struct tdx_module_args args = {
> + .rcx = pamt_2mb_arg(pfn),
> + };
> + u64 ret;
> +
> + ret = seamcall_ret(TDH_PHYMEM_PAMT_REMOVE, &args);
> + if (ret)
> + return ret;
> +
> + /* Copy PAMT pages out of the struct per the TDX ABI */
> + pamt_pages[0] = phys_to_page(args.rdx);
> + pamt_pages[1] = phys_to_page(args.r8);
> +
> + return 0;
> +}
> +
> +/* Allocate PAMT memory for the given page */
> +static int tdx_pamt_get(kvm_pfn_t pfn)
> +{
> + struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> + u64 tdx_status;
> + int ret;
> +
> + if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> + return 0;
> +
> + ret = alloc_pamt_array(pamt_pages);
> + if (ret)
> + return ret;
> +
> + tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> + if (tdx_status != TDX_SUCCESS) {
> + ret = -EIO;
> + goto out_free;
> + }
> +
> + return 0;
Blank line here as well.
> +out_free:
> + free_pamt_array(pamt_pages);
> + return ret;
> +}
> +
^ permalink raw reply
* Re: [PATCH v5 08/51] x86/sev: Shove SNP's secure/trusted TSC frequency directly into "calibration"
From: Nikunj A. Dadhania @ 2026-07-08 4:52 UTC (permalink / raw)
To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Andy Lutomirski, Peter Zijlstra, Juergen Gross,
Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-9-seanjc@google.com>
On 7/2/2026 1:01 AM, Sean Christopherson wrote:
> As a first step towards dropping .calibrate_{cpu,tsc}() and explicitly
> defining precedence/priority for "calibration" routines, pass the secure
> TSC frequency obtained from SNP firmware directly to
> determine_cpu_tsc_frequencies() instead of overriding the .calibrate_tsc()
> hook.
>
> Unlike the native calibration routines, all of the paravirtual overrides,
> including SNP and TDX, are constant in the sense that the frequency
> provided by the hypervisor or trusted firmware is fixed, known, and always
> available during early boot. More importantly, for CoCo (SNP and TDX) VMs,
> it's imperative that the kernel uses the frequency provided by the trusted
> firmware, not by the untrusted hypervisor. Enforcing the priority between
> sources by carefully ordering seemingly unrelated init calls, so that the
> trusted override "wins", is brittle and all but impossible to follow.
>
> Explicitly ignore tsc_early_khz if the exact TSC frequency was obtained
> from trusted firmware, as per commit bd35c77e32e4 ("x86/tsc: Add
> tsc_early_khz command line parameter"), the goal of the param is to play
> nice with setups that provide partial frequency information in CPUID, i.e.
> is NOT intended to be a hard override. Neither SNP's secure TSC nor TDX
> was supported when commit bd35c77e32e4 landed back in 2020, i.e. lack of
> consideration for the interaction was purely due to oversight when SNP and
> TDX support came along.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +++
> arch/x86/coco/sev/core.c | 14 +++--------
> arch/x86/include/asm/sev.h | 4 +--
> arch/x86/kernel/tsc.c | 25 ++++++++++++++-----
> 4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b5493a7f8f22..181149f633c3 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7946,6 +7946,10 @@ Kernel parameters
> with CPUID.16h support and partial CPUID.15h support.
> Format: <unsigned int>
>
> + Note, tsc_early_khz is ignored if the TSC frequency is
> + provided by trusted firmware when running as an SNP
> + guest.
> +
> tsx= [X86] Control Transactional Synchronization
> Extensions (TSX) feature in Intel processors that
> support TSX control.
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 403dcea86452..bc5ae9ef74da 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -99,7 +99,6 @@ static const char * const sev_status_feat_names[] = {
> */
> static u64 snp_tsc_scale __ro_after_init;
> static u64 snp_tsc_offset __ro_after_init;
> -static unsigned long snp_tsc_freq_khz __ro_after_init;
>
> DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
> DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
> @@ -2014,15 +2013,10 @@ void __init snp_secure_tsc_prepare(void)
> pr_debug("SecureTSC enabled");
> }
>
> -static unsigned long securetsc_get_tsc_khz(void)
> -{
> - return snp_tsc_freq_khz;
> -}
> -
> -void __init snp_secure_tsc_init(void)
> +unsigned int __init snp_secure_tsc_init(void)
> {
> + unsigned long snp_tsc_freq_khz, tsc_freq_mhz;
> struct snp_secrets_page *secrets;
> - unsigned long tsc_freq_mhz;
> void *mem;
>
> mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);
> @@ -2043,7 +2037,7 @@ void __init snp_secure_tsc_init(void)
>
> snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
>
> - x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
> -
> early_memunmap(mem, PAGE_SIZE);
> +
> + return snp_tsc_freq_khz;
> }
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 594cfa19cbd4..05ebf0b73ef4 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -530,7 +530,7 @@ int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
> int snp_svsm_vtpm_send_command(u8 *buffer);
>
> void __init snp_secure_tsc_prepare(void);
> -void __init snp_secure_tsc_init(void);
> +unsigned int snp_secure_tsc_init(void);
It seems __init got dropped here accidentally?
Apart from this:
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Tested-by: Nikunj A Dadhania <nikunj@amd.com>
> enum es_result savic_register_gpa(u64 gpa);
> enum es_result savic_unregister_gpa(u64 *gpa);
> u64 savic_ghcb_msr_read(u32 reg);
> @@ -637,7 +637,7 @@ static inline int snp_send_guest_request(struct snp_msg_desc *mdesc,
> struct snp_guest_req *req) { return -ENODEV; }
> static inline int snp_svsm_vtpm_send_command(u8 *buffer) { return -ENODEV; }
> static inline void __init snp_secure_tsc_prepare(void) { }
> -static inline void __init snp_secure_tsc_init(void) { }
> +static inline unsigned int __init snp_secure_tsc_init(void) { return 0; }
> static inline void sev_evict_cache(void *va, int npages) {}
> static inline enum es_result savic_register_gpa(u64 gpa) { return ES_UNSUPPORTED; }
> static inline enum es_result savic_unregister_gpa(u64 *gpa) { return ES_UNSUPPORTED; }
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 8f1604ffe986..f049c126e47c 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1440,15 +1440,16 @@ static int __init init_tsc_clocksource(void)
> */
> device_initcall(init_tsc_clocksource);
>
> -static bool __init determine_cpu_tsc_frequencies(bool early)
> +static bool __init determine_cpu_tsc_frequencies(bool early,
> + unsigned int known_tsc_khz)
> {
> /* Make sure that cpu and tsc are not already calibrated */
> WARN_ON(cpu_khz || tsc_khz);
>
> if (early) {
> cpu_khz = x86_platform.calibrate_cpu();
> - if (tsc_early_khz)
> - tsc_khz = tsc_early_khz;
> + if (known_tsc_khz)
> + tsc_khz = known_tsc_khz;
> else
> tsc_khz = x86_platform.calibrate_tsc();
> } else {
> @@ -1503,6 +1504,8 @@ static void __init tsc_enable_sched_clock(void)
>
> void __init tsc_early_init(void)
> {
> + unsigned int known_tsc_khz = 0;
> +
> if (!boot_cpu_has(X86_FEATURE_TSC))
> return;
> /* Don't change UV TSC multi-chassis synchronization */
> @@ -1510,9 +1513,19 @@ void __init tsc_early_init(void)
> return;
>
> if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> - snp_secure_tsc_init();
> + known_tsc_khz = snp_secure_tsc_init();
>
> - if (!determine_cpu_tsc_frequencies(true))
> + /*
> + * Ignore the user-provided TSC frequency if the exact frequency was
> + * obtained from trusted firmware, as the user-provided frequency is
> + * intended as a "starting point", not a known, guaranteed frequency.
> + */
> + if (!known_tsc_khz)
> + known_tsc_khz = tsc_early_khz;
> + else if (tsc_early_khz)
> + pr_err("Ignoring 'tsc_early_khz' in favor of trusted firmware.\n");
> +
> + if (!determine_cpu_tsc_frequencies(true, known_tsc_khz))
> return;
> tsc_enable_sched_clock();
> }
> @@ -1533,7 +1546,7 @@ void __init tsc_init(void)
>
> if (!tsc_khz) {
> /* We failed to determine frequencies earlier, try again */
> - if (!determine_cpu_tsc_frequencies(false)) {
> + if (!determine_cpu_tsc_frequencies(false, 0)) {
> mark_tsc_unstable("could not calculate TSC khz");
> setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
> return;
^ permalink raw reply
* Re: [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
From: Nikunj A. Dadhania @ 2026-07-08 4:54 UTC (permalink / raw)
To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Andy Lutomirski, Peter Zijlstra, Juergen Gross,
Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-7-seanjc@google.com>
On 7/2/2026 1:01 AM, Sean Christopherson wrote:
> Don't override the kernel's CPU frequency calibration routine when
> registering SNP's Secure TSC calibration routine. SNP (the architecture)
> provides zero guarantees that the CPU runs at the same frequency as the
> TSC. The justification for clobbering the CPU routine was:
>
> Since the difference between CPU base and TSC frequency does not apply
> in this case, the same callback is being used.
>
> but that's simply not true. E.g. if APERF/MPERF is exposed to the VM, then
> the CPU frequency absolutely does matter.
>
> While relying on heuristics and/or the untrusted hypervisor to provide the
> CPU frequency isn't ideal, it's at least not outright wrong.
>
> Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests")
> Cc: Nikunj A Dadhania <nikunj@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/coco/sev/core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index ed0ac52a765e..665de1aea0ee 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
>
> snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
>
> - x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
> x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
>
> early_memunmap(mem, PAGE_SIZE);
^ permalink raw reply
* Re: [PATCH 06/15] PCI/TSM: Add device evidence support
From: Alexey Kardashevskiy @ 2026-07-08 5:00 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Bjorn Helgaas, Alistair Francis,
Lukas Wunner, Xu Yilun, Aneesh Kumar K.V (Arm)
In-Reply-To: <20260705220819.2472765-7-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> Register the PCI Trusted Execution Environment Security Manager (TSM)
> framework with the device-evidence netlink ABI. The security operations
> that PCI core coordinates with a device security manager (DSM) through a
> platform TEE security manager (TSM), builds upon the SPDM protocol.
>
> A TSM owns an SPDM session and publishes the corresponding evidence through
> TSM firwmware ABIs. A low level TSM driver is responsible for creating a
> 'struct device_evidence' context and carrying out refresh_evidence()
a nit: so it is really refresh_measurements() as it won't refresh the report? may be call it so?
> requests for regenerating measurement transcripts with a nonce.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> drivers/pci/Kconfig | 1 +
> drivers/pci/Makefile | 2 +-
> drivers/pci/tsm/Makefile | 8 +++
> include/linux/pci-tsm.h | 14 ++++
> drivers/pci/{tsm.c => tsm/core.c} | 9 +--
> drivers/pci/tsm/evidence.c | 110 ++++++++++++++++++++++++++++++
> MAINTAINERS | 2 +-
> 7 files changed, 137 insertions(+), 9 deletions(-)
> create mode 100644 drivers/pci/tsm/Makefile
> rename drivers/pci/{tsm.c => tsm/core.c} (99%)
> create mode 100644 drivers/pci/tsm/evidence.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 0c7408509ba2..ed17b5d2d5ae 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -126,6 +126,7 @@ config PCI_IDE
>
> config PCI_TSM
> bool "PCI TSM: Device security protocol support"
> + select DEVICE_EVIDENCE if NET
> select PCI_IDE
> select PCI_DOE
> select TSM
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 41ebc3b9a518..211f195ff2c9 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -35,7 +35,7 @@ obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
> obj-$(CONFIG_VGA_ARB) += vgaarb.o
> obj-$(CONFIG_PCI_DOE) += doe.o
> obj-$(CONFIG_PCI_IDE) += ide.o
> -obj-$(CONFIG_PCI_TSM) += tsm.o
> +obj-$(CONFIG_PCI_TSM) += tsm/
> obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o
> obj-$(CONFIG_PCI_NPEM) += npem.o
> obj-$(CONFIG_PCIE_TPH) += tph.o
> diff --git a/drivers/pci/tsm/Makefile b/drivers/pci/tsm/Makefile
> new file mode 100644
> index 000000000000..fd7ac2e862f1
> --- /dev/null
> +++ b/drivers/pci/tsm/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the PCI/TSM infrastructure
> +
> +obj-$(CONFIG_PCI_TSM) += tsm.o
> +
> +tsm-y := core.o
> +tsm-$(CONFIG_DEVICE_EVIDENCE) += evidence.o
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index a6435aba03f9..8bc16029d31e 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -3,7 +3,10 @@
> #define __PCI_TSM_H
> #include <linux/mutex.h>
> #include <linux/pci.h>
> +#include <linux/rwsem.h>
then this file is missing extern struct rw_semaphore pci_tsm_rwsem;
> #include <linux/sockptr.h>
> +#include <linux/tsm.h>
> +#include <linux/device/evidence.h>
>
> struct pci_tsm;
> struct tsm_dev;
> @@ -18,6 +21,7 @@ enum pci_tsm_req_scope;
> * @devsec_ops: Lock, unlock, and interrogate the security state of the
> * function via the platform TSM (typically virtual function
> * operations).
> + * @refresh_evidence: Common operation to regenerate attestation objects
> *
> * This operations are mutually exclusive either a tsm_dev instance
> * manages physical link properties or it manages function security
> @@ -75,6 +79,9 @@ struct pci_tsm_ops {
> struct pci_dev *pdev);
> void (*unlock)(struct pci_tsm *tsm);
> );
> +
> + int (*refresh_evidence)(struct pci_tsm *tsm, const void *nonce,
> + size_t nonce_len);
> };
>
> /**
> @@ -96,6 +103,8 @@ struct pci_tdi {
> * @tsm_dev: PCI TEE Security Manager device for Link Confidentiality or Device
> * Function Security operations
> * @tdi: TDI context established by the @bind link operation
> + * @evidence: cached evidence from SPDM session establishment (connect), or
> + * TDISP bind (lock)
> *
> * This structure is wrapped by low level TSM driver data and returned by
> * probe()/lock(), it is freed by the corresponding remove()/unlock().
> @@ -112,6 +121,7 @@ struct pci_tsm {
> struct pci_dev *dsm_dev;
> struct tsm_dev *tsm_dev;
> struct pci_tdi *tdi;
> + struct device_evidence *evidence;
> };
>
> /**
> @@ -216,6 +226,10 @@ void pci_tsm_tdi_constructor(struct pci_dev *pdev, struct pci_tdi *tdi,
> ssize_t pci_tsm_guest_req(struct pci_dev *pdev, enum pci_tsm_req_scope scope,
> sockptr_t req_in, size_t in_len, sockptr_t req_out,
> size_t out_len, u64 *tsm_code);
> +static inline const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> +{
> + return tsm->tsm_dev->pci_ops;
> +}
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> {
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm/core.c
> similarity index 99%
> rename from drivers/pci/tsm.c
> rename to drivers/pci/tsm/core.c
This move + related makefile changes + exposing pci_tsm_rwsem are better be in a separate patch. Thanks,
> index 5fdcd7f2e820..220842df42bc 100644
> --- a/drivers/pci/tsm.c
> +++ b/drivers/pci/tsm/core.c
> @@ -15,13 +15,13 @@
> #include <linux/sysfs.h>
> #include <linux/tsm.h>
> #include <linux/xarray.h>
> -#include "pci.h"
> +#include "../pci.h"
>
> /*
> * Provide a read/write lock against the init / exit of pdev tsm
> * capabilities and arrival/departure of a TSM instance
> */
> -static DECLARE_RWSEM(pci_tsm_rwsem);
> +DECLARE_RWSEM(pci_tsm_rwsem);
>
> /*
> * Count of TSMs registered that support physical link operations vs device
> @@ -30,11 +30,6 @@ static DECLARE_RWSEM(pci_tsm_rwsem);
> static int pci_tsm_link_count;
> static int pci_tsm_devsec_count;
>
> -static const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> -{
> - return tsm->tsm_dev->pci_ops;
> -}
> -
> static inline bool is_dsm(struct pci_dev *pdev)
> {
> return pdev->tsm && pdev->tsm->dsm_dev == pdev;
> diff --git a/drivers/pci/tsm/evidence.c b/drivers/pci/tsm/evidence.c
> new file mode 100644
> index 000000000000..ffb08208f0c0
> --- /dev/null
> +++ b/drivers/pci/tsm/evidence.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2026 NVIDIA Corporation & Affiliates */
> +
> +#include <linux/device/evidence.h>
> +#include <linux/init.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/pci-tsm.h>
> +#include <linux/tsm.h>
> +
> +extern struct rw_semaphore pci_tsm_rwsem;
> +
> +static bool evidence_available(struct pci_dev *pdev)
> +{
> + return pdev->tsm && pdev->tsm->evidence;
> +}
> +
> +static struct device *pci_tsm_evidence_find_device(const char *name)
> +{
> + ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
> + if (ACQUIRE_ERR(rwsem_read_intr, &lock))
> + return NULL;
> +
> + struct device *dev __free(put_device) =
> + bus_find_device_by_name(&pci_bus_type, NULL, name);
> +
> + /*
> + * Bail evidence gathering early if we know at this point that
> + * the device has no valid evidence provider, but still need to
> + * revalidate the same in pci_tsm_evidence_read_begin().
> + */
> + if (!dev || !evidence_available(to_pci_dev(dev)))
> + return NULL;
> +
> + return no_free_ptr(dev);
> +}
> +
> +static struct device_evidence *pci_tsm_evidence_read_begin(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct device_evidence *evidence;
> + int rc;
> +
> + rc = down_read_interruptible(&pci_tsm_rwsem);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + if (!evidence_available(pdev))
> + goto err;
> +
> + /* Hold the evidence stable against conflicting refresh updates */
> + evidence = pdev->tsm->evidence;
> + rc = down_read_interruptible(&evidence->lock);
> + if (rc)
> + goto err;
> +
> + return evidence;
> +err:
> + up_read(&pci_tsm_rwsem);
> + return ERR_PTR(-ENXIO);
> +}
> +
> +static void pci_tsm_evidence_read_end(struct device_evidence *evidence)
> +{
> + up_read(&evidence->lock);
> + up_read(&pci_tsm_rwsem);
> +}
> +
> +static int pci_tsm_refresh_evidence(struct device *dev, const void *nonce,
> + size_t nonce_len)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct device_evidence *evidence;
> + const struct pci_tsm_ops *ops;
> + int rc;
> +
> + /* Sync against disconnect */
> + ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, &lock)))
> + return rc;
> +
> + if (!pdev->tsm)
> + return -ENXIO;
> +
> + ops = to_pci_tsm_ops(pdev->tsm);
> + if (!ops->refresh_evidence)
> + return -EOPNOTSUPP;
> +
> + /* Sync against pci_tsm_evidence_read_begin */
> + evidence = pdev->tsm->evidence;
> + ACQUIRE(rwsem_write_kill, elock)(&evidence->lock);
> + if ((rc = ACQUIRE_ERR(rwsem_write_kill, &elock)))
> + return rc;
> +
> + return ops->refresh_evidence(pdev->tsm, nonce, nonce_len);
> +}
> +
> +static const struct device_evidence_ops pci_tsm_evidence_ops = {
> + .subsys_name = "pci",
> + .find_device = pci_tsm_evidence_find_device,
> + .evidence_read_begin = pci_tsm_evidence_read_begin,
> + .evidence_read_end = pci_tsm_evidence_read_end,
> + .refresh_evidence = pci_tsm_refresh_evidence,
> +};
> +
> +static int __init pci_tsm_evidence_init(void)
> +{
> + return device_evidence_register(&pci_tsm_evidence_ops);
> +}
> +subsys_initcall(pci_tsm_evidence_init);
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb4f74957f69..6cefaddc1120 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27472,7 +27472,7 @@ F: Documentation/driver-api/coco/
> F: Documentation/driver-api/pci/tsm.rst
> F: Documentation/netlink/specs/device-evidence.yaml
> F: drivers/base/*evidence*
> -F: drivers/pci/tsm.c
> +F: drivers/pci/tsm/
> F: drivers/virt/coco/guest/
> F: include/uapi/linux/device-evidence.h
> F: include/linux/device/evidence.h
--
Alexey
^ permalink raw reply
* Re: [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: Nikunj A. Dadhania @ 2026-07-08 5:00 UTC (permalink / raw)
To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Andy Lutomirski, Peter Zijlstra, Juergen Gross,
Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-48-seanjc@google.com>
On 7/2/2026 1:02 AM, Sean Christopherson wrote:
> Silently ignore attempts to switch to a paravirt sched_clock when running
> as a CoCo guest with trusted TSC. In hand-wavy theory, a misbehaving
> hypervisor could attack the guest by manipulating the PV clock to affect
> guest scheduling in some weird and/or predictable way. More importantly,
> reading TSC on such platforms is faster than any PV clock, and sched_clock
> is all about speed.
>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kernel/tsc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 012321fed5e5..a146fc7b5e74 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
> int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
> void (*save)(void), void (*restore)(void))
> {
> + /*
> + * Don't replace TSC with a PV clock when running as a CoCo guest and
> + * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
> + * which isn't in the guest's TCB.
> + */
> + if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
> + boot_cpu_has(X86_FEATURE_TDX_GUEST))
> + return -EPERM;
> +
> if (!stable)
> clear_sched_clock_stable();
>
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Yan Zhao @ 2026-07-08 6:46 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-6-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:09PM -0700, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> backing for the 2MB region covering the passed pfn. However, multiple
> callers can concurrently operate on 4KB pages that fall within the same
> 2MB region. When this happens only one Dynamic PAMT page pair needs to be
What "this" stands for is not clear and a comma is missing after "happens".
> installed to cover the 2MB range. And when one page is freed, the Dynamic
> PAMT backing cannot be freed until all pages in the range are no longer in
> use. Make the helpers handle these races internally.
>
> Use the per-2MB refcounts from previous changes to track how many 4KB
> pages are in use within each region. Gate the actual Dynamic PAMT add and
> remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> check and SEAMCALL with a global spinlock so the read-decide-act sequence
> is atomic. This also avoids TDX module BUSY errors, as Dynamic PAMT add
> and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
"an internal TDX module locks" --> "internal TDX module locks"?
> so simultaneous attempts on the same region would conflict.
How about:
"This also avoids TDX module BUSY errors, as Dynamic PAMT add
and remove SEAMCALLs take internal TDX module locks for the 2MB ranges of
the specified PFN and the PAMT page pair PFNs, so simultaneous attempts on
the same 2MB ranges of the PFNs would conflict." ?
> The lock is global and heavyweight. Use simple conditional logic to keep
> correctness obvious. This will be optimized in a later change.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> v6:
> - Split from "x86/virt/tdx: Add tdx_alloc/free_control_page() helpers"
> - Return 0 instead of ret to be clearer (Binbin)
> - Clarify log (Nikolay)
> - Justify why the patch is not optimized in response to comments by
> (Nikolay)
> - Move tdx_find_pamt_refcount() to faciliate patch re-order
> - Adjustments from dropping error helper patches
> - Log tweaks
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 72 ++++++++++++++++++++++++++++---------
> 1 file changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 6658a6be6697c..50333eb96efa6 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -2043,10 +2043,14 @@ static u64 tdh_phymem_pamt_remove(kvm_pfn_t pfn, struct page **pamt_pages)
> return 0;
> }
>
> -/* Allocate PAMT memory for the given page */
> +/* Serializes adding/removing PAMT memory */
> +static DEFINE_SPINLOCK(pamt_lock);
> +
> +/* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
How about
"Bump the refcount of the PAMT page pair for the given PFN and add the PAMT
page pair on the first reference." ?
> static int tdx_pamt_get(kvm_pfn_t pfn)
> {
> struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> + atomic_t *pamt_refcount;
> u64 tdx_status;
> int ret;
>
> @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> if (ret)
> return ret;
>
> - tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> - if (tdx_status != TDX_SUCCESS) {
> - ret = -EIO;
> - goto out_free;
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
> +
> + scoped_guard(spinlock, &pamt_lock) {
> + /*
> + * If the pamt page is already added (i.e. refcount >= 1),
> + * then just increment the refcount.
> + */
> + if (atomic_read(pamt_refcount)) {
> + atomic_inc(pamt_refcount);
> + goto out_free;
> + }
> +
> + /* Try to add the pamt page and take the refcount 0->1. */
> + tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> + if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS)) {
> + ret = -EIO;
> + goto out_free;
> + }
> +
> + atomic_set(pamt_refcount, 1);
> }
>
> return 0;
> @@ -2069,26 +2089,46 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> return ret;
> }
>
> -/* Free PAMT memory for the given page */
> +/*
> + * Drop PAMT refcount for the given page and free PAMT memory if it is no
> + * longer needed.
How about:
"Drop the refcount of the PAMT page pair for the given PFN, and remove the
PAMT page pair if it is no longer needed." ?
> + */
> static void tdx_pamt_put(kvm_pfn_t pfn)
> {
> struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT] = {};
> + atomic_t *pamt_refcount;
> u64 tdx_status;
>
> if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> return;
>
> - tdx_status = tdh_phymem_pamt_remove(pfn, pamt_pages);
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
>
> - /*
> - * Don't free pamt_pages as it could hold garbage when
> - * tdh_phymem_pamt_remove() fails. Don't panic/BUG_ON(), as
> - * there is no risk of data corruption, but do yell loudly as
> - * failure indicates a kernel bug, memory is being leaked, and
> - * the dangling PAMT entry may cause future operations to fail.
> - */
> - if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS))
> - return;
> + scoped_guard(spinlock, &pamt_lock) {
> + /*
> + * If the there are more than 1 references on the pamt page,
s/If the/If
s/references/reference
> + * don't remove it yet. Just decrement the refcount.
> + */
> + if (atomic_read(pamt_refcount) > 1) {
> + atomic_dec(pamt_refcount);
> + return;
> + }
> +
> + /* Try to remove the pamt page and take the refcount 1->0. */
> + tdx_status = tdh_phymem_pamt_remove(pfn, pamt_pages);
> +
> + /*
> + * Don't free pamt_pages as it could hold garbage when
> + * tdh_phymem_pamt_remove() fails. Don't panic/BUG_ON(), as
> + * there is no risk of data corruption, but do yell loudly as
> + * failure indicates a kernel bug, memory is being leaked, and
> + * the dangling PAMT entry may cause future operations to fail.
> + */
> + if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS))
> + return;
> +
> + atomic_set(pamt_refcount, 0);
> + }
>
> free_pamt_array(pamt_pages);
> }
Another nit:
How about renaming the title from
"Handle concurrent callers in tdx_pamt_get/put()" to
"Handle concurrent calls to tdx_pamt_get/put()" ?
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Nikolay Borisov @ 2026-07-08 7:52 UTC (permalink / raw)
To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
Cc: djbw, kas, rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, peter.fang, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen, seanjc
In-Reply-To: <20260618081355.3253581-9-yilun.xu@linux.intel.com>
On 6/18/26 11:13, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
>
> During TDX attestation, the TDX guest asks the host to generate a
> signed, verifiable structure (a "Quote"). With the Quoting extension,
> the TDX module returns the Quote in pages that the host shares via an
> Extension-SEAMCALL.
nit: Just say the host provides the pages to the TDX module, no need to
specify if it's via an ext seamcall or some otherway. One can infer this
from the code itself.
>
> The SEAMCALL accepts the host buffer pages as a linked list of 4KB
> "HPA_LINKED_LIST" nodes. Each entry holds the physical address of a 4KB
> data page, except for the last entry, which points to the next node. The
> TDX module reports the required Quote buffer size through a global
> metadata field. See [1] for details.
That HPA_LINKED_LIST is basically a linked list of arrays, each node
into the linked list being the array, and each entry of that array being
the HPA witht he last being a pointer to the next HPA_LINKED_LIST, so in
a way a 2 level data structure. Can you be more explicit and just say
something along the lines of :
"SEAMCALL accepts host buffer pages arranged in a custom data structure,
consisting of nodes containing HPAs, arranged linearly, and each node is
linked to the next one via the last entry"
>
> For simplicity, let all guests share a global buffer. Build the buffer's
> HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> va-to-pa conversions at runtime.
>
> [1] Intel TDX Module ABI specification, Section "Physical Memory
> Management Types"
>
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx_global_metadata.h | 4 +
> arch/x86/virt/vmx/tdx/tdx.c | 115 +++++++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 14 +++
> 3 files changed, 129 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index b3442b7c88bb..17cb13a1bb40 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -57,4 +57,8 @@ struct tdx_sys_info_ext {
> bool ext_required;
> };
>
> +struct tdx_sys_info_quote {
> + u32 max_quote_size;
> +};
> +
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 06c42b86b05e..9716424a301f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -32,6 +32,7 @@
> #include <linux/idr.h>
> #include <linux/kvm_types.h>
> #include <linux/bitfield.h>
> +#include <linux/vmalloc.h>
> #include <asm/page.h>
> #include <asm/special_insns.h>
> #include <asm/msr-index.h>
> @@ -71,6 +72,24 @@ static LIST_HEAD(tdx_memlist);
>
> static struct tdx_sys_info tdx_sysinfo;
>
> +/*
> + * Quote buffer shared with the TDX module for quote generation, in HPA linked
> + * list format.
> + *
> + * @buf: Virtual address of the quote buffer.
> + * @buf_len: Size of @buf in bytes.
> + * @hpa_entries: HPA entries, starting at the first list node.
> + * @hpa_entries_pa: Physical address for @hpa_entries.
> + */
> +struct tdx_quote_data {
> + void *buf;
> + u64 buf_len;
> + u64 *hpa_entries;
> + phys_addr_t hpa_entries_pa;
> +};
> +
> +static struct tdx_quote_data tdx_quote;
> +
> static DEFINE_RAW_SPINLOCK(sysinit_lock);
>
> /*
> @@ -1167,6 +1186,81 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
> return 0;
> }
>
> +static inline phys_addr_t tdx_vmalloc_to_pa(const void *addr)
> +{
> + unsigned long pfn = vmalloc_to_pfn(addr);
> +
> + return PFN_PHYS(pfn);
> +}
> +
> +#define HPAS_PER_NODE (PAGE_SIZE / sizeof(u64))
> +
> +/*
> + * Pass the quote buffer to the TDX module as an HPA linked list, where each
> + * node holds 4KB page HPAs and the last entry points to the next node.
> + */
> +static __init int tdx_quote_create_buf(unsigned int npages,
> + struct tdx_quote_data *qdata)
> +{
> + unsigned int nnodes;
> + u64 *hpas;
> + void *qbuf;
> + int i, j;
> +
> + if (!npages)
> + return -EINVAL;
> +
> + /*
> + * Each node holds up to (HPAS_PER_NODE - 1) 4KB page HPAs.
> + * The last entry of the node points to the next node.
> + */
> + nnodes = DIV_ROUND_UP(npages, HPAS_PER_NODE - 1);
nit: It's somewhat arbitrary but num_nodes seems more explicit, saving 3
chars is not worth it.
> +
> + hpas = vmalloc_array(nnodes, PAGE_SIZE);
hpas is basically the starting node, so name it "nodes". Also I'm
slightly confused why you use vmalloc to allocate a contiguous address
space when the nodes are linked in a list? I.e you keep referring to
list in the changelog and the code seems to be working in an array chunks?
> + if (!hpas)
> + return -ENOMEM;
> +
> + /*
> + * ~0ULL is the list terminator for HPA_LINKED_LIST.
> + *
> + * Pre-fill the last node with 0xff bytes so that unused entries are
> + * terminators. Overwrite populated entries later.
> + */
> + memset((u8 *)hpas + (nnodes - 1) * PAGE_SIZE, 0xff, PAGE_SIZE);
> +
> + qbuf = vcalloc(npages, PAGE_SIZE);
> + if (!qbuf)
> + goto out_nomem;
'qbuf' is rather arbitrary here. You simply pre-allocate a bunch of
pages which you then initialize into the nodes. Can't this allocation be
moved inside the loop itself, of course it will increase the number of
allocs happening but this is during initialization so it's not
performance critical, yes?
> +
> + /* Populate the linked list */
> + for (i = 0, j = 0; j < npages; i++) {
nit: This feedback was given before, but both variables can be defined
in this loop.
> + if ((i % HPAS_PER_NODE) == HPAS_PER_NODE - 1) {
> + /*
> + * The last node entry always points to the next node.
> + * The address of the following entry must be on next
> + * node's page boundary.
> + */
> + hpas[i] = tdx_vmalloc_to_pa(&hpas[i + 1]);
> + continue;
> + }
> +
> + hpas[i] = tdx_vmalloc_to_pa((u8 *)qbuf + j * PAGE_SIZE);
> + j++;
> + }
> +> + qdata->buf = qbuf;
> + qdata->buf_len = (u64)npages * PAGE_SIZE;
> + qdata->hpa_entries = hpas;
> + qdata->hpa_entries_pa = tdx_vmalloc_to_pa(hpas);
> +
> + return 0;
> +
> +out_nomem:
> + vfree(hpas);
> +
> + return -ENOMEM;
> +}
> +
> /* Initialize quoting extension */
> static __init int tdx_quote_init(void)
> {
> @@ -1185,12 +1279,25 @@ static __init int tdx_quote_init(void)
>
> static __init void init_tdx_quoting_extension(void)
> {
> - int ret;
> + struct tdx_sys_info_quote sysinfo_quote;
> + unsigned int nr_quote_pages;
> +
> + if (!(tdx_addon_feature0 & TDX_FEATURES0_QUOTE))
> + return;
>
> - if (tdx_addon_feature0 & TDX_FEATURES0_QUOTE) {
> - ret = tdx_quote_init();
> - WARN_ON_ONCE(ret);
> + if (tdx_quote_init()) {
> + WARN_ON_ONCE(1);
> + return;
> }
> +
> + /* Quoting metadata is valid only after initialization */
> + if (get_tdx_sys_info_quote(&sysinfo_quote))
> + return;
> +
> + nr_quote_pages = PAGE_ALIGN(sysinfo_quote.max_quote_size) /
> + PAGE_SIZE;
> + if (tdx_quote_create_buf(nr_quote_pages, &tdx_quote))
> + pr_err("Failed to create quote buffer\n");
> }
>
> /* Initialize TDX module extensions for extension SEAMCALLs */
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 84364da89649..1eb2985307c6 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -151,3 +151,17 @@ static int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
>
> return 0;
> }
> +
> +static __init int get_tdx_sys_info_quote(struct tdx_sys_info_quote *sysinfo_quote)
> +{
> + int ret;
> + u64 val;
> +
> + ret = read_sys_metadata_field(0x2300000200000002, &val);
> + if (ret)
> + return ret;
> +
> + sysinfo_quote->max_quote_size = val;
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Chao Gao @ 2026-07-08 8:35 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, yan.y.zhao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-10-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:13PM -0700, Rick Edgecombe wrote:
>From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
>Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
>adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
>removing guest memory (PAGE.REMOVE).
>
>To access the per-vCPU PAMT caches without plumbing @vcpu throughout the
>TDP MMU, begrudgingly use kvm_get_running_vcpu() to get the vCPU, and bug
>the VM if KVM attempts to set an S-EPT leaf without an active vCPU. KVM
>only supports creating _new_ mappings in page (pre)fault paths, all of
>which require an active vCPU.
>
>The PAMT memory holds metadata for TDX-protected memory. With Dynamic
>PAMT, PAMT_4K is allocated on demand. The kernel supplies the TDX module
>with a few pages that cover 2M of host physical memory.
>
>Releases are balanced via tdx_pamt_put(): every control-page free goes
>through tdx_free_control_page(), and guest data pages are put directly on
>the successful tdh_mem_page_remove() path and in the
>tdx_mem_page_add/aug() error path.
>
>Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
>Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>Co-developed-by: Sean Christopherson <seanjc@google.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>---
>v6:
> - Don't have topup op take a min param (Yan, Sean)
The topup_external_cache x86 op still takes int min_nr_spts. Did you
forget to remove it, or am I misreading this changelog?
^ permalink raw reply
* Re: [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put()
From: Yan Zhao @ 2026-07-08 8:46 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-7-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:10PM -0700, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> The Dynamic PAMT get/put helpers use a global spinlock to serialize all
> refcount updates and SEAMCALL invocations. This gives correct behavior for
> concurrent callers, but leads to contention. It is especially bad from the
> KVM side, which is designed to allow faulting in EPT under a shared lock.
> With the global spinlock, not only is the lock an exclusive one, but it is
> for all TDs instead of just a single one.
>
> But taking the global lock each time is actually unnecessary. Only the 0->1
> and 1->0 refcount transitions actually need the lock (to pair with
> SEAMCALLs that actually add and remove with the Dynamic PAMT pages). The
> common case of incrementing or decrementing a non-zero refcount can be
> done locklessly.
>
> So create a fast and slow path. Check the refcount outside the lock and
> only take it for the slowpath (0->1 and 1->0 transitions).
>
> On the put side make the refcount adjustment and lock taking atomic so if
> a 'get' happens between them, it doesn't cause the Dynamic PAMT to be
> freed incorrectly. On the get side there is no technique for doing the
> refcount adjustment and lock atomically, so check the refcount again
> inside the lock.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
The optimization LGTM.
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Yan Zhao @ 2026-07-08 9:13 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-8-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:11PM -0700, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Use control page helpers for allocating and freeing TD control structures,
> such these operations can work for Dynamic PAMT.
>
> The TDX module tracks some state for each page of physical memory that it
> might use. It calls this state the PAMT. It includes separate state for
Nit: It records this state in the PAMT ?
> each page size a physical page could be utilized at within the TDX module
> (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> allocated dynamically. So the kernel must install PAMT backing for each 4KB
How about
"..., only the backend to hold the 4KB page size state is allocated
dynamically" ?
> page before gifting it to the TDX module, and tear it down after the page
> is reclaimed.
How about
"So the kernel must check whether it is necessary to install/remove PAMT
backing for each 4KB page, and do so when necessary before gifting the page
to the TDX module or after it is reclaimed." ?
> TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> TDCX) are all handed to the TDX module at 4KB page size and are therefore
> subject to this requirement. Replace the raw alloc_page()/__free_page()
> calls for these pages with tdx_alloc/free_control_page().
>
> Switching between special Dynamic PAMT operations or normal page
> alloc/free operations is handled internally in
> tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> calls. Just call them unconditionally. Similarly, drop the NULL checks
> before freeing, as tdx_free_control_page() handles NULL internally.
>
> No functional change intended when Dynamic PAMT is not in use.
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Peter Fang @ 2026-07-08 9:25 UTC (permalink / raw)
To: Dave Hansen
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, seanjc
In-Reply-To: <e06806e8-7334-4533-b1a3-e1efbe3fab8a@intel.com>
On Wed, Jul 01, 2026 at 12:56:19PM -0700, Dave Hansen wrote:
> On 6/18/26 01:13, Xu Yilun wrote:
> > For simplicity, let all guests share a global buffer. Build the buffer's
> > HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> > va-to-pa conversions at runtime.
>
> va-to-pa conversions are not expensive. Even for vmalloc() memory.If
>
> I don't like the global buffer. It just generally seems like a bad idea.
> It needs locking, it wastes memory when nobody is doing quotes, etc..
>
> Is there a way to do this without vmalloc()?
>
> Could userspace provide the memory and then the kernel just does a gup
> to keep it in place while the TDX module is writing to it? Or, could it
> just be allocated:
Hi Dave,
The GUP idea makes a lot of sense at a high level I think. However, the
shared buffer used by the GHCI call is not 4K-aligned because it has a
header in front of it, while the SEAMCALL expects 4K-aligned HPAs
(lesson learned...). This memory does come from userspace though:
struct tdx_quote_req { <-- 4K-aligned
u64 version;
u64 status;
u32 in_len;
u32 out_len;
u8 data[]; <-- shared buffer
};
So perhaps the simplest thing here is to just allocate a kernel buffer
before the SEAMCALL. There's also another subtle wrinkle here: KVM is
shifting from a FOLL_PIN-based API to an MMU-notifier-based API for
guest page accesses [1], so I think that means more complex code to talk
to the TDX module.
>
> void *buffer = vmalloc(size);
> mutex_lock();
> // talk to TDX module
> mutex_unlock();
> vfree(buffer);
>
> return ret;
>
> That actually has some nice properties. It doesn't require locking for
> the buffer. It also penalizes folks doing lots of quotes. If you hammer
> on quoting, you hammer on the memory allocator and TLB flushing code and
> slow yourself down more. If you hammer on it from multiple threads, it
> gets even worse.
>
> That seems like a feature not a bug.
This is a really good point... I'm a bit worried about the global impact
of vfree() though. IIUC it eventually triggers an all-CPU TLB shootdown,
so a guest abusing this path would not only throttle itself, but could
also hurt everyone else. This might be more useful if the buffer was
allocated in userspace? A free() in userspace usually triggers a more
limited TLB shootdown.
I think I can use alloc_pages_bulk() instead to avoid the TLB shootdown
cost altogether. It's a bit more work in KVM, but maybe worth it...
Thanks,
Peter
[1] https://lore.kernel.org/kvm/20211115165030.7422-8-dwmw2@infradead.org/
^ permalink raw reply
* Re: [PATCH 14/15] PCI/TSM: Create MMIO descriptors via TDISP Report
From: Alexey Kardashevskiy @ 2026-07-08 9:49 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Arnd Bergmann, Xu Yilun
In-Reply-To: <20260705220819.2472765-15-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> After pci_tsm_bind() and pci_tsm_lock() the low level TSM driver is
> expected to populate PCI_TSM_EVIDENCE_TYPE_REPORT in its evidence store.
> This report is defined by the TDISP GET_DEVICE_INTERFACE_REPORT response
> payload.
>
> Add a helper to create encrypted MMIO descriptors from that report
> data. With those descriptors the TSM driver can use pci_tsm_mmio_setup() to
> inform ioremap() how to map the device per the device's expectations. The
> VM is expected to validate the interface with the relying party before
> accepting the device for operation.
>
> The helper also provides the obfuscated starting address for each encrypted
> MMIO range as the VM is never disclosed on the hpa that correlates to the
> gpa of the device's mmio. The obfuscated address is BAR aligned.
>
> Based on an original patch by Aneesh [1]
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@kernel.org/ [1]
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> include/linux/ioport.h | 2 +
> include/linux/pci-tsm.h | 35 +++++++
> drivers/pci/tsm/core.c | 224 ++++++++++++++++++++++++++++++++++++++++
> kernel/resource.c | 8 ++
> 4 files changed, 269 insertions(+)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index f7930b3dfd0a..122f1eefb4b9 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -144,6 +144,7 @@ enum {
> IORES_DESC_RESERVED = 7,
> IORES_DESC_SOFT_RESERVED = 8,
> IORES_DESC_CXL = 9,
> + IORES_DESC_ENCRYPTED = 10,
> };
>
> /*
> @@ -240,6 +241,7 @@ struct resource_constraint {
> extern struct resource ioport_resource;
> extern struct resource iomem_resource;
> extern struct resource soft_reserve_resource;
> +extern struct resource encrypted_iomem_resource;
>
> extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
> extern int request_resource(struct resource *root, struct resource *new);
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index 397e5d8459cb..6d5fadd79360 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -156,12 +156,42 @@ struct pci_tsm_pf0 {
> struct pci_doe_mb *doe_mb;
> };
>
> +/**
> + * struct pci_tsm_mmio_entry - an encrypted MMIO range
> + * @res: MMIO address range (typically Guest Physical Address, GPA)
> + * @tsm_offset: Host Physical Address, HPA obfuscation offset added by the TSM.
> + * Translates report addresses to GPA.
> + */
> +struct pci_tsm_mmio_entry {
> + struct resource res;
> + u64 tsm_offset;
> +};
> +
> +struct pci_tsm_mmio {
> + int nr;
> + struct pci_tsm_mmio_entry mmio[];
> +};
> +
> +static inline struct pci_tsm_mmio_entry *
> +pci_tsm_mmio_entry(struct pci_tsm_mmio *mmio, int idx)
> +{
> + return &mmio->mmio[idx];
> +}
> +
> +static inline struct resource *pci_tsm_mmio_resource(struct pci_tsm_mmio *mmio,
> + int idx)
> +{
> + return &mmio->mmio[idx].res;
> +}
> +
> /**
> * struct pci_tsm_devsec - context for tracking private/accepted PCI resources
> * @base_tsm: generic core "tsm" context
> + * @mmio: encrypted MMIO resources for this assigned device
> */
> struct pci_tsm_devsec {
> struct pci_tsm base_tsm;
> + struct pci_tsm_mmio *mmio;
> };
>
> /* physical function0 and capable of 'connect' */
> @@ -263,6 +293,11 @@ static inline const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> return tsm->tsm_dev->pci_ops;
> }
> struct pci_tsm_devsec *to_pci_tsm_devsec(struct pci_tsm *tsm);
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio);
> +
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev);
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> {
> diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
> index 372363a39a44..9ac216ad896d 100644
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c
> @@ -15,6 +15,7 @@
> #include <linux/pci-tsm.h>
> #include <linux/sysfs.h>
> #include <linux/tsm.h>
> +#include <linux/unaligned.h>
> #include <linux/xarray.h>
> #include "../pci.h"
>
> @@ -552,6 +553,229 @@ static ssize_t dsm_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(dsm);
>
> +static void mmio_teardown(struct pci_tsm_mmio *mmio, int nr)
> +{
> + while (nr--)
> + remove_resource(pci_tsm_mmio_resource(mmio, nr));
> +}
> +
> +/**
> + * pci_tsm_mmio_setup() - mark device MMIO as encrypted in iomem
> + * @pdev: device owner of MMIO resources
> + * @mmio: container of an array of resources to mark encrypted
> + */
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + int i;
> +
> + device_lock_assert(&pdev->dev);
> + if (pdev->dev.driver)
> + return -EBUSY;
> +
> + for (i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> + int j;
> +
> + if (resource_size(res) == 0 || !res->end)
> + break;
> +
> + /* Only require the caller to set the range, init remainder */
> + *res = DEFINE_RES_NAMED_DESC(res->start, resource_size(res),
> + pci_name(pdev), IORESOURCE_MEM,
> + IORES_DESC_ENCRYPTED);
> +
> + for (j = 0; j < PCI_NUM_RESOURCES; j++)
> + if (resource_contains(pci_resource_n(pdev, j), res))
> + break;
> +
> + /* Request is outside of device MMIO */
> + if (j >= PCI_NUM_RESOURCES)
> + break;
> +
> + if (insert_resource(&encrypted_iomem_resource, res) != 0)
> + break;
> + }
> +
> + if (i >= mmio->nr)
> + return 0;
> +
> + mmio_teardown(mmio, i);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_setup);
> +
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio)
> +{
> + mmio_teardown(mmio, mmio->nr);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_teardown);
> +
> +/*
> + * PCIe ECN TEE Device Interface Security Protocol (TDISP)
> + *
> + * Device Interface Report data object layout as defined by PCIe r7.0 section
> + * 11.3.11
> + */
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_TABLE BIT(0)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_PBA BIT(1)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE BIT(2)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
> +
> +/* An interface report 'pfn' is 4K in size */
> +struct pci_tsm_devif_mmio {
> + __le64 phys;
> + __le32 nr_pfns;
> + __le32 attributes;
> +};
> +
> +struct pci_tsm_devif_report {
> + __le16 interface_info;
> + __le16 reserved;
> + __le16 msi_x_message_control;
> + __le16 lnr_control;
> + __le32 tph_control;
> + __le32 mmio_range_count;
> + struct pci_tsm_devif_mmio mmio[];
> +};
> +
> +/**
> + * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
> + * @pdev: device owner of MMIO ranges
> + *
> + * Return: the encrypted MMIO range descriptor on success, NULL on failure
> + *
> + * Assumes that this is called within the live lifetime of a PCI device's
> + * association with a low level TSM.
> + */
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +{
> + struct device_evidence *evidence = pdev->tsm->evidence;
> + u64 reporting_bar_base, last_reporting_end;
> + struct device_evidence_object *report_obj;
> + const struct pci_tsm_devif_report *report;
> + u32 mmio_range_count;
> + int last_bar = -1;
> + int i;
> +
> + if (!evidence)
> + return NULL;
> +
> + report_obj = &evidence->obj[DEVICE_EVIDENCE_TYPE_REPORT];
> +
> + guard(rwsem_read)(&evidence->lock);
> + if (report_obj->len < sizeof(struct pci_tsm_devif_report))
> + return NULL;
> +
> + report = report_obj->data;
> + mmio_range_count = __le32_to_cpu(report->mmio_range_count);
> +
> + /* check that the report object is self-consistent on mmio entries */
> + if (report_obj->len < struct_size(report, mmio, mmio_range_count))
> + return NULL;
> +
> + /* create pci_tsm_mmio descriptors from the report data */
> + struct pci_tsm_mmio *mmio __free(kfree) =
> + kzalloc_flex(*mmio, mmio, mmio_range_count);
> + if (!mmio)
> + return NULL;
> +
> + for (i = 0; i < mmio_range_count; i++) {
> + u64 range_off;
> + struct range range;
> + const struct pci_tsm_devif_mmio *mmio_data = &report->mmio[i];
> + struct pci_tsm_mmio_entry *entry =
> + pci_tsm_mmio_entry(mmio, mmio->nr);
> + u64 tsm_offset = __le64_to_cpu(mmio_data->phys);
* SZ_4K is missing.
> + u64 size = __le32_to_cpu(mmio_data->nr_pfns) * SZ_4K;
> + u32 attr = __le32_to_cpu(mmio_data->attributes);
> + int bar = FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID,
> + attr);
> +
> + if (bar >= PCI_STD_NUM_BARS ||
> + !(pci_resource_flags(pdev, bar) & IORESOURCE_MEM) ||
> + (pci_resource_flags(pdev, bar) & IORESOURCE_UNSET)) {
> + pci_dbg(pdev, "Invalid reporting bar ID %d\n", bar);
> + return NULL;
> + }
> +
> + if (last_bar > bar) {
> + pci_dbg(pdev, "Reporting bar ID not in ascending order\n");
> + return NULL;
> + }
> +
> + if (last_bar < bar) {
> + unsigned long mask = pci_resource_len(pdev, bar) - 1;
> +
> + /* Transition to a new bar */
> + last_bar = bar;
> +
> + /*
> + * Determine the obfuscated base of the BAR. BAR
> + * offsets are never obfuscated.
> + */
> + reporting_bar_base = tsm_offset & ~mask;
> + } else if (tsm_offset < last_reporting_end) {
> + pci_dbg(pdev, "Reporting ranges within BAR not in ascending order\n");
> + return NULL;
> + }
> +
> + /* Per spec the tsm_offset never results in overflow / underflow */
> + last_reporting_end = tsm_offset + size;
> + if (last_reporting_end < tsm_offset) {
> + pci_dbg(pdev, "Reporting range overflow\n");
> + return NULL;
> + }
> +
> + range_off = tsm_offset - reporting_bar_base;
If we all do the mmio_reporting_offset in the same way (use the bar size low bits), then this can be simplified and should be as it does not account for the non-locked (==not reported) MSIX anyway. Thanks,
> + if (pci_resource_len(pdev, bar) < range_off + size) {
> + pci_dbg(pdev, "Reporting range larger than BAR size\n");
> + return NULL;
> + }
> +
> + range.start = pci_resource_start(pdev, bar) + range_off;
> + range.end = range.start + size - 1;
> +
> + /* Only record the TEE ranges for later consideration by ioremap() */
> + if (FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE,
> + attr)) {
> + pci_dbg(pdev, "Skipping non-TEE range, BAR%d %pra\n",
> + bar, &range);
> + continue;
> + }
> +
> + entry->res.start = range.start;
> + entry->res.end = range.end;
> + entry->tsm_offset = tsm_offset;
> + mmio->nr++;
> + }
> +
> + return_ptr(mmio);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_alloc);
> +
> +/**
> + * pci_tsm_mmio_free() - free a pci_tsm_mmio instance
> + * @pdev: device owner of MMIO ranges
> + * @mmio: instance to free
> + *
> + * Returns 0 if @mmio was idle on entry, -EBUSY otherwise
> + */
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + for (int i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> +
> + if (dev_WARN_ONCE(&pdev->dev, resource_assigned(res),
> + "MMIO resource still assigned %pr\n", res))
> + return -EBUSY;
> + }
> + kfree(mmio);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_free);
> +
> /**
> * pci_tsm_accept() - accept a device for private MMIO operation
> * @pdev: PCI device to accept
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 3d17e3196a3e..ba91ce43ff7a 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -56,6 +56,14 @@ struct resource soft_reserve_resource = {
> .flags = IORESOURCE_MEM,
> };
>
> +struct resource encrypted_iomem_resource = {
> + .name = "Encrypted MMIO",
> + .start = 0,
> + .end = -1,
> + .desc = IORES_DESC_ENCRYPTED,
> + .flags = IORESOURCE_MEM,
> +};
> +
> static DEFINE_RWLOCK(resource_lock);
>
> /*
--
Alexey
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Catalin Marinas @ 2026-07-08 11:35 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Jiri Pirko, Jason Gunthorpe, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260701054926.825925-17-aneesh.kumar@kernel.org>
On Wed, Jul 01, 2026 at 11:19:20AM +0530, Aneesh Kumar K.V (Arm) wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 97987f850a33..acf67c7064db 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
> unsigned int flags = SWIOTLB_VERBOSE;
> bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
>
> - if (is_realm_world()) {
> + if (is_realm_world())
> swiotlb = true;
> - flags |= SWIOTLB_FORCE;
> - }
For this part:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> index e05dc7649366..f3fc28f352ba 100644
> --- a/kernel/dma/direct.h
> +++ b/kernel/dma/direct.h
> @@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
> {
> dma_addr_t dma_addr;
>
> + /*
> + * For a device requiring unencrypted DMA, MMIO memory is treated
> + * as shared by default.
> + */
> + if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
> + attrs |= DMA_ATTR_CC_SHARED;
> +
> if (is_swiotlb_force_bounce(dev)) {
> - if (!(attrs & DMA_ATTR_CC_SHARED)) {
> - if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> - return DMA_MAPPING_ERROR;
> + if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> + return DMA_MAPPING_ERROR;
>
> - return swiotlb_map(dev, phys, size, dir, attrs);
> - }
> - } else if (attrs & DMA_ATTR_CC_SHARED) {
> - return DMA_MAPPING_ERROR;
> + return swiotlb_map(dev, phys, size, dir, attrs);
> }
>
> - if (attrs & DMA_ATTR_MMIO) {
> - dma_addr = phys;
> - if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
> - goto err_overflow;
> - } else if (attrs & DMA_ATTR_CC_SHARED) {
> + if (attrs & DMA_ATTR_CC_SHARED)
> dma_addr = phys_to_dma_unencrypted(dev, phys);
> + else
> + dma_addr = phys_to_dma_encrypted(dev, phys);
For AMD/SME, on host with memory encryption we now end up setting the C
bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
some other MMIO bus understands this attribute. Maybe we should stick to
something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
since this is not universally defined, just use the old dma_addr = phys
if MMIO and ignore any unlikely DMA offsets.
In the other case, for an arm CCA guest, if the MMIO is shared we end up
setting the shared attribute but that's fine, it's only an IPA address.
--
Catalin
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Suzuki K Poulose @ 2026-07-08 12:11 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco
Cc: Robin Murphy, Marek Szyprowski, Will Deacon, Marc Zyngier,
Steven Price, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260701054926.825925-17-aneesh.kumar@kernel.org>
On 01/07/2026 06:49, Aneesh Kumar K.V (Arm) wrote:
> Teach dma_direct_map_phys() to select the DMA address encoding based on
> DMA_ATTR_CC_SHARED.
>
> Use phys_to_dma_unencrypted() for decrypted mappings and
> phys_to_dma_encrypted() otherwise. If a device requires unencrypted DMA
> but the source physical address is still encrypted, force the mapping
> through swiotlb so the DMA address and backing memory attributes remain
> consistent.
>
> Update the arm64, x86, s390 and powerpc secure-guest setup to not use
> swiotlb force option
>
> Tested-by: Jiri Pirko <jiri@nvidia.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> Changes from v3:
> * Handle DMA_ATTR_MMIO
> ---
> arch/arm64/mm/init.c | 4 +--
> arch/powerpc/platforms/pseries/svm.c | 2 +-
> arch/s390/mm/init.c | 2 +-
> arch/x86/kernel/pci-dma.c | 4 +--
> kernel/dma/direct.c | 4 ++-
> kernel/dma/direct.h | 45 +++++++++++++++-------------
> 6 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 97987f850a33..acf67c7064db 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
> unsigned int flags = SWIOTLB_VERBOSE;
> bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
>
> - if (is_realm_world()) {
> + if (is_realm_world())
> swiotlb = true;
> - flags |= SWIOTLB_FORCE;
> - }
>
> if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb) {
> /*
For arm64 CCA bits:
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply
* Re: [PATCH 01/15] netlink: specs: Introduce multi-message blobs for SPDM
From: Donald Hunter @ 2026-07-08 11:13 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-2-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> The SPDM, Security Protocol and Data Model, underpins PCI device security
> and other use cases. It defines objects that allow for verification of
> device identity and configuration. These objects can be large in size 16MB.
> Netlink is otherwise suitable to define the operations, with optional
> parameters, and notifications for working with these objects. For example,
> operations like "regenerate evidence with nonce", "mark evidence
> validated", and "broadcast evidence / security state change events".
>
> A netlink 'blob' is introduced as a way to teach YNL that one instance of a
> attribute may span multiple messages. It enables netlink to convey all the
> data needed for verification and manipulation of SPDM transported evidence.
>
> The schema change to allows YNL to infer that an attribute may span
change to allows -> change allows
> multiple messages and interrogate its length to preallocate an
> appropriately sized receive buffer.
>
> The design direction to extend the netlink schema for a "multi-message
> object receive" case was the result of this discussion [1].
>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Link: http://lore.kernel.org/20260318170014.6650d2bf@kernel.org [1]
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> Documentation/netlink/genetlink-legacy.yaml | 6 ++++++
> Documentation/netlink/genetlink.yaml | 7 +++++++
> Documentation/netlink/netlink-raw.yaml | 7 +++++++
I suggest you drop the changes to genetlink-legacy and netlink-raw
because I don't think we want to support the blob functionality for
legacy families.
Can you also update Documentation/userspace-api/netlink/specs.rst to
describe the multi-attr / bloblen API behaviour.
^ permalink raw reply
* Re: [PATCH 04/15] device core: Introduce "device evidence" over netlink
From: Donald Hunter @ 2026-07-08 13:22 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski, Bjorn Helgaas, Xu Yilun,
Aneesh Kumar K.V (Arm), Alexey Kardashevskiy
In-Reply-To: <20260705220819.2472765-5-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> Multiple device security mechanisms are built on top of the SPDM protocol
> from the DMTF. The protocol arranges for devices to verify their identity
> and establish secure sessions between requesters and responders. In turn,
> device security protocols like PCIe IDE use SPDM sessions to program link
> encryption keys, and protocols like PCIe TDISP use exclusive secure
> sessions for managing the device's security state.
>
> All of these uses share a need to retrieve the device's installed
> certificates, its measurements, and other protocol specific objects like
> interface reports. Unify all of those object retrieval and regeneration
> cases into a device-scoped ABI that buses can leverage.
>
> When PCI CMA was presented at Linux Plumbers the consensus reaction was
> that it was not suitable to be driven via sysfs. Given the varied use
> cases of the same fundamental objects the consensus converged on
> netlink. With the new 'blob' extensions to netlink, it can support up to
> the 16MB signed transcripts that SPDM generates.
>
> A bus opts in to evidence gathering via device_evidence_register() and
> passing device_evidence_ops. The ops resolve a device handle to its
> corresponding 'struct device_evidence' context.
>
> See inline documentation in
> Documentation/netlink/specs/device-evidence.yaml for the object types and
> options. This includes objects like TDISP interface reports and options
> like digests instead of full blobs.
Can you include sample pyynl output; it helps immensely during review.
>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
You probably need to cc the netdev list so that the series gets picked
up by patchwork.
> ---
> drivers/base/Kconfig | 4 +
> drivers/base/Makefile | 1 +
> .../netlink/specs/device-evidence.yaml | 186 +++++++
> drivers/base/device-evidence-netlink.h | 23 +
> include/linux/device/evidence.h | 85 ++++
> include/uapi/linux/device-evidence.h | 110 +++++
> drivers/base/device-evidence-netlink.c | 44 ++
> drivers/base/evidence.c | 453 ++++++++++++++++++
> MAINTAINERS | 4 +
> 9 files changed, 910 insertions(+)
> create mode 100644 Documentation/netlink/specs/device-evidence.yaml
> create mode 100644 drivers/base/device-evidence-netlink.h
> create mode 100644 include/linux/device/evidence.h
> create mode 100644 include/uapi/linux/device-evidence.h
> create mode 100644 drivers/base/device-evidence-netlink.c
> create mode 100644 drivers/base/evidence.c
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index f7d385cbd3ba..e3929fe6b240 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -190,6 +190,10 @@ config HMEM_REPORTING
> Enable reporting for heterogeneous memory access attributes under
> their non-uniform memory nodes.
>
> +config DEVICE_EVIDENCE
> + bool
> + depends on NET
> +
> source "drivers/base/test/Kconfig"
>
> config SYS_HYPERVISOR
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 8074a10183dc..02bdc4f74019 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> +obj-$(CONFIG_DEVICE_EVIDENCE) += evidence.o device-evidence-netlink.o
> obj-$(CONFIG_ACPI) += physical_location.o
>
> obj-y += test/
> diff --git a/Documentation/netlink/specs/device-evidence.yaml b/Documentation/netlink/specs/device-evidence.yaml
> new file mode 100644
> index 000000000000..44b278bc20e1
> --- /dev/null
> +++ b/Documentation/netlink/specs/device-evidence.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +#
> +---
> +name: device-evidence
The naming seems to be too narrow - if the scope expands at all then the
name would need to change. A subsystem name like spdm, tsm or tsm-device
seems better?
> +protocol: genetlink
> +uapi-header: linux/device-evidence.h
> +doc: Device evidence retrieval over generic netlink
> +
> +definitions:
> + -
> + type: const
> + name: max-object-size
> + value: 0x01000000
> + -
> + type: const
> + name: max-nonce-size
> + value: 32
> + -
> + name: type
> + type: enum
> + doc: Device security evidence objects
> + render-max: true
> + entries:
> + -
> + name: cert0
> + doc: SPDM certificate chain from device slot0
> + -
> + name: cert1
> + doc: SPDM certificate chain from device slot1
> + -
> + name: cert2
> + doc: SPDM certificate chain from device slot2
> + -
> + name: cert3
> + doc: SPDM certificate chain from device slot3
> + -
> + name: cert4
> + doc: SPDM certificate chain from device slot4
> + -
> + name: cert5
> + doc: SPDM certificate chain from device slot5
> + -
> + name: cert6
> + doc: SPDM certificate chain from device slot6
> + -
> + name: cert7
> + doc: SPDM certificate chain from device slot7
> + -
> + name: vca
> + doc: |
> + SPDM version, capabilities, and algorithms transcript
> + negotiated at session establishment. An implementation may not
> + provide this separately and instead include it in the
> + measurements transcript.
> + -
> + name: measurements
> + doc: SPDM GET_MEASUREMENTS response
> + -
> + name: report
> + doc: |
> + A bus that implements a device interface security protocol
> + like TDISP may convey an interface report that details
> + interface settings and capabilities.
> + -
> + name: type-flag
> + type: flags
> + doc: Device security evidence request flags
> + render-max: true
> + # NOTE! these values must match the type enum name and order
> + entries:
> + -
> + name: cert0
> + -
> + name: cert1
> + -
> + name: cert2
> + -
> + name: cert3
> + -
> + name: cert4
> + -
> + name: cert5
> + -
> + name: cert6
> + -
> + name: cert7
> + -
> + name: vca
> + -
> + name: measurements
> + -
> + name: report
> + -
> + name: flag
> + type: flags
> + render-max: true
> + doc: Flags to control evidence retrieval
> + entries:
> + -
> + name: digest
> + doc: |
> + Request a secure hash of objects like vca and measurements.
> + The expectation is that this digest is produced by a responder
> + within the TCB like a platform TSM. It validates a blob that
> + may have traversed a transport without integrity protections.
> +
> +attribute-sets:
> + -
> + name: object
> + attributes:
> + -
> + name: type
> + type: u32
Is this an instance of the type enum? If so then:
enum: type
> + doc: evidence type-id
> + -
> + name: type-mask
> + type: u32
Likewise:
enum: type-flag
> + doc: evidence types as a flag mask
> + -
> + name: flags
> + type: u32
And:
enum: flags
> + doc: evidence modifier flags like 'request object digest'
> + -
> + name: subsys
> + type: string
> + doc: device subsystem name (e.g. bus or class name)
> + -
> + name: dev-name
> + type: string
> + doc: device name
> + -
> + name: nonce
> + type: binary
> + checks:
> + max-len: max-nonce-size
> + -
> + name: generation
> + type: u32
> + doc: detect local threads racing evidence refresh
> + -
> + name: count
> + type: u64
> + doc: TSM, if present, observed count of evidence refresh events
> + -
> + name: length
> + type: u32
> + checks:
> + max: max-object-size
> + doc: |
> + Final size of 'val' blob after all messages received.
> + -
> + name: val
Any reason for the abbreviated name?
> + type: binary
> + multi-attr: true
> + bloblen: length
> + doc: |
> + Ordered evidence object value chunk. Large evidence objects
> + may be split over several dump reply messages up to 'length'.
> + 'length' is transmitted before the first chunk.
> +
> +operations:
> + list:
> + -
> + name: read
> + doc: |
> + Read device evidence objects of a given type mask. The dump reply
> + reports the total value length before val attributes fill the evidence
> + object, possibly across multiple messages.
> + attribute-set: object
> + flags: [admin-perm]
> + dump:
> + pre: device-evidence-nl-read-pre
> + post: device-evidence-nl-read-post
> + request:
> + attributes:
> + - type-mask
> + - flags
> + - subsys
> + - dev-name
> + - nonce
> + reply:
> + attributes:
> + - type
> + - generation
> + - length
> + - val
Is the intention that the dump filter must narrow to a single device? Is
the user expected to enumerate the devices in subsystem then read device
evidence for each in turn?
^ permalink raw reply
* Re: [PATCH 01/15] netlink: specs: Introduce multi-message blobs for SPDM
From: Donald Hunter @ 2026-07-08 13:23 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-2-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
> index d3f3f3399ddf..4caee7d55303 100644
> --- a/Documentation/netlink/genetlink.yaml
> +++ b/Documentation/netlink/genetlink.yaml
> @@ -153,6 +153,13 @@ properties:
> enum: [ little-endian, big-endian ]
> multi-attr:
> type: boolean
> + bloblen:
> + description: |
> + Marks this attribute as a variable-length blob that may be split across
> + multiple messages. The value names a separate scalar attribute that carries
> + the total blob length, and which is sent ahead of the blob data so a consumer
> + can preallocate the full payload.
> + type: string
> nested-attributes:
> description: Name of the space (sub-space) used inside the attribute.
> type: string
Can you add a schema constraint so that bloblen allowed only when
multi-attr == true and type == binary
^ permalink raw reply
* Re: [PATCH 02/15] tools: ynl: Teach pyynl to handle blobs
From: Donald Hunter @ 2026-07-08 13:48 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-3-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> The generic "device evidence" facility wants to convey large objects
> (certificate chains, measurements) and support various security commands
> relative to that evidence (like validate) [1].
>
> Update pyynl to understand the "bloblen" property indicates:
> * the associated attribute is a variable length byte array
> * it is multi-attr where each message with the same attribute represents
> chunks of the byte array
> * the byte array's size is conveyed in a scalar attribute named by the
> "bloblen" property.
> * the attribute identified by "bloblen" always arrives in a message before
> the first chunk of the associated blob
>
> A "multi-attr" without "bloblen" retains the legacy behavior where repeated
> reception of the same attribute across messages results in a new instance
> of the object. The new behavior causes repeated reception of an
> attribute to be treated as filling consecutive bytes of an array.
>
> Assisted-by: Codex:GPT-5
> [codex: drafted initial python changes]
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Link: http://lore.kernel.org/20260318170014.6650d2bf@kernel.org [1]
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> tools/net/ynl/pyynl/lib/nlspec.py | 12 ++++++
> tools/net/ynl/pyynl/lib/ynl.py | 62 ++++++++++++++++++++++++++++++-
> 2 files changed, 72 insertions(+), 2 deletions(-)
I am mid-review of this but I think it could be simplified. See initial
thoughts below. Can you show me how to configure a test env so I can try
it out?
>
> diff --git a/tools/net/ynl/pyynl/lib/nlspec.py b/tools/net/ynl/pyynl/lib/nlspec.py
> index 0469a0e270d0..4764263968ea 100644
> --- a/tools/net/ynl/pyynl/lib/nlspec.py
> +++ b/tools/net/ynl/pyynl/lib/nlspec.py
> @@ -172,6 +172,7 @@ class SpecAttr(SpecElement):
> selector string, name of attribute used to select
> sub-message type
>
> + bloblen string, name of attr which reports the blob length
> is_auto_scalar bool, attr is a variable-size scalar
> """
> def __init__(self, family, attr_set, yaml, value):
> @@ -181,6 +182,7 @@ class SpecAttr(SpecElement):
> self.value = value
> self.attr_set = attr_set
> self.is_multi = yaml.get('multi-attr', False)
> + self.bloblen = yaml.get('bloblen')
> self.struct_name = yaml.get('struct')
> self.sub_type = yaml.get('sub-type')
> self.byte_order = yaml.get('byte-order')
> @@ -191,6 +193,10 @@ class SpecAttr(SpecElement):
>
> self.is_auto_scalar = self.type in ("sint", "uint")
>
> + if self.bloblen and not self.is_multi:
> + raise SpecException(
> + f"Attribute '{attr_set.name}.{self.name}' has bloblen without multi-attr")
> +
>
> class SpecAttrSet(SpecElement):
> """ Netlink Attribute Set class.
> @@ -355,6 +361,8 @@ class SpecOperation(SpecElement):
> attr_set attribute set name
> fixed_header string, optional name of fixed header struct
>
> + dump_blob_attrs set, reply blob attribute names which may span messages
> +
> yaml raw spec as loaded from the spec file
> """
> def __init__(self, family, yaml, req_value, rsp_value):
> @@ -369,6 +377,7 @@ class SpecOperation(SpecElement):
> self.is_async = 'notify' in yaml or 'event' in yaml
> self.is_resv = not self.is_async and not self.is_call
> self.fixed_header = self.yaml.get('fixed-header', family.fixed_header)
> + self.dump_blob_attrs = frozenset()
Initialising to None would avoid some other logic.
>
> # Added by resolve:
> self.attr_set = None
> @@ -388,6 +397,9 @@ class SpecOperation(SpecElement):
> raise SpecException(f"Can't resolve attribute set for op '{self.name}'")
> if attr_set_name:
> self.attr_set = self.family.attr_sets[attr_set_name]
> + reply_attrs = self.yaml.get('dump', {}).get('reply', {}).get('attributes', ())
> + self.dump_blob_attrs = frozenset(
> + name for name in reply_attrs if self.attr_set[name].bloblen)
>
>
> class SpecMcastGroup(SpecElement):
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index 092d132edec1..f21b68043bd3 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -998,6 +998,47 @@ class YnlFamily(SpecFamily):
> else:
> rsp[name] = [decoded]
>
> + def _blob_fill(self, buf, off, chunks, name):
> + for chunk in chunks:
> + end = off + len(chunk)
> + if end > len(buf):
> + raise YnlException(f"Blob '{name}' data exceeds reported length")
> + buf[off:end] = chunk
> + off = end
> + return off
> +
> + def _blob_init(self, op, rsp, blob_attrs):
> + """
> + Start a new blob object. Preallocate each blob payload using the
> + length carried ahead of the data and copy in the first chunk(s).
> + Return the per-attribute write offsets for any following messages.
> + """
> + offsets = {}
> + for name in blob_attrs:
> + if name not in rsp:
> + continue
> + length = rsp.get(op.attr_set[name].bloblen, 0)
> + buf = bytearray(length)
> + offsets[name] = self._blob_fill(buf, 0, rsp[name], name)
> + rsp[name] = buf
> + return offsets
> +
> + def _blob_extend(self, rsp, update, blob_attrs, offsets):
> + """
> + Continue filling the previous object's blob(s) when the current
> + message carries only blob continuation attributes.
> + """
> + if any(name in rsp and name not in blob_attrs for name in update):
> + return False
> +
> + for name, value in update.items():
> + if name in blob_attrs and name in rsp:
> + offsets[name] = self._blob_fill(rsp[name], offsets.get(name, 0),
> + value, name)
> + else:
> + rsp[name] = value
> + return True
> +
> def _resolve_selector(self, attr_spec, search_attrs):
> sub_msg = attr_spec.sub_message
> if sub_msg not in self.sub_msgs:
> @@ -1356,7 +1397,10 @@ class YnlFamily(SpecFamily):
> for (method, vals, flags) in ops:
> op = self.ops[method]
> msg = self._encode_message(op, vals, flags, req_seq)
> - reqs_by_seq[req_seq] = (op, vals, msg, flags)
> + blob_attrs = None
> + if Netlink.NLM_F_DUMP in flags:
> + blob_attrs = op.dump_blob_attrs or None
> + reqs_by_seq[req_seq] = (op, vals, msg, flags, blob_attrs)
I think this hunk can be dropped and instead use op.dump_blob_attrs directly
> payload += msg
> req_seq += 1
>
> @@ -1365,6 +1409,7 @@ class YnlFamily(SpecFamily):
> done = False
> rsp = []
> op_rsp = []
> + blob_off = {}
> while not done:
> reply, ancdata = self._recvmsg()
> nsid = self._decode_nsid(ancdata)
> @@ -1372,13 +1417,14 @@ class YnlFamily(SpecFamily):
> self._recv_dbg_print(reply, nms)
> for nl_msg in nms:
> if nl_msg.nl_seq in reqs_by_seq:
> - (op, vals, req_msg, req_flags) = reqs_by_seq[nl_msg.nl_seq]
> + (op, vals, req_msg, req_flags, blob_attrs) = reqs_by_seq[nl_msg.nl_seq]
> if nl_msg.extack:
> nl_msg.annotate_extack(op.attr_set)
> self._decode_extack(req_msg, op, nl_msg.extack, vals)
> else:
> op = None
> req_flags = []
> + blob_attrs = None
>
> if nl_msg.error:
> raise NlError(nl_msg)
> @@ -1388,6 +1434,11 @@ class YnlFamily(SpecFamily):
> print(nl_msg)
>
> if Netlink.NLM_F_DUMP in req_flags:
> + if blob_attrs:
> + for obj in op_rsp:
> + for name in blob_attrs:
> + if isinstance(obj.get(name), bytearray):
> + obj[name] = bytes(obj[name])
> rsp.append(op_rsp)
> elif not op_rsp:
> rsp.append(None)
> @@ -1414,6 +1465,13 @@ class YnlFamily(SpecFamily):
> rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
> if op.fixed_header:
> rsp_msg.update(self._decode_struct(decoded.raw, op.fixed_header))
> +
> + if blob_attrs:
> + if op_rsp and self._blob_extend(op_rsp[-1], rsp_msg,
> + blob_attrs, blob_off):
> + continue
> + blob_off = self._blob_init(op, rsp_msg, blob_attrs)
> +
This approach seems unnecessary for pyynl. Just let multi-attr do its
thing and then concatenate them at the end.
> op_rsp.append(rsp_msg)
>
> return rsp
^ permalink raw reply
* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Jason Gunthorpe @ 2026-07-08 14:31 UTC (permalink / raw)
To: Dan Williams (nvidia)
Cc: linux-coco, linux-pci, driver-core, ankita, Aaron Tomlin,
Alexey Kardashevskiy, Alistair Francis, Aneesh Kumar K.V,
Arnd Bergmann, Bjorn Helgaas, Daniel Gomez, Danilo Krummrich,
Dexuan Cui, Donald Hunter, Greg Kroah-Hartman, Jakub Kicinski,
Luis Chamberlain, Lukas Wunner, Petr Pavlu, Rafael J. Wysocki,
Robin Murphy, Sami Tolvanen, Samuel Ortiz, Saravana Kannan,
Will Deacon, Xu Yilun
In-Reply-To: <6a4d95fcc92c2_2f05d5100f7@djbw-dev.notmuch>
On Tue, Jul 07, 2026 at 05:12:44PM -0700, Dan Williams (nvidia) wrote:
> 1/ We previously discussed a use case to operate a device's private MMIO
> while not allowing access to private memory (software encrypted NVME
> with private MMIO [1]). Many of the following comments are based on
> preserving this assumption so you can save some reading if we agree that
> use case can be abandoned.
force_dma_unencrypted() does not *prevent* device access to private
memory and provides no security properties on its own. It's only
purpose is to inform the DMA API what the HW restrictions are for
doing DMA.
The use case I had in mind relied on the vIOMMU to police access to
the private memory. I think if you do T=1 and ADVERSARY you must have
a vIOMMU or it is an illogical configuration.
So, IMHO there is no use case for a T=1 device to only use shared
memory, I would abandon that combination.
> 2/ The confirmation of the trust level and the enabling of DMA are
> separated in time from setting the trust level and entering the RUN
> state.
>
> All the archs separate the RUN step from the ENABLE DMA step, and the
> implementation separates those steps in time.
Yes, until the device is fully accepted the DMA has to be kept off. If
you have to measure in the RUN state then we must have a second step
after measurement completes, however we are measuring in LOCKED right?
> echo tsm0 > $pdev/tsm/lock
> cat $nonce | device-evidence dump $pdev
> device-evidence validate $pdev $generation
So the device is in LOCKED here, DMA remains off both because of
LOCKED and becuase ENABLE DMA has not been done.
> echo 1 > $pdev/tsm/accept
And now it is RUN. So I don't see the issue with enabling DMA at the
same time as gonig to RUN? (though defering it to driver probe would
be a very nice touch as well)
With this API you can't set the devices to RUN and *then* collect the
evidence, is that restriction OK?
> echo full > $pdev/trust
> echo $pdev > $driver/bind
Then we can set the trust and bind the driver, knowing that the T=1
security property is in place.
> The configuration window where $pdev/trust and $pdev/accept can be
> dynamically changing should not be changing the force_dma_unencrypyted()
> result if only because that value will mismatch the hardware state until
> the next ->dma_configure() event.
T=x cannot be changed while a driver is bound, we need to block
that. Changing it will mess up all DMA and the DMA API won't work
anymore.
Thus force_dma_unencrypted() can only change when a driver is
unbound.
If a driver is unbound nothing ever reads force_dma_unencrypted() so
it is safe to change it, thus we can just synchronize T=1 and
force_dma_unencrypted() whenever T changes.
The error case has to wait for the driver to unbind if userspace wants
to go from ERROR->UNLOCKED. IMHO for RAS we are going to need a flow
to logically go from ERROR->RUN, probably with the 'same device
acceptance' I suggested earlier. In this case the DMAs will be halted,
the devices gets back to T=1 and RUN, then the DMAs are turned on
again. The driver remains in T=1 and force_dma_unencrypted() is always
false.
> There are also buses and paravisors that may know that private-DMA is
> enabled for a device by construction. In that case it is also a "trusted
> to access" signal, and not a "required to access" signal.
In this case they wire force_dma_unencrypted()=false.
> My answer, given the concerns of drivers dangerously flipping the
> force_dma_unencrypted() result at runtime was to place it in 'struct
> device_private'
Yes, this seems good idea in general
> Otherwise it is confusing when 2 devices are at FULL trust, but
> one has private memory access and the other does not. One is FULL the
> other is FULL+.
It is confusing when phrased like this, but this is also why I was
suggesting below that a pure trust level may not be sufficient as we
have quite a cross product of scenarios.
If the user wants to operate a device without T=1 and without
ADVERSARY then that's goofy, but technically the kernel doesn't really
need to care? It can do it.
IMHO a big question is how much do we want the kernel to nanny
userspace by having options to double check its work?
Either we trust the user to have checked all the policy conditions
before writing /sys/trust, or we encode some policy conditions and
tell the kernel so it can double-check prior to probe.
At least if we omit the double check it can be fairly easy to add in
later if it really was needed for some time of use reason.
> The question is whether these requirements belong on a central device
> trust mechanism bitmap or should be pushed out to other ABI.
Yes, for sure, I don't have a good sense what is better.
> For
> example, if the presence of the secure IOMMU is enumerated
> after the device is in the LOCKED state then userspace policy can know
> what it is getting into without needing to tell the kernel.
Yes, I think for almost all of this we could push it out to userspace
and have knobs all over. Like IOMMU already has a knob to switch to
DMA mode, we could add an IOMMU specific trust level to set ADVERSARY
also if we go in this direction.
But, I also do really like the idea of a central knob that was fairly
general and simple DISABLE/ADVERSARY/FULL which all sorts of things
can hang off. Including IOMMU and the drivers themselves that may
change operation in ADVERSARY mode.
That means they need simple general definitions for what they are, and
I really like this ADVERSARY naming vs trust as it makes it very clear
what the device disposition intention is.
> Not essential, but it is useful to have the "operate device" intent,
> IOMMU default domain configuration, trust level, and private DMA enable
> all in the same bus operation (->dma_configure). Which is why "accept"
> was demoted to just set RUN state and leave the rest to be finalized
> later.
I agree it is nice, and I think this can work fine, the bus code when
it does the dma configure step can reach out to the TSM and have it
enable DMA.
We also want the IOMMU to change to a default-disabled state, so we
also want its hook in dma configure to switch to non-default too.
It makes alot of sense to me that *all* DMA enable knobs will be
disabled until right before probe starts and then disabled again once
remove completes. TSM, IOMMU, PCI, etc.
Jason
^ permalink raw reply
* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Dave Hansen @ 2026-07-08 14:46 UTC (permalink / raw)
To: Edgecombe, Rick P, linux-kernel@vger.kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com, kas@kernel.org,
djbw@kernel.org, Fang, Peter, linux-coco@lists.linux.dev
In-Reply-To: <7d3eba7a5442cf3e84fe3658ae53337a183290b1.camel@intel.com>
On 7/6/26 14:44, Edgecombe, Rick P wrote:
>> Two alternative schemes were considered:
>>
>> 1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
>> However, this breaks naming consistency unless all existing stable
>> function macros are changed to TDH_XXX_V0.
>>
>> 2. Add an explicit 'version' parameter to the base seamcall() API. This
>> unnecessarily forces all stable SEAMCALL helpers to pass a
>> meaningless '0' argument. Additionally, the magic '0' or '1' values
>> at caller sites are not descriptive.
>>
> Dave was recently saying something to the effect of "make every word count". I
> think we could lose some filler words.
This can also just go below the --- so it doesn't make it to the final
changelog.
I like the discussion happening so far. I, too, dislike adding another
level of C wrappers. I'm looking forward to v2.
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-07-08 15:09 UTC (permalink / raw)
To: Catalin Marinas
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Jiri Pirko, Jason Gunthorpe, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <ak42F240d-53QeFN@arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> On Wed, Jul 01, 2026 at 11:19:20AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 97987f850a33..acf67c7064db 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
>> unsigned int flags = SWIOTLB_VERBOSE;
>> bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
>>
>> - if (is_realm_world()) {
>> + if (is_realm_world())
>> swiotlb = true;
>> - flags |= SWIOTLB_FORCE;
>> - }
>
> For this part:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
>> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
>> index e05dc7649366..f3fc28f352ba 100644
>> --- a/kernel/dma/direct.h
>> +++ b/kernel/dma/direct.h
>> @@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>> {
>> dma_addr_t dma_addr;
>>
>> + /*
>> + * For a device requiring unencrypted DMA, MMIO memory is treated
>> + * as shared by default.
>> + */
>> + if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
>> + attrs |= DMA_ATTR_CC_SHARED;
>> +
>> if (is_swiotlb_force_bounce(dev)) {
>> - if (!(attrs & DMA_ATTR_CC_SHARED)) {
>> - if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
>> - return DMA_MAPPING_ERROR;
>> + if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
>> + return DMA_MAPPING_ERROR;
>>
>> - return swiotlb_map(dev, phys, size, dir, attrs);
>> - }
>> - } else if (attrs & DMA_ATTR_CC_SHARED) {
>> - return DMA_MAPPING_ERROR;
>> + return swiotlb_map(dev, phys, size, dir, attrs);
>> }
>>
>> - if (attrs & DMA_ATTR_MMIO) {
>> - dma_addr = phys;
>> - if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
>> - goto err_overflow;
>> - } else if (attrs & DMA_ATTR_CC_SHARED) {
>> + if (attrs & DMA_ATTR_CC_SHARED)
>> dma_addr = phys_to_dma_unencrypted(dev, phys);
>> + else
>> + dma_addr = phys_to_dma_encrypted(dev, phys);
>
> For AMD/SME, on host with memory encryption we now end up setting the C
> bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
> some other MMIO bus understands this attribute. Maybe we should stick to
> something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
> since this is not universally defined, just use the old dma_addr = phys
> if MMIO and ignore any unlikely DMA offsets.
>
> In the other case, for an arm CCA guest, if the MMIO is shared we end up
> setting the shared attribute but that's fine, it's only an IPA address.
>
P2PDMA with CoCo is currently not supported, as discussed here:
https://lore.kernel.org/all/20260521175420.GA7702@ziepe.ca
https://lore.kernel.org/all/20260522132240.GD7702@ziepe.ca
Since I do not have much experience with P2PDMA, I have prepared a patch
that Leon Romanovsky <leonro@nvidia.com> has kindly agreed to review to
ensure it is correct. I have attached it below.
The conclusion is that P2P is not currently supported, so we get this
series merged first and handle P2PDMA support as a follow-up outside
this series.
commit f39bcb603c5cc4cfa738597715828ac45f01f142
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Date: Fri May 22 10:10:35 2026 +0530
PCI/P2PDMA: Carry host bridge DMA attrs through map and unmap
P2PDMA host bridge mappings currently pass DMA_ATTR_MMIO and rely on
dma-direct to add DMA_ATTR_CC_SHARED for devices that require unencrypted
DMA. That puts P2PDMA-specific confidential-computing policy in the generic
dma-direct MMIO mapping path and does not preserve the selected attributes
through all unmap paths.
Add DMA mapping flags to struct p2pdma_provider for host bridge mappings
and use those flags in the block, dma-buf and HMM mapping paths. Store the
attributes selected at map time where the unmap side no longer has provider
context, so that the unmap path uses the same attributes that were used for
mapping.
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index bfdb9ed70741..fafc6d6b8522 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -87,10 +87,11 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
struct blk_dma_iter *iter, struct phys_vec *vec)
{
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
+ attrs |= iter->p2pdma.mem->dma_mapping_flags;
+ iter->attrs = attrs;
iter->addr = dma_map_phys(dma_dev, vec->paddr, vec->len,
rq_dma_dir(req), attrs);
@@ -107,7 +108,7 @@ static bool blk_rq_dma_map_iova(struct request *req, struct device *dma_dev,
struct phys_vec *vec)
{
enum dma_data_direction dir = rq_dma_dir(req);
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
size_t mapped = 0;
int error;
@@ -115,7 +116,8 @@ static bool blk_rq_dma_map_iova(struct request *req, struct device *dma_dev,
iter->len = dma_iova_size(state);
if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
+ attrs |= iter->p2pdma.mem->dma_mapping_flags;
+ iter->attrs = attrs;
do {
error = dma_iova_link(dma_dev, state, vec->paddr, mapped,
@@ -168,6 +170,7 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
struct phys_vec vec;
memset(&iter->p2pdma, 0, sizeof(iter->p2pdma));
+ iter->attrs = 0;
iter->status = BLK_STS_OK;
iter->p2pdma.map = PCI_P2PDMA_MAP_NONE;
diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
index 794acff2546a..e6d4ee8acd19 100644
--- a/drivers/dma-buf/dma-buf-mapping.c
+++ b/drivers/dma-buf/dma-buf-mapping.c
@@ -59,11 +59,13 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
* @sgt: Scatter-gather table
* @state: DMA IOVA state relevant in IOMMU-based DMA
* @size: Total size of DMA transfer
+ * @attrs: DMA attributes used for host bridge mappings
*/
struct dma_buf_dma {
struct sg_table sgt;
struct dma_iova_state *state;
size_t size;
+ unsigned long attrs;
};
/**
@@ -119,6 +121,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
*/
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
+ dma->attrs = provider->dma_mapping_flags;
dma->state = kzalloc_obj(*dma->state);
if (!dma->state) {
ret = -ENOMEM;
@@ -147,7 +150,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
ret = dma_iova_link(attach->dev, dma->state,
phys_vec[i].paddr, 0,
phys_vec[i].len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
if (ret)
goto err_unmap_dma;
@@ -155,7 +158,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
} else {
addr = dma_map_phys(attach->dev, phys_vec[i].paddr,
phys_vec[i].len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
ret = dma_mapping_error(attach->dev, addr);
if (ret)
goto err_unmap_dma;
@@ -195,11 +198,11 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
; /* Do nothing */
} else if (dma_use_iova(dma->state)) {
dma_iova_destroy(attach->dev, dma->state, mapped_len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
} else {
for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
dma_unmap_phys(attach->dev, sg_dma_address(sgl),
- sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
+ sg_dma_len(sgl), dir, dma->attrs);
}
sg_free_table(&dma->sgt);
err_free_state:
@@ -231,13 +234,13 @@ void dma_buf_free_sgt(struct dma_buf_attachment *attach, struct sg_table *sgt,
; /* Do nothing */
} else if (dma_use_iova(dma->state)) {
dma_iova_destroy(attach->dev, dma->state, dma->size, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
} else {
struct scatterlist *sgl;
for_each_sgtable_dma_sg(sgt, sgl, i)
dma_unmap_phys(attach->dev, sg_dma_address(sgl),
- sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
+ sg_dma_len(sgl), dir, dma->attrs);
}
sg_free_table(sgt);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 69932d640b53..8f05f53b78fd 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -436,6 +436,7 @@ struct nvme_iod {
size_t total_len;
struct dma_iova_state dma_state;
+ unsigned long dma_attrs;
void *descriptors[NVME_MAX_NR_DESCRIPTORS];
struct nvme_dma_vec *dma_vecs;
unsigned int nr_dma_vecs;
@@ -443,6 +444,7 @@ struct nvme_iod {
dma_addr_t meta_dma;
size_t meta_total_len;
struct dma_iova_state meta_dma_state;
+ unsigned long meta_dma_attrs;
struct nvme_sgl_desc *meta_descriptor;
};
@@ -859,7 +861,7 @@ static void nvme_free_descriptors(struct request *req)
}
}
-static void nvme_free_prps(struct request *req, unsigned int attrs)
+static void nvme_free_prps(struct request *req, unsigned long attrs)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
@@ -872,7 +874,7 @@ static void nvme_free_prps(struct request *req, unsigned int attrs)
}
static void nvme_free_sgls(struct request *req, struct nvme_sgl_desc *sge,
- struct nvme_sgl_desc *sg_list, unsigned int attrs)
+ struct nvme_sgl_desc *sg_list, unsigned long attrs)
{
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
enum dma_data_direction dir = rq_dma_dir(req);
@@ -899,7 +901,7 @@ static void nvme_unmap_metadata(struct request *req)
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct device *dma_dev = nvmeq->dev->dev;
struct nvme_sgl_desc *sge = iod->meta_descriptor;
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iod->flags & IOD_SINGLE_META_SEGMENT) {
dma_unmap_page(dma_dev, iod->meta_dma,
@@ -912,11 +914,11 @@ static void nvme_unmap_metadata(struct request *req)
map = PCI_P2PDMA_MAP_BUS_ADDR;
else if (iod->flags & IOD_META_MMIO) {
map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
- attrs |= DMA_ATTR_MMIO;
+ attrs = iod->meta_dma_attrs;
}
if (!blk_rq_dma_unmap(req, dma_dev, &iod->meta_dma_state,
- iod->meta_total_len, map)) {
+ iod->meta_total_len, map, attrs)) {
if (nvme_pci_cmd_use_meta_sgl(&iod->cmd))
nvme_free_sgls(req, sge, &sge[1], attrs);
else
@@ -935,7 +937,7 @@ static void nvme_unmap_data(struct request *req)
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
struct device *dma_dev = nvmeq->dev->dev;
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iod->flags & IOD_SINGLE_SEGMENT) {
static_assert(offsetof(union nvme_data_ptr, prp1) ==
@@ -949,11 +951,11 @@ static void nvme_unmap_data(struct request *req)
map = PCI_P2PDMA_MAP_BUS_ADDR;
else if (iod->flags & IOD_DATA_MMIO) {
map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
- attrs |= DMA_ATTR_MMIO;
+ attrs = iod->dma_attrs;
}
if (!blk_rq_dma_unmap(req, dma_dev, &iod->dma_state, iod->total_len,
- map)) {
+ map, attrs)) {
if (nvme_pci_cmd_use_sgl(&iod->cmd))
nvme_free_sgls(req, &iod->cmd.common.dptr.sgl,
iod->descriptors[0], attrs);
@@ -1270,6 +1272,7 @@ static blk_status_t nvme_map_data(struct request *req)
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
iod->flags |= IOD_DATA_MMIO;
+ iod->dma_attrs = iter.attrs;
break;
case PCI_P2PDMA_MAP_NONE:
break;
@@ -1305,6 +1308,7 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req)
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
iod->flags |= IOD_META_MMIO;
+ iod->meta_dma_attrs = iter.attrs;
break;
case PCI_P2PDMA_MAP_NONE:
break;
@@ -1405,6 +1409,8 @@ static blk_status_t nvme_prep_rq(struct request *req)
iod->nr_descriptors = 0;
iod->total_len = 0;
iod->meta_total_len = 0;
+ iod->dma_attrs = 0;
+ iod->meta_dma_attrs = 0;
iod->nr_dma_vecs = 0;
ret = nvme_setup_cmd(req->q->queuedata, req);
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index b2d5266f8653..35ab3f8b53a8 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -285,6 +285,11 @@ int pcim_p2pdma_init(struct pci_dev *pdev)
continue;
p2p->mem[i].owner = &pdev->dev;
+
+ p2p->mem[i].dma_mapping_flags = DMA_ATTR_MMIO;
+ if (force_dma_unencrypted(dev))
+ p2p->mem[i].dma_mapping_flags |= DMA_ATTR_CC_SHARED;
+
p2p->mem[i].bus_offset =
pci_bus_address(pdev, i) - pci_resource_start(pdev, i);
}
diff --git a/include/linux/blk-mq-dma.h b/include/linux/blk-mq-dma.h
index 214c181ff2c9..fab72c0f8053 100644
--- a/include/linux/blk-mq-dma.h
+++ b/include/linux/blk-mq-dma.h
@@ -16,6 +16,7 @@ struct blk_dma_iter {
/* Output address range for this iteration */
dma_addr_t addr;
u32 len;
+ unsigned long attrs;
struct pci_p2pdma_map_state p2pdma;
/* Status code. Only valid when blk_rq_dma_map_iter_* returned false */
@@ -49,23 +50,19 @@ static inline bool blk_rq_dma_map_coalesce(struct dma_iova_state *state)
* @state: DMA IOVA state
* @mapped_len: number of bytes to unmap
* @map: peer-to-peer mapping type
+ * @attrs: DMA attributes used for the mapping
*
* Returns %false if the callers need to manually unmap every DMA segment
* mapped using @iter or %true if no work is left to be done.
*/
static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
struct dma_iova_state *state, size_t mapped_len,
- enum pci_p2pdma_map_type map)
+ enum pci_p2pdma_map_type map, unsigned long attrs)
{
if (map == PCI_P2PDMA_MAP_BUS_ADDR)
return true;
if (dma_use_iova(state)) {
- unsigned int attrs = 0;
-
- if (map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
-
dma_iova_destroy(dma_dev, state, mapped_len, rq_dma_dir(req),
attrs);
return true;
diff --git a/include/linux/hmm-dma.h b/include/linux/hmm-dma.h
index f58b9fc71999..d9f967ed7ce1 100644
--- a/include/linux/hmm-dma.h
+++ b/include/linux/hmm-dma.h
@@ -14,12 +14,14 @@ struct pci_p2pdma_map_state;
* @state: DMA IOVA state
* @pfns: array of PFNs
* @dma_list: array of DMA addresses
+ * @dma_attrs: array of DMA attributes
* @dma_entry_size: size of each DMA entry in the array
*/
struct hmm_dma_map {
struct dma_iova_state state;
unsigned long *pfn_list;
dma_addr_t *dma_list;
+ unsigned long *dma_attrs;
size_t dma_entry_size;
};
diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
index 873de20a2247..402dc5e5d62b 100644
--- a/include/linux/pci-p2pdma.h
+++ b/include/linux/pci-p2pdma.h
@@ -21,10 +21,12 @@ struct scatterlist;
*
* A p2pdma provider is a range of MMIO address space available to the CPU.
* @owner: Device to which this provider belongs.
+ * @dma_mapping_flags: DMA attributes to use for host bridge mappings.
* @bus_offset: Bus offset for p2p communication.
*/
struct p2pdma_provider {
struct device *owner;
+ unsigned long dma_mapping_flags;
u64 bus_offset;
};
diff --git a/mm/hmm.c b/mm/hmm.c
index c72c9ddfdb2f..a871d8cb7dd9 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -720,6 +720,11 @@ int hmm_dma_map_alloc(struct device *dev, struct hmm_dma_map *map,
if (!map->pfn_list)
return -ENOMEM;
+ map->dma_attrs = kvcalloc(nr_entries, sizeof(*map->dma_attrs),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!map->dma_attrs)
+ goto err_attrs;
+
use_iova = dma_iova_try_alloc(dev, &map->state, 0,
nr_entries * PAGE_SIZE);
if (!use_iova && dma_need_unmap(dev)) {
@@ -731,6 +736,8 @@ int hmm_dma_map_alloc(struct device *dev, struct hmm_dma_map *map,
return 0;
err_dma:
+ kvfree(map->dma_attrs);
+err_attrs:
kvfree(map->pfn_list);
return -ENOMEM;
}
@@ -749,6 +756,7 @@ void hmm_dma_map_free(struct device *dev, struct hmm_dma_map *map)
dma_iova_free(dev, &map->state);
kvfree(map->pfn_list);
kvfree(map->dma_list);
+ kvfree(map->dma_attrs);
}
EXPORT_SYMBOL_GPL(hmm_dma_map_free);
@@ -811,11 +819,12 @@ dma_addr_t hmm_dma_map_pfn(struct device *dev, struct hmm_dma_map *map,
case PCI_P2PDMA_MAP_NONE:
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
- attrs |= DMA_ATTR_MMIO;
+ attrs |= p2pdma_state->mem->dma_mapping_flags;
pfns[idx] |= HMM_PFN_P2PDMA;
break;
case PCI_P2PDMA_MAP_BUS_ADDR:
pfns[idx] |= HMM_PFN_P2PDMA_BUS | HMM_PFN_DMA_MAPPED;
+ map->dma_attrs[idx] = 0;
return pci_p2pdma_bus_addr_map(p2pdma_state->mem, paddr);
default:
return DMA_MAPPING_ERROR;
@@ -848,10 +857,12 @@ dma_addr_t hmm_dma_map_pfn(struct device *dev, struct hmm_dma_map *map,
if (dma_need_unmap(dev))
dma_addrs[idx] = dma_addr;
}
+ map->dma_attrs[idx] = attrs;
pfns[idx] |= HMM_PFN_DMA_MAPPED;
return dma_addr;
error:
pfns[idx] &= ~HMM_PFN_P2PDMA;
+ map->dma_attrs[idx] = 0;
return DMA_MAPPING_ERROR;
}
@@ -871,14 +882,11 @@ bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
struct dma_iova_state *state = &map->state;
dma_addr_t *dma_addrs = map->dma_list;
unsigned long *pfns = map->pfn_list;
- unsigned long attrs = DMA_ATTR_REQUIRE_COHERENT;
+ unsigned long attrs = DMA_ATTR_REQUIRE_COHERENT | map->dma_attrs[idx];
if ((pfns[idx] & valid_dma) != valid_dma)
return false;
- if (pfns[idx] & HMM_PFN_P2PDMA)
- attrs |= DMA_ATTR_MMIO;
-
if (pfns[idx] & HMM_PFN_P2PDMA_BUS)
; /* no need to unmap bus address P2P mappings */
else if (dma_use_iova(state))
@@ -890,6 +898,7 @@ bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
pfns[idx] &=
~(HMM_PFN_DMA_MAPPED | HMM_PFN_P2PDMA | HMM_PFN_P2PDMA_BUS);
+ map->dma_attrs[idx] = 0;
return true;
}
EXPORT_SYMBOL_GPL(hmm_dma_unmap_pfn);
^ permalink raw reply related
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