* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 17:58 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <maleev6ofzlnhi3rmqjawlllxkda4mrgwmb6msz5gz77klfrxl@adpqtakqrxrj>
On 12/1/25 11:48 AM, Kiryl Shutsemau wrote:
> On Mon, Dec 01, 2025 at 11:15:13AM -0600, Pratik R. Sampat wrote:
>>
>>
>> On 11/27/25 11:40 AM, Kiryl Shutsemau wrote:
>>> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>>>
>>>>
>>>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>>> accessing pages that have not transitioned to private before use.
>>>>>
>>>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>>>> so we can size bitmap accordingly upfront.
>>>>>
>>>>
>>>> I'm not quite sure if I fully understand. Do you mean to refer to the
>>>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>>>> memory? If so, wouldn't it still be desirable to increase the size of
>>>> the bitmap to what was marked as hotpluggable initially?
>>>
>>> I just don't understand how hotpluggable memory presented in EFI memory
>>> map in presence of unaccepted memory. If not-yet-plugged memory marked
>>> as unaccepted we can preallocate bitmap upfront and make unaccepted
>>> memory transparent wrt hotplug.
>>
>> If memory that hasn't been plugged yet never gets plugged in or is only
>> partially plugged in, wouldn't we be wasting space by preallocating
>> the bitmap upfront? Or would that not be a concern in favor of
>> transparency?
>
> 4k per 64GiB of physical address space should be low enough to ignore, no?
>
> We can look into optimizing it out when it is an actual, not imaginary
> problem.
>
Sure, that's fair!
--Pratik
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Kiryl Shutsemau @ 2025-12-01 17:48 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <f60d968e-6be3-4bad-8bf4-8500ad039817@amd.com>
On Mon, Dec 01, 2025 at 11:15:13AM -0600, Pratik R. Sampat wrote:
>
>
> On 11/27/25 11:40 AM, Kiryl Shutsemau wrote:
> > On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
> >>
> >>
> >> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
> >>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
> >>>> The unaccepted memory structure currently only supports accepting memory
> >>>> present at boot time. The unaccepted table uses a fixed-size bitmap
> >>>> reserved in memblock based on the initial memory layout, preventing
> >>>> dynamic addition of memory ranges after boot. This causes guest
> >>>> termination when memory is hot-added in a secure virtual machine due to
> >>>> accessing pages that have not transitioned to private before use.
> >>>
> >>> How does the hot-pluggable memory look in EFI memory map? I thought
> >>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
> >>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
> >>> so we can size bitmap accordingly upfront.
> >>>
> >>
> >> I'm not quite sure if I fully understand. Do you mean to refer to the
> >> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
> >> memory? If so, wouldn't it still be desirable to increase the size of
> >> the bitmap to what was marked as hotpluggable initially?
> >
> > I just don't understand how hotpluggable memory presented in EFI memory
> > map in presence of unaccepted memory. If not-yet-plugged memory marked
> > as unaccepted we can preallocate bitmap upfront and make unaccepted
> > memory transparent wrt hotplug.
>
> If memory that hasn't been plugged yet never gets plugged in or is only
> partially plugged in, wouldn't we be wasting space by preallocating
> the bitmap upfront? Or would that not be a concern in favor of
> transparency?
4k per 64GiB of physical address space should be low enough to ignore, no?
We can look into optimizing it out when it is an actual, not imaginary
problem.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 17:21 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), linux-mm, linux-coco, linux-efi, x86,
linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <ac479414-e0fa-49be-8a30-8f9c0e7b7d32@kernel.org>
On 11/28/25 3:32 AM, David Hildenbrand (Red Hat) wrote:
> On 11/25/25 18:57, Pratik R. Sampat wrote:
>> The unaccepted memory structure currently only supports accepting memory
>> present at boot time. The unaccepted table uses a fixed-size bitmap
>> reserved in memblock based on the initial memory layout, preventing
>> dynamic addition of memory ranges after boot. This causes guest
>> termination when memory is hot-added in a secure virtual machine due to
>> accessing pages that have not transitioned to private before use.
>>
>> Extend the unaccepted memory framework to handle hotplugged memory by
>> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
>> hotplugged ranges exceed the reserved bitmap capacity and switch to
>> kernel-managed allocation.
>>
>> Hotplugged memory also follows the same acceptance policy using the
>> accept_memory=[eager|lazy] kernel parameter to accept memory either
>> up-front when added or before first use.
>>
>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>> ---
>> arch/x86/boot/compressed/efi.h | 1 +
>> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
>> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
>> include/linux/efi.h | 1 +
>> include/linux/mm.h | 11 +++
>> mm/memory_hotplug.c | 7 ++
>> mm/page_alloc.c | 2 +
>> 7 files changed, 106 insertions(+)
>>
>> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
>> index 4f7027f33def..a220a1966cae 100644
>> --- a/arch/x86/boot/compressed/efi.h
>> +++ b/arch/x86/boot/compressed/efi.h
>> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
>> u32 unit_size;
>> u64 phys_base;
>> u64 size;
>> + bool mem_reserved;
>> unsigned long *bitmap;
>> };
>> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c b/drivers/firmware/efi/libstub/unaccepted_memory.c
>> index c1370fc14555..b16bd61c12bf 100644
>> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
>> @@ -83,6 +83,7 @@ efi_status_t allocate_unaccepted_bitmap(__u32 nr_desc,
>> unaccepted_table->unit_size = EFI_UNACCEPTED_UNIT_SIZE;
>> unaccepted_table->phys_base = unaccepted_start;
>> unaccepted_table->size = bitmap_size;
>> + unaccepted_table->mem_reserved = true;
>> memset(unaccepted_table->bitmap, 0, bitmap_size);
>> status = efi_bs_call(install_configuration_table,
>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>> index 4479aad258f8..8537812346e2 100644
>> --- a/drivers/firmware/efi/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>> @@ -218,6 +218,89 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>> return ret;
>> }
>> +static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
>> + unsigned long mem_range_size)
>> +{
>> + struct efi_unaccepted_memory *unacc_tbl;
>> + unsigned long *old_bitmap, *new_bitmap;
>> + phys_addr_t start, end, mem_range_end;
>> + u64 phys_base, size, unit_size;
>> + unsigned long flags;
>> +
>> + unacc_tbl = efi_get_unaccepted_table();
>> + if (!unacc_tbl || !unacc_tbl->unit_size)
>> + return -EIO;
>> +
>> + unit_size = unacc_tbl->unit_size;
>> + phys_base = unacc_tbl->phys_base;
>> +
>> + mem_range_end = round_up(mem_range_start + mem_range_size, unit_size);
>> + size = DIV_ROUND_UP(mem_range_end - phys_base, unit_size * BITS_PER_BYTE);
>> +
>> + /* Translate to offsets from the beginning of the bitmap */
>> + start = mem_range_start - phys_base;
>> + end = mem_range_end - phys_base;
>> +
>> + old_bitmap = efi_get_unaccepted_bitmap();
>> + if (!old_bitmap)
>> + return -EIO;
>> +
>> + /* If the bitmap is already large enough, just set the bits */
>> + if (unacc_tbl->size >= size) {
>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> + bitmap_set(old_bitmap, start / unit_size, (end - start) / unit_size);
>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> +
>> + return 0;
>> + }
>> +
>> + /* Reserved memblocks cannot be extended so allocate a new bitmap */
>> + if (unacc_tbl->mem_reserved) {
>> + new_bitmap = kzalloc(size, GFP_KERNEL);
>> + if (!new_bitmap)
>> + return -ENOMEM;
>> +
>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> + memcpy(new_bitmap, old_bitmap, unacc_tbl->size);
>> + unacc_tbl->mem_reserved = false;
>> + free_reserved_area(old_bitmap, old_bitmap + unacc_tbl->size, -1, NULL);
>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> + } else {
>> + new_bitmap = krealloc(old_bitmap, size, GFP_KERNEL);
>> + if (!new_bitmap)
>> + return -ENOMEM;
>> +
>> + /* Zero the bitmap from the range it was extended from */
>> + memset(new_bitmap + unacc_tbl->size, 0, size - unacc_tbl->size);
>> + }
>> +
>> + bitmap_set(new_bitmap, start / unit_size, (end - start) / unit_size);
>> +
>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> + unacc_tbl->size = size;
>> + unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
>> +{
>> + int ret;
>> +
>> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>> + return 0;
>> +
>> + ret = extend_unaccepted_bitmap(mem_range_start, mem_range_size);
>> + if (ret)
>> + return ret;
>> +
>> + if (!mm_lazy_accept_enabled())
>> + accept_memory(mem_range_start, mem_range_size);
>> +
>> + return 0;
>> +}
>> +
>> #ifdef CONFIG_PROC_VMCORE
>> static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
>> unsigned long pfn)
>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>> index a74b393c54d8..1021eb78388f 100644
>> --- a/include/linux/efi.h
>> +++ b/include/linux/efi.h
>> @@ -545,6 +545,7 @@ struct efi_unaccepted_memory {
>> u32 unit_size;
>> u64 phys_base;
>> u64 size;
>> + bool mem_reserved;
>> unsigned long *bitmap;
>> };
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 1ae97a0b8ec7..bb43876e6c47 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -4077,6 +4077,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
>> bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
>> void accept_memory(phys_addr_t start, unsigned long size);
>> +int accept_hotplug_memory(phys_addr_t mem_range_start,
>> + unsigned long mem_range_size);
>> +bool mm_lazy_accept_enabled(void);
>> #else
>> @@ -4090,6 +4093,14 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
>> {
>> }
>> +static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
>> + unsigned long mem_range_size)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline bool mm_lazy_accept_enabled(void) { return false; }
>> +
>> #endif
>> static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 74318c787715..bf8086682b66 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1581,6 +1581,13 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>> if (!strcmp(res->name, "System RAM"))
>> firmware_map_add_hotplug(start, start + size, "System RAM");
>> + ret = accept_hotplug_memory(start, size);
>
> What makes this special that we have to have "hotplug_memory" as part of the name?
>
> Staring at the helper itself, there isn't anything really hotplug specific happening in there except extending the bitmap, maybe?
>
Right, we are extending the original bitmap and initializing a structure
to track state as well. I added the hotplug_memory keyword without
much thought, since I didn't see anyone else attempting to extend these
structures.
That said, I agree the name is awkward. I could either come up with
something different, or we could eliminate the parent function
entirely and call extend_unaccepted_bitmap() + accept_memory() directly
from add_memory_resource(). Similarly, we could do the same to
s/unaccept_hotplug_memory/unaccept_memory too.
Thanks,
--Pratik
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 17:15 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <27c06cf2-7500-4875-bd22-f55571fb85f9@kernel.org>
Hi David,
On 11/28/25 3:34 AM, David Hildenbrand (Red Hat) wrote:
> On 11/27/25 18:40, Kiryl Shutsemau wrote:
>> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>>
>>>
>>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>> accessing pages that have not transitioned to private before use.
>>>>
>>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>>> so we can size bitmap accordingly upfront.
>>>>
>>>
>>> I'm not quite sure if I fully understand. Do you mean to refer to the
>>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>>> memory? If so, wouldn't it still be desirable to increase the size of
>>> the bitmap to what was marked as hotpluggable initially?
>>
>> I just don't understand how hotpluggable memory presented in EFI memory
>> map in presence of unaccepted memory. If not-yet-plugged memory marked
>> as unaccepted we can preallocate bitmap upfront and make unaccepted
>> memory transparent wrt hotplug.
>>
>> BTW, isn't virtio-mem a more attractive target to support than HW-style
>> hotplug?
>
> I would have thought so as well, such that we can just let virtio-mem take care of any acceptance before actually using hotplugged memory (exposing it to the buddy).
>
> Likely there is desire to support other hypervisors?
That's true. We are certainly thinking about how the RAM discard manager
should look like with multiple states to allow guest_memfd and
virtio-mem to work together.
Since both paths in Linux eventually converge around
add_memory_resource(), based on some light hacking in QEMU I could see
similar hotplug behavior for virtio-mem as well. So I thought I'd get
some feedback on the Linux side of the design since enabling it
for traditional memory seemed like a simpler first step in enabling
hotplug.
Thanks,
--Pratik
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-01 17:15 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <dy65eupwalp5wsljetlto27l6tjjvoygeotjd3n7mk7zjc4dma@jf4hzsy6rtcd>
On 11/27/25 11:40 AM, Kiryl Shutsemau wrote:
> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>
>>
>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>> The unaccepted memory structure currently only supports accepting memory
>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>> reserved in memblock based on the initial memory layout, preventing
>>>> dynamic addition of memory ranges after boot. This causes guest
>>>> termination when memory is hot-added in a secure virtual machine due to
>>>> accessing pages that have not transitioned to private before use.
>>>
>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>> so we can size bitmap accordingly upfront.
>>>
>>
>> I'm not quite sure if I fully understand. Do you mean to refer to the
>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>> memory? If so, wouldn't it still be desirable to increase the size of
>> the bitmap to what was marked as hotpluggable initially?
>
> I just don't understand how hotpluggable memory presented in EFI memory
> map in presence of unaccepted memory. If not-yet-plugged memory marked
> as unaccepted we can preallocate bitmap upfront and make unaccepted
> memory transparent wrt hotplug.
If memory that hasn't been plugged yet never gets plugged in or is only
partially plugged in, wouldn't we be wasting space by preallocating
the bitmap upfront? Or would that not be a concern in favor of
transparency?
--Pratik
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Borislav Petkov @ 2025-12-01 11:12 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <47927c25-a317-488a-823f-ac0588f4eee4@kernel.org>
On Mon, Dec 01, 2025 at 10:18:37AM +0100, David Hildenbrand (Red Hat) wrote:
> Makes sense to me, especially for confidential VMs where we pass such
> kernel-managed data from the old to the new kernel.
It shouldn't matter, right?
I think the question is whether the kernel should agree to the software
contract (/eyeroll) to keep the kernel ABI compatible wrt kexec.
And I don't think we have agreed to that AFAIK.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-12-01 9:18 UTC (permalink / raw)
To: Borislav Petkov
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <20251128113411.GAaSmIs0kSWGhCYkaA@fat_crate.local>
On 11/28/25 12:34, Borislav Petkov wrote:
> On Fri, Nov 28, 2025 at 10:30:15AM +0100, David Hildenbrand (Red Hat) wrote:
>> kexecing the same kernel is typically used for kdump purposes.
>>
>> kexecing different kernels is used for all sorts of things (live-upgrade,
>> grub-emu come to mind). It's quite common to kexec different kernels, or
>> maybe I misunderstood the question here?
>
> And my question is: since when do we enforce no-ABI-changes between kernels so
> that we can kexec any kernel into any kernel?
>
> By that logic I should be able to kexec 5.x into 6.x. I'll bet some money that
> it won't work.
I *think* ordinary kexec would likely work, as I recall that it doesn't
need a lot of that special kexec ABI sauce like unaccepted memory uses.
Within confidential VMs (kexec ...) I am pretty sure that it's a
different discussion.
>
> So unless it is written down somewhere, I think we should probably talk first
> what we want to support and why...
>
> Makes sense?
Makes sense to me, especially for confidential VMs where we pass such
kernel-managed data from the old to the new kernel.
--
Cheers
David
^ permalink raw reply
* Re: [PATCH kernel] crypto/ccp: Fix no-TSM build in SEV-TIO PCIe IDE
From: Aithal, Srikanth @ 2025-12-01 8:09 UTC (permalink / raw)
To: Alexey Kardashevskiy, linux-crypto
Cc: linux-kernel, Ashish Kalra, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Dan Williams, Borislav Petkov (AMD), linux-coco,
Srikanth Aithal
In-Reply-To: <20251201075257.484492-1-aik@amd.com>
On 12/1/2025 1:22 PM, Alexey Kardashevskiy wrote:
> Here are some cleanups for disable TSM+IDE.
>
> Fixes: 3532f6154971 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
> Reported-by: Srikanth Aithal <Srikanth.Aithal@amd.com>
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>
> Better be just squashed into 3532f6154971 while it is in the next.
> ---
> drivers/crypto/ccp/sev-dev-tio.h | 12 ------------
> drivers/crypto/ccp/sev-dev.h | 8 ++++++++
> drivers/crypto/ccp/sev-dev.c | 12 ++++--------
> 3 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sev-dev-tio.h b/drivers/crypto/ccp/sev-dev-tio.h
> index 7c42351210ef..71f232a2b08b 100644
> --- a/drivers/crypto/ccp/sev-dev-tio.h
> +++ b/drivers/crypto/ccp/sev-dev-tio.h
> @@ -7,8 +7,6 @@
> #include <linux/pci-ide.h>
> #include <uapi/linux/psp-sev.h>
>
> -#if defined(CONFIG_CRYPTO_DEV_SP_PSP)
> -
> struct sla_addr_t {
> union {
> u64 sla;
> @@ -129,14 +127,4 @@ int sev_tio_dev_connect(struct tsm_dsm_tio *dev_data, u8 tc_mask, u8 ids[8], u8
> int sev_tio_dev_disconnect(struct tsm_dsm_tio *dev_data, bool force);
> int sev_tio_dev_reclaim(struct tsm_dsm_tio *dev_data);
>
> -#endif /* CONFIG_CRYPTO_DEV_SP_PSP */
> -
> -#if defined(CONFIG_PCI_TSM)
> -void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page);
> -void sev_tsm_uninit(struct sev_device *sev);
> -int sev_tio_cmd_buffer_len(int cmd);
> -#else
> -static inline int sev_tio_cmd_buffer_len(int cmd) { return 0; }
> -#endif
> -
> #endif /* __PSP_SEV_TIO_H__ */
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index dced4a8e9f01..d3e506206dbd 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -81,4 +81,12 @@ void sev_pci_exit(void);
> struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages);
> void snp_free_hv_fixed_pages(struct page *page);
>
> +#if defined(CONFIG_PCI_TSM)
> +void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page);
> +void sev_tsm_uninit(struct sev_device *sev);
> +int sev_tio_cmd_buffer_len(int cmd);
> +#else
> +static inline int sev_tio_cmd_buffer_len(int cmd) { return 0; }
> +#endif
> +
> #endif /* __SEV_DEV_H */
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 365867f381e9..67ea9b30159a 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -38,7 +38,6 @@
>
> #include "psp-dev.h"
> #include "sev-dev.h"
> -#include "sev-dev-tio.h"
>
> #define DEVICE_NAME "sev"
> #define SEV_FW_FILE "amd/sev.fw"
> @@ -1365,11 +1364,6 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
> return 0;
> }
>
> -static bool sev_tio_present(struct sev_device *sev)
> -{
> - return (sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED) != 0;
> -}
> -
> static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
> {
> struct psp_device *psp = psp_master;
> @@ -1448,10 +1442,12 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
> data.list_paddr = __psp_pa(snp_range_list);
>
> #if defined(CONFIG_PCI_TSM)
> - data.tio_en = sev_tio_present(sev) &&
> + bool tio_supp = !!(sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED);
> +
> + data.tio_en = tio_supp &&
> sev_tio_enabled && psp_init_on_probe &&
> amd_iommu_sev_tio_supported();
> - if (sev_tio_present(sev) && !psp_init_on_probe)
> + if (tio_supp && !psp_init_on_probe)
> dev_warn(sev->dev, "SEV-TIO as incompatible with psp_init_on_probe=0\n");
> #endif
> cmd = SEV_CMD_SNP_INIT_EX;
This fixes the issue. Thank you.
Tested-by: Srikanth Aithal <Srikanth.Aithal@amd.com>
^ permalink raw reply
* [PATCH kernel] crypto/ccp: Fix no-TSM build in SEV-TIO PCIe IDE
From: Alexey Kardashevskiy @ 2025-12-01 7:52 UTC (permalink / raw)
To: linux-crypto
Cc: linux-kernel, Ashish Kalra, Tom Lendacky, John Allen, Herbert Xu,
David S. Miller, Dan Williams, Borislav Petkov (AMD),
Alexey Kardashevskiy, linux-coco, Srikanth Aithal
In-Reply-To: <https://lore.kernel.org/r/ffbe5f5f-48c6-42b0-9f62-36fb0a4f67ab@amd.com>
Here are some cleanups for disable TSM+IDE.
Fixes: 3532f6154971 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Reported-by: Srikanth Aithal <Srikanth.Aithal@amd.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Better be just squashed into 3532f6154971 while it is in the next.
---
drivers/crypto/ccp/sev-dev-tio.h | 12 ------------
drivers/crypto/ccp/sev-dev.h | 8 ++++++++
drivers/crypto/ccp/sev-dev.c | 12 ++++--------
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev-tio.h b/drivers/crypto/ccp/sev-dev-tio.h
index 7c42351210ef..71f232a2b08b 100644
--- a/drivers/crypto/ccp/sev-dev-tio.h
+++ b/drivers/crypto/ccp/sev-dev-tio.h
@@ -7,8 +7,6 @@
#include <linux/pci-ide.h>
#include <uapi/linux/psp-sev.h>
-#if defined(CONFIG_CRYPTO_DEV_SP_PSP)
-
struct sla_addr_t {
union {
u64 sla;
@@ -129,14 +127,4 @@ int sev_tio_dev_connect(struct tsm_dsm_tio *dev_data, u8 tc_mask, u8 ids[8], u8
int sev_tio_dev_disconnect(struct tsm_dsm_tio *dev_data, bool force);
int sev_tio_dev_reclaim(struct tsm_dsm_tio *dev_data);
-#endif /* CONFIG_CRYPTO_DEV_SP_PSP */
-
-#if defined(CONFIG_PCI_TSM)
-void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page);
-void sev_tsm_uninit(struct sev_device *sev);
-int sev_tio_cmd_buffer_len(int cmd);
-#else
-static inline int sev_tio_cmd_buffer_len(int cmd) { return 0; }
-#endif
-
#endif /* __PSP_SEV_TIO_H__ */
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index dced4a8e9f01..d3e506206dbd 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -81,4 +81,12 @@ void sev_pci_exit(void);
struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages);
void snp_free_hv_fixed_pages(struct page *page);
+#if defined(CONFIG_PCI_TSM)
+void sev_tsm_init_locked(struct sev_device *sev, void *tio_status_page);
+void sev_tsm_uninit(struct sev_device *sev);
+int sev_tio_cmd_buffer_len(int cmd);
+#else
+static inline int sev_tio_cmd_buffer_len(int cmd) { return 0; }
+#endif
+
#endif /* __SEV_DEV_H */
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 365867f381e9..67ea9b30159a 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -38,7 +38,6 @@
#include "psp-dev.h"
#include "sev-dev.h"
-#include "sev-dev-tio.h"
#define DEVICE_NAME "sev"
#define SEV_FW_FILE "amd/sev.fw"
@@ -1365,11 +1364,6 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
return 0;
}
-static bool sev_tio_present(struct sev_device *sev)
-{
- return (sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED) != 0;
-}
-
static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
{
struct psp_device *psp = psp_master;
@@ -1448,10 +1442,12 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
data.list_paddr = __psp_pa(snp_range_list);
#if defined(CONFIG_PCI_TSM)
- data.tio_en = sev_tio_present(sev) &&
+ bool tio_supp = !!(sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED);
+
+ data.tio_en = tio_supp &&
sev_tio_enabled && psp_init_on_probe &&
amd_iommu_sev_tio_supported();
- if (sev_tio_present(sev) && !psp_init_on_probe)
+ if (tio_supp && !psp_init_on_probe)
dev_warn(sev->dev, "SEV-TIO as incompatible with psp_init_on_probe=0\n");
#endif
cmd = SEV_CMD_SNP_INIT_EX;
--
2.51.1
^ permalink raw reply related
* Re: [PATCH 1/3] KVM: guest_memfd: Remove preparation tracking
From: Yan Zhao @ 2025-12-01 2:51 UTC (permalink / raw)
To: Vishal Annapurve
Cc: Michael Roth, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <CAGtprH-ZhHO4C5gTqWgMNpf5MKvL0yz6QG2h01sz=0o=ZwOF0g@mail.gmail.com>
On Sun, Nov 30, 2025 at 05:35:41PM -0800, Vishal Annapurve wrote:
> On Mon, Nov 24, 2025 at 7:15 PM Yan Zhao <yan.y.zhao@intel.com> wrote:
> > > > > @@ -889,7 +872,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > > p = src ? src + i * PAGE_SIZE : NULL;
> > > > > ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
> > > > > if (!ret)
> > > > > - kvm_gmem_mark_prepared(folio);
> > > > > + folio_mark_uptodate(folio);
> > > > As also asked in [1], why is the entire folio marked as uptodate here? Why does
> > > > kvm_gmem_get_pfn() clear all pages of a huge folio when the folio isn't marked
> > > > uptodate?
> > >
> > > Quoting your example from[1] for more context:
> > >
> > > > I also have a question about this patch:
> > > >
> > > > Suppose there's a 2MB huge folio A, where
> > > > A1 and A2 are 4KB pages belonging to folio A.
> > > >
> > > > (1) kvm_gmem_populate() invokes __kvm_gmem_get_pfn() and gets folio A.
> > > > It adds page A1 and invokes folio_mark_uptodate() on folio A.
> > >
> > > In SNP hugepage patchset you responded to, it would only mark A1 as
> > You mean code in
> > https://github.com/amdese/linux/commits/snp-inplace-conversion-rfc1 ?
> >
> > > prepared/cleared. There was 4K-granularity tracking added to handle this.
> > I don't find the code that marks only A1 as "prepared/cleared".
> > Instead, I just found folio_mark_uptodate() is invoked by kvm_gmem_populate()
> > to mark the entire folio A as uptodate.
> >
> > However, according to your statement below that "uptodate flag only tracks
> > whether a folio has been cleared", I don't follow why and where the entire folio
> > A would be cleared if kvm_gmem_populate() only adds page A1.
>
> I think kvm_gmem_populate() is currently only used by SNP and TDX
> logic, I don't see an issue with marking the complete folio as
> uptodate even if its partially updated by kvm_gmem_populate() paths as
> the private memory will eventually get initialized anyways.
Still using the above example,
If only page A1 is passed to sev_gmem_post_populate(), will SNP initialize the
entire folio A?
- if yes, could you kindly point me to the code that does this? .
- if sev_gmem_post_populate() only initializes page A1, after marking the
complete folio A as uptodate in kvm_gmem_populate(), later faulting in page A2
in kvm_gmem_get_pfn() will not clear page A2 by invoking clear_highpage(),
since the entire folio A is uptodate. I don't understand why this is OK.
Or what's the purpose of invoking clear_highpage() on other folios?
Thanks
Yan
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Vishal Annapurve @ 2025-12-01 1:47 UTC (permalink / raw)
To: Yan Zhao
Cc: Michael Roth, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <aSQmAuxGK7+MUfRW@yzhao56-desk.sh.intel.com>
On Mon, Nov 24, 2025 at 1:34 AM Yan Zhao <yan.y.zhao@intel.com> wrote:
>
> > > > + if (src_offset) {
> > > > + src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i + 1]));
> > > > +
> > > > + memcpy(dst_vaddr + PAGE_SIZE - src_offset, src_vaddr, src_offset);
> > > > + kunmap_local(src_vaddr);
> > > IIUC, src_offset is the src's offset from the first page. e.g.,
> > > src could be 0x7fea82684100, with src_offset=0x100, while npages could be 512.
> > >
> > > Then it looks like the two memcpy() calls here only work when npages == 1 ?
> >
> > src_offset ends up being the offset into the pair of src pages that we
> > are using to fully populate a single dest page with each iteration. So
> > if we start at src_offset, read a page worth of data, then we are now at
> > src_offset in the next src page and the loop continues that way even if
> > npages > 1.
> >
> > If src_offset is 0 we never have to bother with straddling 2 src pages so
> > the 2nd memcpy is skipped on every iteration.
> >
> > That's the intent at least. Is there a flaw in the code/reasoning that I
> > missed?
> Oh, I got you. SNP expects a single src_offset applies for each src page.
>
> So if npages = 2, there're 4 memcpy() calls.
>
> src: |---------|---------|---------| (VA contiguous)
> ^ ^ ^
> | | |
> dst: |---------|---------| (PA contiguous)
>
>
> I previously incorrectly thought kvm_gmem_populate() should pass in src_offset
> as 0 for the 2nd src page.
>
> Would you consider checking if params.uaddr is PAGE_ALIGNED() in
> snp_launch_update() to simplify the design?
>
IIUC, this ship has sailed, as asserting this would break existing
userspace which can pass unaligned userspace buffers.
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Vishal Annapurve @ 2025-12-01 1:44 UTC (permalink / raw)
To: Michael Roth
Cc: Yan Zhao, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <20251121130144.u7eeaafonhcqf2bd@amd.com>
On Fri, Nov 21, 2025 at 5:02 AM Michael Roth <michael.roth@amd.com> wrote:
>
> >
> > Increasing GMEM_GUP_NPAGES to (1UL << PUD_ORDER) is probabaly not a good idea.
> >
> > Given both TDX/SNP map at 4KB granularity, why not just invoke post_populate()
> > per 4KB while removing the max_order from post_populate() parameters, as done
> > in Sean's sketch patch [1]?
>
> That's an option too, but SNP can make use of 2MB pages in the
> post-populate callback so I don't want to shut the door on that option
> just yet if it's not too much of a pain to work in. Given the guest BIOS
> lives primarily in 1 or 2 of these 2MB regions the benefits might be
> worthwhile, and SNP doesn't have a post-post-populate promotion path
> like TDX (at least, not one that would help much for guest boot times)
Given the small initial payload size, do you really think optimizing
for setting up huge page-aligned RMP entries is worthwhile?
The code becomes somewhat complex when trying to get this scenario
working and IIUC it depends on userspace-passed initial payload
regions aligning to the huge page size. What happens if userspace
tries to trigger snp_launch_update() for two unaligned regions within
the same huge page?
What Sean suggested earlier[1] seems relatively simpler to maintain.
[1] https://lore.kernel.org/kvm/aHEwT4X0RcfZzHlt@google.com/
>
> Thanks,
>
> Mike
^ permalink raw reply
* Re: [PATCH 1/3] KVM: guest_memfd: Remove preparation tracking
From: Vishal Annapurve @ 2025-12-01 1:35 UTC (permalink / raw)
To: Yan Zhao
Cc: Michael Roth, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <aSUe1UfD3hXg2iMZ@yzhao56-desk.sh.intel.com>
On Mon, Nov 24, 2025 at 7:15 PM Yan Zhao <yan.y.zhao@intel.com> wrote:
>
> On Fri, Nov 21, 2025 at 06:43:14AM -0600, Michael Roth wrote:
> > On Thu, Nov 20, 2025 at 05:12:55PM +0800, Yan Zhao wrote:
> > > On Thu, Nov 13, 2025 at 05:07:57PM -0600, Michael Roth wrote:
> > > > @@ -797,19 +782,25 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> > > > {
> > > > pgoff_t index = kvm_gmem_get_index(slot, gfn);
> > > > struct folio *folio;
> > > > - bool is_prepared = false;
> > > > int r = 0;
> > > >
> > > > CLASS(gmem_get_file, file)(slot);
> > > > if (!file)
> > > > return -EFAULT;
> > > >
> > > > - folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
> > > > + folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
> > > > if (IS_ERR(folio))
> > > > return PTR_ERR(folio);
> > > >
> > > > - if (!is_prepared)
> > > > - r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> > > > + if (!folio_test_uptodate(folio)) {
> > > > + unsigned long i, nr_pages = folio_nr_pages(folio);
> > > > +
> > > > + for (i = 0; i < nr_pages; i++)
> > > > + clear_highpage(folio_page(folio, i));
> > > > + folio_mark_uptodate(folio);
> > > Here, the entire folio is cleared only when the folio is not marked uptodate.
> > > Then, please check my questions at the bottom
> >
> > Yes, in this patch at least where I tried to mirror the current logic. I
> > would not be surprised if we need to rework things for inplace/hugepage
> > support though, but decoupling 'preparation' from the uptodate flag is
> > the main goal here.
> Could you elaborate a little why the decoupling is needed if it's not for
> hugepage?
IMO, decoupling is useful in general and we don't necessarily need to
wait till hugepage support lands to clean up this logic. Current
preparation logic has created some confusion regarding multiple
features for guest_memfd under discussion such as generic write, uffd
support, and direct map removal. It would be useful to simplify the
guest_memfd logic in this regard.
>
>
> > > > + }
> > > > +
> > > > + r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> > > >
> > > > folio_unlock(folio);
> > > >
> > > > @@ -852,7 +843,6 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > struct folio *folio;
> > > > gfn_t gfn = start_gfn + i;
> > > > pgoff_t index = kvm_gmem_get_index(slot, gfn);
> > > > - bool is_prepared = false;
> > > > kvm_pfn_t pfn;
> > > >
> > > > if (signal_pending(current)) {
> > > > @@ -860,19 +850,12 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > break;
> > > > }
> > > >
> > > > - folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, &max_order);
> > > > + folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> > > > if (IS_ERR(folio)) {
> > > > ret = PTR_ERR(folio);
> > > > break;
> > > > }
> > > >
> > > > - if (is_prepared) {
> > > > - folio_unlock(folio);
> > > > - folio_put(folio);
> > > > - ret = -EEXIST;
> > > > - break;
> > > > - }
> > > > -
> > > > folio_unlock(folio);
> > > > WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
> > > > (npages - i) < (1 << max_order));
> > > TDX could hit this warning easily when npages == 1, max_order == 9.
> >
> > Yes, this will need to change to handle that. I don't think I had to
> > change this for previous iterations of SNP hugepage support, but
> > there are definitely cases where a sub-2M range might get populated
> > even though it's backed by a 2M folio, so I'm not sure why I didn't
> > hit it there.
> >
> > But I'm taking Sean's cue on touching as little of the existing
> > hugepage logic as possible in this particular series so we can revisit
> > the remaining changes with some better context.
> Frankly, I don't understand why this patch 1 is required if we only want "moving
> GUP out of post_populate()" to work for 4KB folios.
>
> > >
> > > > @@ -889,7 +872,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> > > > p = src ? src + i * PAGE_SIZE : NULL;
> > > > ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
> > > > if (!ret)
> > > > - kvm_gmem_mark_prepared(folio);
> > > > + folio_mark_uptodate(folio);
> > > As also asked in [1], why is the entire folio marked as uptodate here? Why does
> > > kvm_gmem_get_pfn() clear all pages of a huge folio when the folio isn't marked
> > > uptodate?
> >
> > Quoting your example from[1] for more context:
> >
> > > I also have a question about this patch:
> > >
> > > Suppose there's a 2MB huge folio A, where
> > > A1 and A2 are 4KB pages belonging to folio A.
> > >
> > > (1) kvm_gmem_populate() invokes __kvm_gmem_get_pfn() and gets folio A.
> > > It adds page A1 and invokes folio_mark_uptodate() on folio A.
> >
> > In SNP hugepage patchset you responded to, it would only mark A1 as
> You mean code in
> https://github.com/amdese/linux/commits/snp-inplace-conversion-rfc1 ?
>
> > prepared/cleared. There was 4K-granularity tracking added to handle this.
> I don't find the code that marks only A1 as "prepared/cleared".
> Instead, I just found folio_mark_uptodate() is invoked by kvm_gmem_populate()
> to mark the entire folio A as uptodate.
>
> However, according to your statement below that "uptodate flag only tracks
> whether a folio has been cleared", I don't follow why and where the entire folio
> A would be cleared if kvm_gmem_populate() only adds page A1.
I think kvm_gmem_populate() is currently only used by SNP and TDX
logic, I don't see an issue with marking the complete folio as
uptodate even if its partially updated by kvm_gmem_populate() paths as
the private memory will eventually get initialized anyways.
>
> > There was an odd subtlety in that series though: it was defaulting to the
> > folio_order() for the prep-tracking/post-populate, but it would then clamp
> > it down based on the max order possible according whether that particular
> > order was a homogenous range of KVM_MEMORY_ATTRIBUTE_PRIVATE. Which is not
> > a great way to handle things, and I don't remember if I'd actually intended
> > to implement it that way or not... that's probably why I never tripped over
> > the WARN_ON() above, now that I think of it.
> >
> > But neither of these these apply to any current plans for hugepage support
> > that I'm aware of, so probably not worth working through what that series
> > did and look at this from a fresh perspective.
> >
> > >
> > > (2) kvm_gmem_get_pfn() later faults in page A2.
> > > As folio A is uptodate, clear_highpage() is not invoked on page A2.
> > > kvm_gmem_prepare_folio() is invoked on the whole folio A.
> > >
> > > (2) could occur at least in TDX when only a part the 2MB page is added as guest
> > > initial memory.
> > >
> > > My questions:
> > > - Would (2) occur on SEV?
> > > - If it does, is the lack of clear_highpage() on A2 a problem ?
> > > - Is invoking gmem_prepare on page A1 a problem?
> >
> > Assuming this patch goes upstream in some form, we will now have the
> > following major differences versus previous code:
> >
> > 1) uptodate flag only tracks whether a folio has been cleared
> > 2) gmem always calls kvm_arch_gmem_prepare() via kvm_gmem_get_pfn() and
> > the architecture can handle it's own tracking at whatever granularity
> > it likes.
> 2) looks good to me.
>
> > My hope is that 1) can similarly be done in such a way that gmem does not
> > need to track things at sub-hugepage granularity and necessitate the need
> > for some new data structure/state/flag to track sub-page status.
> I actually don't understand what uptodate flag helps gmem to track.
> Why can't clear_highpage() be done inside arch specific code? TDX doesn't need
> this clearing after all.
Target audience for guest_memfd includes non-confidential VMs as well.
Inline with shmem and other filesystems, guest_memfd should clear
pages on fault before handing them out to the users. There should be a
way to opt-out of this behavior for certain private faults like for
SNP/TDX and possibly for CCA as well.
>
> > My understanding based on prior discussion in guest_memfd calls was that
> > it would be okay to go ahead and clear the entire folio at initial allocation
> > time, and basically never mess with it again. It was also my understanding
> That's where I don't follow in this patch.
> I don't see where the entire folio A is cleared if it's only partially mapped by
> kvm_gmem_populate(). kvm_gmem_get_pfn() won't clear folio A either due to
> kvm_gmem_populate() has set the uptodate flag.
Since kvm_gmem_populate() is specific to SNP and TDX VMs, I don't
think this behavior is concerning.
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Borislav Petkov @ 2025-11-28 11:34 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Kiryl Shutsemau, Pratik R. Sampat, linux-mm, linux-coco,
linux-efi, x86, linux-kernel, tglx, mingo, dave.hansen, ardb,
akpm, osalvador, thomas.lendacky, michael.roth
In-Reply-To: <beec5651-5c23-4f5e-a0a3-d1cc01a8490a@kernel.org>
On Fri, Nov 28, 2025 at 10:30:15AM +0100, David Hildenbrand (Red Hat) wrote:
> kexecing the same kernel is typically used for kdump purposes.
>
> kexecing different kernels is used for all sorts of things (live-upgrade,
> grub-emu come to mind). It's quite common to kexec different kernels, or
> maybe I misunderstood the question here?
And my question is: since when do we enforce no-ABI-changes between kernels so
that we can kexec any kernel into any kernel?
By that logic I should be able to kexec 5.x into 6.x. I'll bet some money that
it won't work.
So unless it is written down somewhere, I think we should probably talk first
what we want to support and why...
Makes sense?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-11-28 9:34 UTC (permalink / raw)
To: Kiryl Shutsemau, Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <dy65eupwalp5wsljetlto27l6tjjvoygeotjd3n7mk7zjc4dma@jf4hzsy6rtcd>
On 11/27/25 18:40, Kiryl Shutsemau wrote:
> On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>>
>>
>> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
>>> On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
>>>> The unaccepted memory structure currently only supports accepting memory
>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>> reserved in memblock based on the initial memory layout, preventing
>>>> dynamic addition of memory ranges after boot. This causes guest
>>>> termination when memory is hot-added in a secure virtual machine due to
>>>> accessing pages that have not transitioned to private before use.
>>>
>>> How does the hot-pluggable memory look in EFI memory map? I thought
>>> hot-pluggable ranges suppose to be declared thare. The cleanest solution
>>> would be to have hot-pluggable and unaccepted indicated in EFI memory,
>>> so we can size bitmap accordingly upfront.
>>>
>>
>> I'm not quite sure if I fully understand. Do you mean to refer to the
>> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
>> memory? If so, wouldn't it still be desirable to increase the size of
>> the bitmap to what was marked as hotpluggable initially?
>
> I just don't understand how hotpluggable memory presented in EFI memory
> map in presence of unaccepted memory. If not-yet-plugged memory marked
> as unaccepted we can preallocate bitmap upfront and make unaccepted
> memory transparent wrt hotplug.
>
> BTW, isn't virtio-mem a more attractive target to support than HW-style
> hotplug?
I would have thought so as well, such that we can just let virtio-mem
take care of any acceptance before actually using hotplugged memory
(exposing it to the buddy).
Likely there is desire to support other hypervisors?
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-11-28 9:32 UTC (permalink / raw)
To: Pratik R. Sampat, linux-mm, linux-coco, linux-efi, x86,
linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <20251125175753.1428857-3-prsampat@amd.com>
On 11/25/25 18:57, Pratik R. Sampat wrote:
> The unaccepted memory structure currently only supports accepting memory
> present at boot time. The unaccepted table uses a fixed-size bitmap
> reserved in memblock based on the initial memory layout, preventing
> dynamic addition of memory ranges after boot. This causes guest
> termination when memory is hot-added in a secure virtual machine due to
> accessing pages that have not transitioned to private before use.
>
> Extend the unaccepted memory framework to handle hotplugged memory by
> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
> hotplugged ranges exceed the reserved bitmap capacity and switch to
> kernel-managed allocation.
>
> Hotplugged memory also follows the same acceptance policy using the
> accept_memory=[eager|lazy] kernel parameter to accept memory either
> up-front when added or before first use.
>
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> ---
> arch/x86/boot/compressed/efi.h | 1 +
> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
> include/linux/efi.h | 1 +
> include/linux/mm.h | 11 +++
> mm/memory_hotplug.c | 7 ++
> mm/page_alloc.c | 2 +
> 7 files changed, 106 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
> index 4f7027f33def..a220a1966cae 100644
> --- a/arch/x86/boot/compressed/efi.h
> +++ b/arch/x86/boot/compressed/efi.h
> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
> u32 unit_size;
> u64 phys_base;
> u64 size;
> + bool mem_reserved;
> unsigned long *bitmap;
> };
>
> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c b/drivers/firmware/efi/libstub/unaccepted_memory.c
> index c1370fc14555..b16bd61c12bf 100644
> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
> @@ -83,6 +83,7 @@ efi_status_t allocate_unaccepted_bitmap(__u32 nr_desc,
> unaccepted_table->unit_size = EFI_UNACCEPTED_UNIT_SIZE;
> unaccepted_table->phys_base = unaccepted_start;
> unaccepted_table->size = bitmap_size;
> + unaccepted_table->mem_reserved = true;
> memset(unaccepted_table->bitmap, 0, bitmap_size);
>
> status = efi_bs_call(install_configuration_table,
> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
> index 4479aad258f8..8537812346e2 100644
> --- a/drivers/firmware/efi/unaccepted_memory.c
> +++ b/drivers/firmware/efi/unaccepted_memory.c
> @@ -218,6 +218,89 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
> return ret;
> }
>
> +static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
> + unsigned long mem_range_size)
> +{
> + struct efi_unaccepted_memory *unacc_tbl;
> + unsigned long *old_bitmap, *new_bitmap;
> + phys_addr_t start, end, mem_range_end;
> + u64 phys_base, size, unit_size;
> + unsigned long flags;
> +
> + unacc_tbl = efi_get_unaccepted_table();
> + if (!unacc_tbl || !unacc_tbl->unit_size)
> + return -EIO;
> +
> + unit_size = unacc_tbl->unit_size;
> + phys_base = unacc_tbl->phys_base;
> +
> + mem_range_end = round_up(mem_range_start + mem_range_size, unit_size);
> + size = DIV_ROUND_UP(mem_range_end - phys_base, unit_size * BITS_PER_BYTE);
> +
> + /* Translate to offsets from the beginning of the bitmap */
> + start = mem_range_start - phys_base;
> + end = mem_range_end - phys_base;
> +
> + old_bitmap = efi_get_unaccepted_bitmap();
> + if (!old_bitmap)
> + return -EIO;
> +
> + /* If the bitmap is already large enough, just set the bits */
> + if (unacc_tbl->size >= size) {
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + bitmap_set(old_bitmap, start / unit_size, (end - start) / unit_size);
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> +
> + return 0;
> + }
> +
> + /* Reserved memblocks cannot be extended so allocate a new bitmap */
> + if (unacc_tbl->mem_reserved) {
> + new_bitmap = kzalloc(size, GFP_KERNEL);
> + if (!new_bitmap)
> + return -ENOMEM;
> +
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + memcpy(new_bitmap, old_bitmap, unacc_tbl->size);
> + unacc_tbl->mem_reserved = false;
> + free_reserved_area(old_bitmap, old_bitmap + unacc_tbl->size, -1, NULL);
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> + } else {
> + new_bitmap = krealloc(old_bitmap, size, GFP_KERNEL);
> + if (!new_bitmap)
> + return -ENOMEM;
> +
> + /* Zero the bitmap from the range it was extended from */
> + memset(new_bitmap + unacc_tbl->size, 0, size - unacc_tbl->size);
> + }
> +
> + bitmap_set(new_bitmap, start / unit_size, (end - start) / unit_size);
> +
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + unacc_tbl->size = size;
> + unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> +
> + return 0;
> +}
> +
> +int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
> +{
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
> + return 0;
> +
> + ret = extend_unaccepted_bitmap(mem_range_start, mem_range_size);
> + if (ret)
> + return ret;
> +
> + if (!mm_lazy_accept_enabled())
> + accept_memory(mem_range_start, mem_range_size);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PROC_VMCORE
> static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
> unsigned long pfn)
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index a74b393c54d8..1021eb78388f 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -545,6 +545,7 @@ struct efi_unaccepted_memory {
> u32 unit_size;
> u64 phys_base;
> u64 size;
> + bool mem_reserved;
> unsigned long *bitmap;
> };
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1ae97a0b8ec7..bb43876e6c47 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4077,6 +4077,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
>
> bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
> void accept_memory(phys_addr_t start, unsigned long size);
> +int accept_hotplug_memory(phys_addr_t mem_range_start,
> + unsigned long mem_range_size);
> +bool mm_lazy_accept_enabled(void);
>
> #else
>
> @@ -4090,6 +4093,14 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
> {
> }
>
> +static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
> + unsigned long mem_range_size)
> +{
> + return 0;
> +}
> +
> +static inline bool mm_lazy_accept_enabled(void) { return false; }
> +
> #endif
>
> static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 74318c787715..bf8086682b66 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1581,6 +1581,13 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> if (!strcmp(res->name, "System RAM"))
> firmware_map_add_hotplug(start, start + size, "System RAM");
>
> + ret = accept_hotplug_memory(start, size);
What makes this special that we have to have "hotplug_memory" as part of
the name?
Staring at the helper itself, there isn't anything really hotplug
specific happening in there except extending the bitmap, maybe?
--
Cheers
David
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: David Hildenbrand (Red Hat) @ 2025-11-28 9:30 UTC (permalink / raw)
To: Borislav Petkov, Kiryl Shutsemau
Cc: Pratik R. Sampat, linux-mm, linux-coco, linux-efi, x86,
linux-kernel, tglx, mingo, dave.hansen, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <20251127181233.GBaSiUkaLzwANS_6WT@fat_crate.local>
On 11/27/25 19:12, Borislav Petkov wrote:
> On Thu, Nov 27, 2025 at 05:35:57PM +0000, Kiryl Shutsemau wrote:
>>> ABI break for kexec? Is that a thing?
>>>
>>> Since when do we enforce ABI compatibility for kexec and where are we
>>> documenting that?
>
> I'll take that as a "no".
>
>> The whole purpose of kexec() is to switch between kernel versions. This
>> struct defines format we communicate information about unaccepted memory
>> between kernels. The mismatch will lead to boot failure.
>>
>> The structure is versioned. Ideally, we should know the format of the
>> structure the next kernel supports and act accordingly in the first
>> kernel. Like, we can accept all memory before kexec on mismatch.
>
> None of that matters if you kexec the same kernels.
>
> IOW, for some reason you want to be able to kexec different kernels. The
> question is why do we care?
kexecing the same kernel is typically used for kdump purposes.
kexecing different kernels is used for all sorts of things
(live-upgrade, grub-emu come to mind). It's quite common to kexec
different kernels, or maybe I misunderstood the question here?
--
Cheers
David
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Xiaoyao Li @ 2025-11-28 2:40 UTC (permalink / raw)
To: Andrew Cooper, kas
Cc: bp, chao.p.peng, chenyi.qiang, dave.hansen, hpa, kvm, linux-coco,
linux-kernel, mingo, reinette.chatre, rick.p.edgecombe, tglx, x86
In-Reply-To: <4676722f-98a3-4217-a357-068440dc6e14@citrix.com>
Hi Andrew,
On 11/28/2025 12:55 AM, Andrew Cooper wrote:
>> I am not sure. Leaving it as produces produces false messages which is
>> not good, but not critical.
>>
>> Maybe just clear X86_FEATURE_BUS_LOCK_DETECT and stop pretending we
>> control split-lock behaviour from the guest?
>
> (Having just played with this mess for another task) you're talking
> about two different things.
>
> Sapphire Rapids has an architectural BUS_LOCK_DETECT (trap semantics,
> #DB or VMExit), and a model-specific BUS_LOCK_DISABLE.
>
> It's BUS_LOCK_DISABLE which generates #AC, with fault semantics,
> preventing forward progress. It also means the Bus Lock didn't happen,
> and there's nothing to trigger the BUS_LOCK_DETECT (trap) behaviour.
>
> Given that TDX is enabling BUS_LOCK_DISABLE, it's probably also enabling
> UC_LOCK_DISABLE (causes #GP) too.
Well, more accurate, it's SPLIT_LOCK_DISABLE, not BUS_LOCK_DISABLE.(bus
lock have two types: split lock and uc lock)
No, it's not TDX who is enabling SPLIT_LOCK_DISABLE, but the host. The
default mode of Linux is "warn", so that by default the host Linux
enables SPLIT_LOCK_DISABLE. And TDX module doesn't context switch
MSR_TEST_CTRL when entering into the TDX vCPU because MSR_TEST_CTRL is
not virtualizable. Thus SPLIT_LOCK_DISABLE remains enabled when TDX vCPU
is running.
Regarding UC_LOCK_DISABLE, Linux doesn't enable it. Not sure if BIOS
enables it or not (as far as I know, I don't see any bios enables it)
> Looking at the backtrace:
>
> x86/split lock detection: #AC: split_lock/1176 took a split_lock trap at address: 0x5630b30921f9
> unchecked MSR access error: WRMSR to 0x33 (tried to write 0x0000000000000000) at rIP: 0xffffffff812a061f (native_write_msr+0xf/0x30)
>
>
> First, "took a split_lock trap" is wrong. It's a fault, not a trap.
Hi x86 maintainers,
Should we fix it?
> Second, because the attempt to disable BUS_LOCK_DISABLE was blocked,
> simply retrying the instruction will generate a new #AC and livelock.
> Linux probably ought to raise SIGSEGV with userspace, for want of
> anything better to do.
This patch is just achieving this, while it raises the SIGBUS to userspace.
> It looks like software in a TDX VM will simply have to accept that it
> cannot cause a bus lock.
If the host doesn't enable SPLIT_LOCK_DISABLE, then split lock might not
be fatal to TDX guests.
> ~Andrew
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Xiaoyao Li @ 2025-11-28 2:28 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco, kvm,
Reinette Chatre, Chenyi Qiang, chao.p.peng
In-Reply-To: <f2hkqt5xtmej7cfnuytigcfszr3qja4l6ywww4qrqxjbqmlko2@r75b6deae2hd>
On 11/28/2025 12:16 AM, Kiryl Shutsemau wrote:
> On Thu, Nov 27, 2025 at 10:00:58AM +0800, Xiaoyao Li wrote:
>> On 11/26/2025 9:35 PM, Kiryl Shutsemau wrote:
>>> On Wed, Nov 26, 2025 at 08:17:18PM +0800, Xiaoyao Li wrote:
>>>> On 11/26/2025 7:25 PM, Kiryl Shutsemau wrote:
>>>>> On Wed, Nov 26, 2025 at 06:02:03PM +0800, Xiaoyao Li wrote:
>>>>>> When the host enables split lock detection feature, the split lock from
>>>>>> guests (normal or TDX) triggers #AC. The #AC caused by split lock access
>>>>>> within a normal guest triggers a VM Exit and is handled in the host.
>>>>>> The #AC caused by split lock access within a TDX guest does not trigger
>>>>>> a VM Exit and instead it's delivered to the guest self.
>>>>>>
>>>>>> The default "warning" mode of handling split lock depends on being able
>>>>>> to temporarily disable detection to recover from the split lock event.
>>>>>> But the MSR that disables detection is not accessible to a guest.
>>>>>>
>>>>>> This means that TDX guests today can not disable the feature or use
>>>>>> the "warning" mode (which is the default). But, they can use the "fatal"
>>>>>> mode.
>>>>>>
>>>>>> Force TDX guests to use the "fatal" mode.
>>>>>>
>>>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>>>> ---
>>>>>> arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
>>>>>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
>>>>>> index 981f8b1f0792..f278e4ea3dd4 100644
>>>>>> --- a/arch/x86/kernel/cpu/bus_lock.c
>>>>>> +++ b/arch/x86/kernel/cpu/bus_lock.c
>>>>>> @@ -315,9 +315,24 @@ void bus_lock_init(void)
>>>>>> wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
>>>>>> }
>>>>>> +static bool split_lock_fatal(void)
>>>>>> +{
>>>>>> + if (sld_state == sld_fatal)
>>>>>> + return true;
>>>>>> +
>>>>>> + /*
>>>>>> + * TDX guests can not disable split lock detection.
>>>>>> + * Force them into the fatal behavior.
>>>>>> + */
>>>>>> + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
>>>>>> + return true;
>>>>>> +
>>>>>> + return false;
>>>>>> +}
>>>>>> +
>>>>>> bool handle_user_split_lock(struct pt_regs *regs, long error_code)
>>>>>> {
>>>>>> - if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
>>>>>> + if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
>>>>>> return false;
>>>>>
>>>>> Maybe it would be cleaner to make it conditional on
>>>>> cpu_model_supports_sld instead of special-casing TDX guest?
>>>>>
>>>>> #AC on any platfrom when we didn't asked for it suppose to be fatal, no?
>>>>
>>>> But TDX is the only one has such special non-architectural behavior.
>>>>
>>>> For example, for normal VMs under KVM, the behavior is x86 architectural.
>>>> MSR_TEST_CTRL is not accessible to normal VMs, and no split lock #AC will be
>>>> delivered to the normal VMs because it's handled by KVM.
>>>
>>> How does it contradict what I suggested?
>>>
>>> For both normal VMs and TDX guest, cpu_model_supports_sld will not be
>>> set to true. So check for cpu_model_supports_sld here is going to be
>>> NOP, unless #AC actually delivered, like we have in TDX case. Handling
>>> it as fatal is sane behaviour in such case regardless if it TDX.
>>>
>>> And we don't need to make the check explicitly about TDX guest.
>>
>> Well, it depends on how defensive we would like to be, and whether to
>> specialize or commonize the issue.
>>
>> Either can work. If the preference and agreement are to commonize the issue,
>> I can do it in v2. And in this direction, what should we do with the patch
>> 2? just drop it since it's specialized for TDX ?
>
> I am not sure. Leaving it as produces produces false messages which is
> not good, but not critical.
>
> Maybe just clear X86_FEATURE_BUS_LOCK_DETECT and stop pretending we
> control split-lock behaviour from the guest?
By clearing X86_FEATURE_BUS_LOCK_DETECT, the TDX guest log doens't print
anything about split lock detection. But the TDX guest is still possible
to get #AC on split locks, which seems no good as well.
More, it's overkill to clear X86_FEATURE_BUS_LOCK_DETECT. Clearing it
means TDX guest cannot use X86_FEATURE_BUS_LOCK_DETECT to detect the bus
lock happens from the guest userspace, even when the host doesn't enable
split lock detection. For example, on cloud environment, if the
customers want to run their legacy workload that can generate split
locks, the CSP would have to disable slit lock detection in the host but
use bus lock VM exit to catch the bus lock in the TDX guest. In this
case, TDX guest is free to use X86_FEATURE_BUS_LOCK_DETECT and it works
as expected.
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Borislav Petkov @ 2025-11-27 18:12 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Pratik R. Sampat, linux-mm, linux-coco, linux-efi, x86,
linux-kernel, tglx, mingo, dave.hansen, ardb, akpm, david,
osalvador, thomas.lendacky, michael.roth
In-Reply-To: <m3l6gcjmbabudtnqwv6w67t7iz2mpmbjyrpnmiq5k2iyargn5d@nyf2zzxx7yme>
On Thu, Nov 27, 2025 at 05:35:57PM +0000, Kiryl Shutsemau wrote:
> > ABI break for kexec? Is that a thing?
> >
> > Since when do we enforce ABI compatibility for kexec and where are we
> > documenting that?
I'll take that as a "no".
> The whole purpose of kexec() is to switch between kernel versions. This
> struct defines format we communicate information about unaccepted memory
> between kernels. The mismatch will lead to boot failure.
>
> The structure is versioned. Ideally, we should know the format of the
> structure the next kernel supports and act accordingly in the first
> kernel. Like, we can accept all memory before kexec on mismatch.
None of that matters if you kexec the same kernels.
IOW, for some reason you want to be able to kexec different kernels. The
question is why do we care?
AFAICT, nowhere do we say that there's an ABI between kexec-ed kernels...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Kiryl Shutsemau @ 2025-11-27 17:40 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <14df1d99-7df0-4982-8029-e66dfb140399@amd.com>
On Wed, Nov 26, 2025 at 04:27:29PM -0600, Pratik R. Sampat wrote:
>
>
> On 11/26/25 5:12 AM, Kiryl Shutsemau wrote:
> > On Tue, Nov 25, 2025 at 11:57:51AM -0600, Pratik R. Sampat wrote:
> >> The unaccepted memory structure currently only supports accepting memory
> >> present at boot time. The unaccepted table uses a fixed-size bitmap
> >> reserved in memblock based on the initial memory layout, preventing
> >> dynamic addition of memory ranges after boot. This causes guest
> >> termination when memory is hot-added in a secure virtual machine due to
> >> accessing pages that have not transitioned to private before use.
> >
> > How does the hot-pluggable memory look in EFI memory map? I thought
> > hot-pluggable ranges suppose to be declared thare. The cleanest solution
> > would be to have hot-pluggable and unaccepted indicated in EFI memory,
> > so we can size bitmap accordingly upfront.
> >
>
> I'm not quite sure if I fully understand. Do you mean to refer to the
> EFI_MEMORY_HOT_PLUGGABLE attribute that is used for cold plugged boot
> memory? If so, wouldn't it still be desirable to increase the size of
> the bitmap to what was marked as hotpluggable initially?
I just don't understand how hotpluggable memory presented in EFI memory
map in presence of unaccepted memory. If not-yet-plugged memory marked
as unaccepted we can preallocate bitmap upfront and make unaccepted
memory transparent wrt hotplug.
BTW, isn't virtio-mem a more attractive target to support than HW-style
hotplug?
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Kiryl Shutsemau @ 2025-11-27 17:35 UTC (permalink / raw)
To: Borislav Petkov
Cc: Pratik R. Sampat, linux-mm, linux-coco, linux-efi, x86,
linux-kernel, tglx, mingo, dave.hansen, ardb, akpm, david,
osalvador, thomas.lendacky, michael.roth
In-Reply-To: <20251126223127.GIaSd_v7juUkaW4RTA@fat_crate.local>
On Wed, Nov 26, 2025 at 11:31:27PM +0100, Borislav Petkov wrote:
> On Wed, Nov 26, 2025 at 11:12:13AM +0000, Kiryl Shutsemau wrote:
> > > diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
> > > index 4f7027f33def..a220a1966cae 100644
> > > --- a/arch/x86/boot/compressed/efi.h
> > > +++ b/arch/x86/boot/compressed/efi.h
> > > @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
> > > u32 unit_size;
> > > u64 phys_base;
> > > u64 size;
> > > + bool mem_reserved;
> > > unsigned long *bitmap;
> > > };
> > >
> >
> > Again, this is ABI break for kexec.
>
> ABI break for kexec? Is that a thing?
>
> Since when do we enforce ABI compatibility for kexec and where are we
> documenting that?
The whole purpose of kexec() is to switch between kernel versions. This
struct defines format we communicate information about unaccepted memory
between kernels. The mismatch will lead to boot failure.
The structure is versioned. Ideally, we should know the format of the
structure the next kernel supports and act accordingly in the first
kernel. Like, we can accept all memory before kexec on mismatch.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH 1/4] efi/libstub: Decouple memory bitmap from the unaccepted table
From: Kiryl Shutsemau @ 2025-11-27 17:29 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: linux-mm, linux-coco, linux-efi, x86, linux-kernel, tglx, mingo,
bp, dave.hansen, ardb, akpm, david, osalvador, thomas.lendacky,
michael.roth
In-Reply-To: <e66ef586-0e19-4e53-af34-129ed28675c7@amd.com>
On Wed, Nov 26, 2025 at 04:27:19PM -0600, Pratik R. Sampat wrote:
> Hi Kiryl,
>
> Thanks for you comments.
>
> On 11/26/25 5:08 AM, Kiryl Shutsemau wrote:
> > On Tue, Nov 25, 2025 at 11:57:50AM -0600, Pratik R. Sampat wrote:
> >> Memory hotplug in secure environments requires the unaccepted memory
> >> bitmap to grow as new memory is added. Currently, the bitmap is
> >> implemented as a flexible array member at the end of struct
> >> efi_unaccepted_memory, which is reserved by memblock at boot and cannot
> >> be resized without reallocating the entire structure.
> >>
> >> Replace the flexible array member with a pointer.
> >
> > Well, it break interoperability between kernel before and after the
> > patch. Consider kexec from kernel without the patch to the kernel with
> > the patch and then back to older kernel. It is ABI break.
> >
> > Is re-allocating the entire structure such a big pain?
> >
>
> We could do that. My concern is that we would then need to protect the
> entire table instead of just the bitmap, which may add an additional
> overhead?
What additional overhead? The main contention is going to be on binmap
anyway.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Andrew Cooper @ 2025-11-27 16:55 UTC (permalink / raw)
To: kas
Cc: bp, chao.p.peng, chenyi.qiang, dave.hansen, hpa, kvm, linux-coco,
linux-kernel, mingo, reinette.chatre, rick.p.edgecombe, tglx, x86,
xiaoyao.li, Andrew Cooper
In-Reply-To: <f2hkqt5xtmej7cfnuytigcfszr3qja4l6ywww4qrqxjbqmlko2@r75b6deae2hd>
> I am not sure. Leaving it as produces produces false messages which is
> not good, but not critical.
>
> Maybe just clear X86_FEATURE_BUS_LOCK_DETECT and stop pretending we
> control split-lock behaviour from the guest?
(Having just played with this mess for another task) you're talking
about two different things.
Sapphire Rapids has an architectural BUS_LOCK_DETECT (trap semantics,
#DB or VMExit), and a model-specific BUS_LOCK_DISABLE.
It's BUS_LOCK_DISABLE which generates #AC, with fault semantics,
preventing forward progress. It also means the Bus Lock didn't happen,
and there's nothing to trigger the BUS_LOCK_DETECT (trap) behaviour.
Given that TDX is enabling BUS_LOCK_DISABLE, it's probably also enabling
UC_LOCK_DISABLE (causes #GP) too.
Looking at the backtrace:
x86/split lock detection: #AC: split_lock/1176 took a split_lock trap at address: 0x5630b30921f9
unchecked MSR access error: WRMSR to 0x33 (tried to write 0x0000000000000000) at rIP: 0xffffffff812a061f (native_write_msr+0xf/0x30)
First, "took a split_lock trap" is wrong. It's a fault, not a trap.
Second, because the attempt to disable BUS_LOCK_DISABLE was blocked,
simply retrying the instruction will generate a new #AC and livelock.
Linux probably ought to raise SIGSEGV with userspace, for want of
anything better to do.
It looks like software in a TDX VM will simply have to accept that it
cannot cause a bus lock.
~Andrew
^ permalink raw reply
* Re: [PATCH 1/2] x86/split_lock: Don't try to handle user split lock in TDX guest
From: Kiryl Shutsemau @ 2025-11-27 16:16 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-kernel, linux-coco, kvm,
Reinette Chatre, Chenyi Qiang, chao.p.peng
In-Reply-To: <0f8983e9-0e23-4a05-8015-de6e2218d8a5@intel.com>
On Thu, Nov 27, 2025 at 10:00:58AM +0800, Xiaoyao Li wrote:
> On 11/26/2025 9:35 PM, Kiryl Shutsemau wrote:
> > On Wed, Nov 26, 2025 at 08:17:18PM +0800, Xiaoyao Li wrote:
> > > On 11/26/2025 7:25 PM, Kiryl Shutsemau wrote:
> > > > On Wed, Nov 26, 2025 at 06:02:03PM +0800, Xiaoyao Li wrote:
> > > > > When the host enables split lock detection feature, the split lock from
> > > > > guests (normal or TDX) triggers #AC. The #AC caused by split lock access
> > > > > within a normal guest triggers a VM Exit and is handled in the host.
> > > > > The #AC caused by split lock access within a TDX guest does not trigger
> > > > > a VM Exit and instead it's delivered to the guest self.
> > > > >
> > > > > The default "warning" mode of handling split lock depends on being able
> > > > > to temporarily disable detection to recover from the split lock event.
> > > > > But the MSR that disables detection is not accessible to a guest.
> > > > >
> > > > > This means that TDX guests today can not disable the feature or use
> > > > > the "warning" mode (which is the default). But, they can use the "fatal"
> > > > > mode.
> > > > >
> > > > > Force TDX guests to use the "fatal" mode.
> > > > >
> > > > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > > > ---
> > > > > arch/x86/kernel/cpu/bus_lock.c | 17 ++++++++++++++++-
> > > > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
> > > > > index 981f8b1f0792..f278e4ea3dd4 100644
> > > > > --- a/arch/x86/kernel/cpu/bus_lock.c
> > > > > +++ b/arch/x86/kernel/cpu/bus_lock.c
> > > > > @@ -315,9 +315,24 @@ void bus_lock_init(void)
> > > > > wrmsrq(MSR_IA32_DEBUGCTLMSR, val);
> > > > > }
> > > > > +static bool split_lock_fatal(void)
> > > > > +{
> > > > > + if (sld_state == sld_fatal)
> > > > > + return true;
> > > > > +
> > > > > + /*
> > > > > + * TDX guests can not disable split lock detection.
> > > > > + * Force them into the fatal behavior.
> > > > > + */
> > > > > + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> > > > > + return true;
> > > > > +
> > > > > + return false;
> > > > > +}
> > > > > +
> > > > > bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> > > > > {
> > > > > - if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal)
> > > > > + if ((regs->flags & X86_EFLAGS_AC) || split_lock_fatal())
> > > > > return false;
> > > >
> > > > Maybe it would be cleaner to make it conditional on
> > > > cpu_model_supports_sld instead of special-casing TDX guest?
> > > >
> > > > #AC on any platfrom when we didn't asked for it suppose to be fatal, no?
> > >
> > > But TDX is the only one has such special non-architectural behavior.
> > >
> > > For example, for normal VMs under KVM, the behavior is x86 architectural.
> > > MSR_TEST_CTRL is not accessible to normal VMs, and no split lock #AC will be
> > > delivered to the normal VMs because it's handled by KVM.
> >
> > How does it contradict what I suggested?
> >
> > For both normal VMs and TDX guest, cpu_model_supports_sld will not be
> > set to true. So check for cpu_model_supports_sld here is going to be
> > NOP, unless #AC actually delivered, like we have in TDX case. Handling
> > it as fatal is sane behaviour in such case regardless if it TDX.
> >
> > And we don't need to make the check explicitly about TDX guest.
>
> Well, it depends on how defensive we would like to be, and whether to
> specialize or commonize the issue.
>
> Either can work. If the preference and agreement are to commonize the issue,
> I can do it in v2. And in this direction, what should we do with the patch
> 2? just drop it since it's specialized for TDX ?
I am not sure. Leaving it as produces produces false messages which is
not good, but not critical.
Maybe just clear X86_FEATURE_BUS_LOCK_DETECT and stop pretending we
control split-lock behaviour from the guest?
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox