* Re: [PATCH] x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
From: Huang, Kai @ 2026-01-20 23:31 UTC (permalink / raw)
To: seanjc@google.com, x86@kernel.org, dave.hansen@linux.intel.com,
kas@kernel.org, bp@alien8.de, mingo@redhat.com,
pbonzini@redhat.com, tglx@kernel.org
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Annapurve, Vishal, linux-kernel@vger.kernel.org,
Edgecombe, Rick P, Zhao, Yan Y, ackerleytng@google.com
In-Reply-To: <20260120203937.1447592-1-seanjc@google.com>
On Tue, 2026-01-20 at 12:39 -0800, Sean Christopherson wrote:
> Rework the TDX APIs to take the kernel's 1-based pg_level enum, not the
> TDX-Module's 0-based level. The APIs are _kernel_ APIs, not TDX-Module
> APIs, and the kernel (and KVM) uses "enum pg_level" literally everywhere.
>
> Using "enum pg_level" eliminates ambiguity when looking at the APIs (it's
> NOT clear that "int level" refers to the TDX-Module's level), and will
> allow for using existing helpers like page_level_size() when support for
> hugepages is added to the S-EPT APIs.
>
> No functional change intended.
>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Vishal Annapurve <vannapurve@google.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
>
Tested that running/destroying TD worked fine:
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Kai Huang <kai.huang@intel.com>
^ permalink raw reply
* Re: [PATCH v1 02/14] x86/realmode: make %gs == 0 an invariant
From: H. Peter Anvin @ 2026-01-21 0:47 UTC (permalink / raw)
To: David Laight
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Uros Bizjak, Petr Mladek, Andrew Morton, Kees Cook,
Peter Zijlstra (Intel), Nathan Chancellor, Kiryl Shutsemau,
Rick Edgecombe, linux-kernel, linux-coco, x86
In-Reply-To: <20260120224014.29e7f326@pumpkin>
On 2026-01-20 14:40, David Laight wrote:
>
> Would it be better to wrap that as (say) read_abs_32() since the
> objective is to read an absolute address (using the relevant zero segment
> register) rather than to read though either fs or gs.
>
> Is this code all running in 32bit linear mode with non-zero cs/ss/ds
> segment registers and 'suitable' entries in the GDT?
> That mode makes my brain hurt :-)
> Or is there a fudge to get 16bit asm from the C.
>
To address your specific question, no I don't think that is a good idea, since
it would imply that you can reach an arbitrary linear address, which is
definitely not the case.
At least this way the user has to look at what they are doing, which I think
is a good thing for this specific code.
-hpa
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Sean Christopherson @ 2026-01-21 0:52 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, tglx, vannapurve, x86,
yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-13-rick.p.edgecombe@intel.com>
On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> static int tdx_topup_external_fault_cache(struct kvm_vcpu *vcpu, unsigned int cnt)
> {
> - struct vcpu_tdx *tdx = to_tdx(vcpu);
> + struct tdx_prealloc *prealloc = &to_tdx(vcpu)->prealloc;
> + int min_fault_cache_size;
>
> - return kvm_mmu_topup_memory_cache(&tdx->mmu_external_spt_cache, cnt);
> + /* External page tables */
> + min_fault_cache_size = cnt;
> + /* Dynamic PAMT pages (if enabled) */
> + min_fault_cache_size += tdx_dpamt_entry_pages() * PT64_ROOT_MAX_LEVEL;
The caller passed in number of pages to be added as @cnt, don't hardcode what
could be conflicting information. If the caller wants to add 50 pages, then this
code damn well needs to prepare for adding 50 pages, not 5.
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Edgecombe, Rick P @ 2026-01-21 0:58 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <aXAjMxbeMO0pioNF@google.com>
On Tue, 2026-01-20 at 16:52 -0800, Sean Christopherson wrote:
> The caller passed in number of pages to be added as @cnt, don't hardcode what
> could be conflicting information. If the caller wants to add 50 pages, then this
> code damn well needs to prepare for adding 50 pages, not 5.
Doh, yes totally.
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Aneesh Kumar K.V @ 2026-01-21 4:50 UTC (permalink / raw)
To: Robin Murphy, Suzuki K Poulose, linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, jgg, steven.price, Marek Szyprowski
In-Reply-To: <b662e137-5d2a-4ee7-b1f7-b4545388d4fe@arm.com>
Robin Murphy <robin.murphy@arm.com> writes:
> On 2026-01-20 2:25 pm, Aneesh Kumar K.V wrote:
>> Robin Murphy <robin.murphy@arm.com> writes:
>>
>>> On 2026-01-20 9:59 am, Suzuki K Poulose wrote:
>>>> On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
>>>>> On systems that apply an address encryption tag or mask to DMA addresses,
>>>>> DMA mask validation must be performed against the canonical DMA address.
>>>>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>>>>> can incorrectly fail capability checks, since architecture-specific
>>>>> encryption bits are not part of the device’s actual DMA addressing
>>>>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>>>>> addresses.
>>>>>
>>>>> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
>>>>> that the architecture encryption mask does not influence the check.
>>>>>
>>>>> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
>>>>
>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>>>> ---
>>>>> kernel/dma/direct.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>>>> index 8e04f72baaa3..a5639e9415f5 100644
>>>>> --- a/kernel/dma/direct.c
>>>>> +++ b/kernel/dma/direct.c
>>>>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64
>>>>> mask)
>>>>> /*
>>>>> * This check needs to be against the actual bit mask value, so use
>>>>> - * phys_to_dma_unencrypted() here so that the SME encryption mask
>>>>> isn't
>>>>> + * __phys_to_dma() here so that the arch specific encryption mask
>>>>> isn't
>>>>> * part of the check.
>>>>> */
>>>>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>>>>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>>>>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>>>>> + return mask >= __phys_to_dma(dev, min_mask);
>>>>
>>>> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
>>>> added to the PA, it is really part of the actual "PA" and thus must be
>>>> checked against the full PA ?
>>>
>>> Yes, it's much the same as for AMD SEV (albeit the other way round) -
>>> the encryption/decryption bit is part of the DMA address because it
>>> needs to be driven by the device, so it is crucial that the device's DMA
>>> mask is capable of expressing that.
>>>
>>
>> Commit c92a54cfa0257e8ffd66b2a17d49e9c0bd4b769f explains these details.
>
> See x86's force_dma_unencrypted() implementation - the reason dma-direct
> doesn't need to be so strict for that is because things are the right
> way round that it can always fall back to shared/untrusted DMA as the
> general case, and a device can only access an encrypted page directly if
> it *can* drive the SME bit in the input address to trigger the inline
> encryption engine.
>
> For CCA we have rather the opposite, where dma-direct requires a device
> to be able to address any IPA directly to be sure of working at all, but
> if we do happen to have a stage 1 IOMMU then we could rely on that to
> map smaller IOVAs to the upper IPA space.
>
>> I was wondering whether the DMA-enable operation should live outside the
>> set_mask operation. Conceptually, set_mask should be derived purely from
>> max_pfn, while the DMA enablement path could additionally verify whether
>> the device requires access to an alias address, depending on whether it
>> is operating in trusted or untrusted mode?
>
> No, the point of the set_mask check is "is this mask big enough to
> accommodate any *DMA address* we might need to give the device?" - that
> includes offsets, magic bits, magic bits encoded as fake offsets (yes
> you, Raspberry Pi...) and whatever else might comprise an actual DMA
> address as handed off to the hardware. It is *not* directly a "can this
> device DMA to all RAM we know about?" check - that is in the assumption
> that if necessary the SWIOTLB buffer will be reachable at a <=32-bit DMA
> address, and thus we should not *need* to give >32-bit addresses to
> devices. It is only that assumption which fundamentally falls apart for
> CCA (with more than 2GB of IPA space, at least).
>
We need the below change then? commit 91ef26f914171cf753330f13724fd9142b5b1640
discuss some hardware that is broken with the usage of phys_to_dma conversion.
modified kernel/dma/direct.c
@@ -14,6 +14,7 @@
#include <linux/set_memory.h>
#include <linux/slab.h>
#include <linux/pci-p2pdma.h>
+#include <linux/cc_platform.h>
#include "direct.h"
/*
@@ -579,17 +580,23 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
int dma_direct_supported(struct device *dev, u64 mask)
{
- u64 min_mask = (max_pfn - 1) << PAGE_SHIFT;
+ u64 min_mask = DMA_BIT_MASK(32);
/*
- * Because 32-bit DMA masks are so common we expect every architecture
- * to be able to satisfy them - either by not supporting more physical
- * memory, or by providing a ZONE_DMA32. If neither is the case, the
- * architecture needs to use an IOMMU instead of the direct mapping.
+ * Only do the conversion for CC platform, to be compatible
+ * commit 91ef26f914171cf753330f13724fd9142b5b1640
*/
- if (mask >= DMA_BIT_MASK(32))
+ if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
+ min_mask = phys_to_dma_unencrypted(dev, min_mask);
+
+ /*
+ * if we support ZONE_DMA32 and device mask can cover the DMA32 range,
+ * then we can support direct dma for any max_pfn value using swiotlb.
+ */
+ if (IS_ENABLED(CONFIG_ZONE_DMA32) && mask >= min_mask)
return 1;
+ min_mask = (max_pfn - 1) << PAGE_SHIFT;
/*
* This check needs to be against the actual bit mask value, so use
* __phys_to_dma() here so that the arch specific encryption mask isn't
-aneesh
^ permalink raw reply
* Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the real-mode boot code
From: Uros Bizjak @ 2026-01-21 8:56 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-12-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> All supported versions of gcc support __seg_fs and __seg_gs now.
> All supported versions of clang support __seg_fs and __seg_gs too,
> except for two bugs (as of clang 21, at least):
>
> 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> 2. An internal compiler error when addressing symbols directly.
>
> However, none of these are required in the boot code. Furthermore,
> this makes it possible to remove the absolute_pointer() hack in the
> fs/gs access functions.
>
> This requires adding a barrier() to a20.c, to prevent the compiler
> from eliding the load from the aliased memory address.
>
> Remove the unused memcmp_[fg]s() functions.
>
> Finally, ds() is by necessity constant, so mark the function as such.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> ---
> arch/x86/boot/a20.c | 1 +
> arch/x86/boot/boot.h | 81 ++++++++++++++------------------------------
> 2 files changed, 27 insertions(+), 55 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 3ab6cd8eaa31..52c3fccdcb70 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -63,6 +63,7 @@ static int a20_test(int loops)
> while (loops--) {
> wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> + barrier(); /* Compiler won't know about fs/gs overlap */
> ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> if (ok)
> break;
This particular issue is not due to the compiler not knowing about
fs/gs overlap (if the compiler determines that write and read are to
the same non-volatile address, it would simply remove both, load and
store), but due to the compiler performing load hoisting and store
sinking from the loop. The compiler considers these two addresses as
two different addresses (they are also defined in two different named
address spaces), and optimizes access to them. So, without barrier(),
it simply loads the value from A20_TEST_ADDR and A20_TEST_ADDR+0x10
before the loop, resulting in:
9: 65 8b 0d 00 02 00 00 mov %gs:0x200,%ecx
...
16: 64 8b 35 10 02 00 00 mov %fs:0x210,%esi
...
28: 83 ea 01 sub $0x1,%edx
2b: 74 0a je 37 <a20_test_ref+0x37>
2d: e6 80 out %al,$0x80
2f: 89 d9 mov %ebx,%ecx
31: 29 d1 sub %edx,%ecx
33: 39 ce cmp %ecx,%esi
35: 74 f1 je 28 <a20_test_ref+0x28>
The solution with barrier() introduces memory clobber between store
and load, so the compiler is now forced to load and store the values
due to the side effects of the barrier() inbetween. This kind of
works, but is just a workaround for what really happens. In reality,
the value at the test address changes "behind the compiler back", IOW
- variable’s value can change in ways the compiler cannot predict.
My proposal is to use a volatile pointer to an absolute address, so
the unwanted optimizations are suppressed. The generated code is the
same as with barrier(), but now the code tells the compiler that every
read and write to this address must happen exactly as written in the
source code. Before your patch, the accessors were defined with
volatile asm, and this is the place where volatile qualifier matters.
So, my proposed code would read:
#define A20_TEST_ADDR (4*0x80)
#define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
#define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
static int a20_test(int loops)
{
int saved, ctr;
set_fs(0xffff);
saved = ctr = A20_TEST_GS;
do {
A20_TEST_GS = ++ctr;
io_delay(); /* Make constant delay */
if (A20_TEST_FS != ctr)
break;
} while (--loops);
A20_TEST_GS = saved;
return loops;
}
BR,
Uros.
^ permalink raw reply
* Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the real-mode boot code
From: Uros Bizjak @ 2026-01-21 9:40 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <CAFULd4bV6jKqHb-vC47skjNvbctit3VccXpF02oYt0icaq1r-Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4487 bytes --]
On Wed, Jan 21, 2026 at 9:56 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >
> > All supported versions of gcc support __seg_fs and __seg_gs now.
> > All supported versions of clang support __seg_fs and __seg_gs too,
> > except for two bugs (as of clang 21, at least):
> >
> > 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> > 2. An internal compiler error when addressing symbols directly.
> >
> > However, none of these are required in the boot code. Furthermore,
> > this makes it possible to remove the absolute_pointer() hack in the
> > fs/gs access functions.
> >
> > This requires adding a barrier() to a20.c, to prevent the compiler
> > from eliding the load from the aliased memory address.
> >
> > Remove the unused memcmp_[fg]s() functions.
> >
> > Finally, ds() is by necessity constant, so mark the function as such.
> >
> > Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> > ---
> > arch/x86/boot/a20.c | 1 +
> > arch/x86/boot/boot.h | 81 ++++++++++++++------------------------------
> > 2 files changed, 27 insertions(+), 55 deletions(-)
> >
> > diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> > index 3ab6cd8eaa31..52c3fccdcb70 100644
> > --- a/arch/x86/boot/a20.c
> > +++ b/arch/x86/boot/a20.c
> > @@ -63,6 +63,7 @@ static int a20_test(int loops)
> > while (loops--) {
> > wrgs32(++ctr, A20_TEST_ADDR);
> > io_delay(); /* Serialize and make delay constant */
> > + barrier(); /* Compiler won't know about fs/gs overlap */
> > ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> > if (ok)
> > break;
>
> This particular issue is not due to the compiler not knowing about
> fs/gs overlap (if the compiler determines that write and read are to
> the same non-volatile address, it would simply remove both, load and
> store), but due to the compiler performing load hoisting and store
> sinking from the loop. The compiler considers these two addresses as
> two different addresses (they are also defined in two different named
> address spaces), and optimizes access to them. So, without barrier(),
> it simply loads the value from A20_TEST_ADDR and A20_TEST_ADDR+0x10
> before the loop, resulting in:
>
> 9: 65 8b 0d 00 02 00 00 mov %gs:0x200,%ecx
> ...
> 16: 64 8b 35 10 02 00 00 mov %fs:0x210,%esi
> ...
> 28: 83 ea 01 sub $0x1,%edx
> 2b: 74 0a je 37 <a20_test_ref+0x37>
> 2d: e6 80 out %al,$0x80
> 2f: 89 d9 mov %ebx,%ecx
> 31: 29 d1 sub %edx,%ecx
> 33: 39 ce cmp %ecx,%esi
> 35: 74 f1 je 28 <a20_test_ref+0x28>
>
> The solution with barrier() introduces memory clobber between store
> and load, so the compiler is now forced to load and store the values
> due to the side effects of the barrier() inbetween. This kind of
> works, but is just a workaround for what really happens. In reality,
> the value at the test address changes "behind the compiler back", IOW
> - variable’s value can change in ways the compiler cannot predict.
>
> My proposal is to use a volatile pointer to an absolute address, so
> the unwanted optimizations are suppressed. The generated code is the
> same as with barrier(), but now the code tells the compiler that every
> read and write to this address must happen exactly as written in the
> source code. Before your patch, the accessors were defined with
> volatile asm, and this is the place where volatile qualifier matters.
> So, my proposed code would read:
>
> #define A20_TEST_ADDR (4*0x80)
>
> #define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
> #define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
>
> static int a20_test(int loops)
> {
> int saved, ctr;
>
> set_fs(0xffff);
>
> saved = ctr = A20_TEST_GS;
>
> do {
> A20_TEST_GS = ++ctr;
> io_delay(); /* Make constant delay */
> if (A20_TEST_FS != ctr)
> break;
> } while (--loops);
>
> A20_TEST_GS = saved;
> return loops;
> }
Now also in the form of the attached patch vs. yout git hpa/boot2
branch, tested with gcc-15.2.1 and clang-21.1.8.
BR,
Uros.
[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1239 bytes --]
diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
index 38a1cad8a553..ff51b0ce4dfd 100644
--- a/arch/x86/boot/a20.c
+++ b/arch/x86/boot/a20.c
@@ -48,8 +48,9 @@ static int empty_8042(void)
used as a test is the int $0x80 vector, which should be safe. */
#define A20_TEST_ADDR (4*0x80)
-#define A20_TEST_SHORT 32
-#define A20_TEST_LONG 2097152 /* 2^21 */
+
+#define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
+#define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
static int a20_test(int loops)
{
@@ -57,20 +58,22 @@ static int a20_test(int loops)
set_fs(0xffff);
- saved = ctr = rdgs32(A20_TEST_ADDR);
+ saved = ctr = A20_TEST_GS;
do {
- wrgs32(++ctr, A20_TEST_ADDR);
- io_delay(); /* Serialize and make delay constant */
- barrier(); /* Compiler won't know about fs/gs overlap */
- if (rdfs32(A20_TEST_ADDR+0x10) != ctr)
+ A20_TEST_GS = ++ctr;
+ io_delay(); /* Make constant delay */
+ if (A20_TEST_FS != ctr)
break;
} while (--loops);
- wrgs32(saved, A20_TEST_ADDR);
+ A20_TEST_GS = saved;
return loops;
}
+#define A20_TEST_SHORT 32
+#define A20_TEST_LONG 2097152 /* 2^21 */
+
/* Quick test to see if A20 is already enabled */
static int a20_test_short(void)
{
^ permalink raw reply related
* Re: [PATCH v1 02/14] x86/realmode: make %gs == 0 an invariant
From: Uros Bizjak @ 2026-01-21 10:03 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-3-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> When accessing data that is not "near", either only one segment is
> used or one segment is always zero. Leave %gs == 0 at all times
> throughout the C code; this reduces the number of segment loads
> needed.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/a20.c | 11 +++++------
> arch/x86/boot/header.S | 3 +++
> arch/x86/boot/regs.c | 3 ++-
> arch/x86/boot/video-bios.c | 5 ++---
> arch/x86/boot/video-mode.c | 5 ++---
> arch/x86/boot/video.c | 7 +++----
> arch/x86/realmode/rm/wakeup_asm.S | 10 ++++++----
> 7 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index bda042933a05..3ab6cd8eaa31 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -56,20 +56,19 @@ static int a20_test(int loops)
> int ok = 0;
> int saved, ctr;
>
> - set_fs(0x0000);
> - set_gs(0xffff);
> + set_fs(0xffff);
>
> - saved = ctr = rdfs32(A20_TEST_ADDR);
> + saved = ctr = rdgs32(A20_TEST_ADDR);
>
> while (loops--) {
> - wrfs32(++ctr, A20_TEST_ADDR);
> + wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> - ok = rdgs32(A20_TEST_ADDR+0x10) ^ ctr;
> + ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> if (ok)
> break;
> }
>
> - wrfs32(saved, A20_TEST_ADDR);
> + wrgs32(saved, A20_TEST_ADDR);
> return ok;
> }
>
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index 9bea5a1e2c52..bda20395658f 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -596,6 +596,9 @@ start_of_setup:
> shrw $2, %cx
> rep stosl
>
> +# The C code uses %gs == 0 as invariant
> + movw %ax, %gs
> +
> # Jump to C code (should not return)
> calll main
>
> diff --git a/arch/x86/boot/regs.c b/arch/x86/boot/regs.c
> index 55de6b3092b8..54d6dfd129c5 100644
> --- a/arch/x86/boot/regs.c
> +++ b/arch/x86/boot/regs.c
> @@ -22,6 +22,7 @@ void initregs(struct biosregs *reg)
> reg->eflags |= X86_EFLAGS_CF;
> reg->ds = ds();
> reg->es = ds();
> + /* The input values of %cs and %ss are ignored by intcall() */
> reg->fs = fs();
> - reg->gs = gs();
> + /* %gs == 0 */
> }
> diff --git a/arch/x86/boot/video-bios.c b/arch/x86/boot/video-bios.c
> index 6eb8c06bc287..e8be64424a40 100644
> --- a/arch/x86/boot/video-bios.c
> +++ b/arch/x86/boot/video-bios.c
> @@ -73,7 +73,6 @@ static int bios_probe(void)
> if (adapter != ADAPTER_EGA && adapter != ADAPTER_VGA)
> return 0;
>
> - set_fs(0);
> crtc = vga_crtc();
>
> video_bios.modes = GET_HEAP(struct mode_info, 0);
> @@ -105,8 +104,8 @@ static int bios_probe(void)
> mi = GET_HEAP(struct mode_info, 1);
> mi->mode = VIDEO_FIRST_BIOS+mode;
> mi->depth = 0; /* text */
> - mi->x = rdfs16(0x44a);
> - mi->y = rdfs8(0x484)+1;
> + mi->x = rdgs16(0x44a);
> + mi->y = rdgs8(0x484)+1;
> nmodes++;
> }
>
> diff --git a/arch/x86/boot/video-mode.c b/arch/x86/boot/video-mode.c
> index 9ada55dc1ab7..e5b9bc96bd42 100644
> --- a/arch/x86/boot/video-mode.c
> +++ b/arch/x86/boot/video-mode.c
> @@ -119,9 +119,8 @@ static void vga_recalc_vertical(void)
> u16 crtc;
> u8 pt, ov;
>
> - set_fs(0);
> - font_size = rdfs8(0x485); /* BIOS: font size (pixels) */
> - rows = force_y ? force_y : rdfs8(0x484)+1; /* Text rows */
> + font_size = rdgs8(0x485); /* BIOS: font size (pixels) */
> + rows = force_y ? force_y : rdgs8(0x484)+1; /* Text rows */
>
> rows *= font_size; /* Visible scan lines */
> rows--; /* ... minus one */
> diff --git a/arch/x86/boot/video.c b/arch/x86/boot/video.c
> index 0641c8c46aee..09b810faa5c0 100644
> --- a/arch/x86/boot/video.c
> +++ b/arch/x86/boot/video.c
> @@ -79,12 +79,11 @@ static void store_mode_params(void)
> video_segment = 0xb800;
> }
>
> - set_fs(0);
> - font_size = rdfs16(0x485); /* Font size, BIOS area */
> + font_size = rdgs16(0x485); /* Font size, BIOS area */
> boot_params.screen_info.orig_video_points = font_size;
>
> - x = rdfs16(0x44a);
> - y = (adapter == ADAPTER_CGA) ? 25 : rdfs8(0x484)+1;
> + x = rdgs16(0x44a);
> + y = (adapter == ADAPTER_CGA) ? 25 : rdgs8(0x484)+1;
>
> if (force_x)
> x = force_x;
> diff --git a/arch/x86/realmode/rm/wakeup_asm.S b/arch/x86/realmode/rm/wakeup_asm.S
> index 02d0ba16ae33..a8a8580158d7 100644
> --- a/arch/x86/realmode/rm/wakeup_asm.S
> +++ b/arch/x86/realmode/rm/wakeup_asm.S
> @@ -70,15 +70,17 @@ SYM_CODE_START(wakeup_start)
> movl $rm_stack_end, %esp
> movw %ax, %ds
> movw %ax, %es
> - movw %ax, %fs
> - movw %ax, %gs
>
> - lidtl .Lwakeup_idt
> + xorl %eax, %eax
> + movw %ax, %fs
> + movw %ax, %gs /* The real mode code requires %gs == 0 */
>
> /* Clear the EFLAGS */
> - pushl $0
> + pushl %eax
> popfl
>
> + lidtl .Lwakeup_idt
> +
> /* Check header signature... */
> movl signature, %eax
> cmpl $WAKEUP_HEADER_SIGNATURE, %eax
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 03/14] x86/boot: use <linux/compiler.h>
From: Uros Bizjak @ 2026-01-21 10:04 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-4-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> Include <linux/compiler.h> in arch/x86/boot/boot.h and replace
> __attribute__((noreturn)) with __noreturn.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/boot.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index f185931283cb..1f2b7252e387 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -18,6 +18,7 @@
>
> #ifndef __ASSEMBLER__
>
> +#include <linux/compiler.h>
> #include <asm/shared/io.h>
> #include <linux/stdarg.h>
> #include <linux/types.h>
> @@ -279,17 +280,16 @@ void console_init(void);
> void query_edd(void);
>
> /* header.S */
> -void __attribute__((noreturn)) die(void);
> +void __noreturn die(void);
>
> /* memory.c */
> void detect_memory(void);
>
> /* pm.c */
> -void __attribute__((noreturn)) go_to_protected_mode(void);
> +void __noreturn go_to_protected_mode(void);
>
> /* pmjump.S */
> -void __attribute__((noreturn))
> - protected_mode_jump(u32 entrypoint, u32 bootparams);
> +void __noreturn protected_mode_jump(u32 entrypoint, u32 bootparams);
>
> /* printf.c */
> int sprintf(char *buf, const char *fmt, ...);
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 05/14] x86/boot: call puts() from within die()
From: Uros Bizjak @ 2026-01-21 10:07 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-6-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> Every call to die() has a call to puts() in front of it, which adds
> unnecessary code bloat. Move the puts() into die() which, being a
> noreturn function, is terminal and allows the compiler to not generate
> unnecessary code around it.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/boot.h | 2 +-
> arch/x86/boot/header.S | 5 +++--
> arch/x86/boot/main.c | 6 ++----
> arch/x86/boot/pm.c | 6 ++----
> 4 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 1f2b7252e387..b4eb8405ba55 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -280,7 +280,7 @@ void console_init(void);
> void query_edd(void);
>
> /* header.S */
> -void __noreturn die(void);
> +void __noreturn die(const char *msg);
>
> /* memory.c */
> void detect_memory(void);
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index 85a21d576f5b..5a54d33e51c2 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -611,14 +611,15 @@ SETUP_SIGNATURE = 0x5a5aaa55
> # Setup corrupt somehow...
> setup_bad:
> movl $setup_corrupt, %eax
> - calll puts
> # Fall through...
>
> .globl die
> .type die, @function
> die:
> + calll puts
> +1:
> hlt
> - jmp die
> + jmp 1b
>
> .size die, .-die
>
> diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
> index da01ade4959e..ad8869aad6db 100644
> --- a/arch/x86/boot/main.c
> +++ b/arch/x86/boot/main.c
> @@ -143,10 +143,8 @@ void main(void)
> init_heap();
>
> /* Make sure we have all the proper CPU support */
> - if (validate_cpu()) {
> - puts("Unable to boot - please use a kernel appropriate for your CPU.\n");
> - die();
> - }
> + if (validate_cpu())
> + die("Unable to boot - please use a kernel appropriate for your CPU.\n");
>
> /* Tell the BIOS what CPU mode we intend to run in */
> set_bios_mode();
> diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
> index 5941f930f6c5..3be89ba4b1b3 100644
> --- a/arch/x86/boot/pm.c
> +++ b/arch/x86/boot/pm.c
> @@ -106,10 +106,8 @@ void go_to_protected_mode(void)
> realmode_switch_hook();
>
> /* Enable the A20 gate */
> - if (enable_a20()) {
> - puts("A20 gate not responding, unable to boot...\n");
> - die();
> - }
> + if (enable_a20())
> + die("A20 gate not responding, unable to boot...\n");
>
> /* Reset coprocessor (IGNNE#) */
> reset_coprocessor();
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 07/14] x86/boot: factor out the 16-bit startup code from header.S
From: Uros Bizjak @ 2026-01-21 10:08 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-8-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> Move the 16-bit startup code to its own assembly file, instead of
> mixing it into header.S. header.S is now a pure data file.
>
> This also means the .code16 directive is no longer needed for this
> file.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/Makefile | 4 +-
> arch/x86/boot/header.S | 91 +--------------------------------
> arch/x86/boot/start16.S | 108 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 112 insertions(+), 91 deletions(-)
> create mode 100644 arch/x86/boot/start16.S
>
> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> index 3f9fb3698d66..d7944bf196b9 100644
> --- a/arch/x86/boot/Makefile
> +++ b/arch/x86/boot/Makefile
> @@ -22,8 +22,8 @@ subdir- := compressed
>
> setup-y += a20.o bioscall.o cmdline.o copy.o cpu.o cpuflags.o cpucheck.o
> setup-y += early_serial_console.o edd.o header.o main.o memory.o
> -setup-y += pm.o pmjump.o printf.o regs.o string.o tty.o video.o
> -setup-y += video-mode.o version.o
> +setup-y += pm.o pmjump.o printf.o regs.o start16.o string.o tty.o
> +setup-y += video.o video-mode.o version.o
> setup-$(CONFIG_X86_APM_BOOT) += apm.o
>
> # The link order of the video-*.o modules can matter. In particular,
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index d74db02928e6..10b2971320f3 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -25,7 +25,6 @@
> #include "voffset.h"
> #include "zoffset.h"
>
> -BOOTSEG = 0x07C0 /* original address of boot-sector */
> SYSSEG = 0x1000 /* historical load address >> 4 */
>
> #ifndef SVGA_MODE
> @@ -39,8 +38,6 @@ SYSSEG = 0x1000 /* historical load address >> 4 */
> .set salign, 0x1000
> .set falign, 0x200
>
> - .code16
> -
> # EFI PECOFF header ##########################################################
>
> .section ".header", "a"
> @@ -277,6 +274,7 @@ type_of_loader: .byte 0 # 0 means ancient bootloader, newer
> # assigned ids
>
> # flags, unused bits must be zero (RFU) bit within loadflags
> + .globl loadflags
> loadflags:
> .byte LOADED_HIGH # The kernel is to be loaded high
>
> @@ -301,6 +299,7 @@ ramdisk_size: .long 0 # its size in bytes
> bootsect_kludge:
> .long 0 # obsolete
>
> + .globl heap_end_ptr
> heap_end_ptr: .word _end+STACK_SIZE-512
> # (Header version 0x0201 or later)
> # space from here (exclusive) down to
> @@ -551,89 +550,3 @@ kernel_info_offset: .long ZO_kernel_info
> end_of_bzheader:
>
> # End of bzImage header ######################################################
> -
> - .section ".entrytext", "ax"
> - .globl start_of_setup
> -start_of_setup:
> -# Force %es = %ds
> - movw %ds, %ax
> - movw %ax, %es
> - cld
> -
> -# Apparently some ancient versions of LILO invoked the kernel with %ss != %ds,
> -# which happened to work by accident for the old code. Recalculate the stack
> -# pointer if %ss is invalid. Otherwise leave it alone, LOADLIN sets up the
> -# stack behind its own code, so we can't blindly put it directly past the heap.
> -
> - movw %ss, %dx
> - cmpw %ax, %dx # %ds == %ss?
> - movw %sp, %dx
> - je 2f # -> assume %sp is reasonably set
> -
> - # Invalid %ss, make up a new stack
> - movw $_end, %dx
> - testb $CAN_USE_HEAP, loadflags
> - jz 1f
> - movw heap_end_ptr, %dx
> -1: addw $STACK_SIZE, %dx
> - jnc 2f
> - xorw %dx, %dx # Prevent wraparound
> -
> -2: # Now %dx should point to the end of our stack space
> - andw $~3, %dx # dword align (might as well...)
> - jnz 3f
> - movw $0xfffc, %dx # Make sure we're not zero
> -3: movw %ax, %ss
> - movzwl %dx, %esp # Clear upper half of %esp
> - sti # Now we should have a working stack
> -
> -# We will have entered with %cs = %ds+0x20, normalize %cs so
> -# it is on par with the other segments.
> - pushw %ds
> - pushw $6f
> - lretw
> -6:
> -
> -# Check signature at end of setup
> -SETUP_SIGNATURE = 0x5a5aaa55
> - cmpl $SETUP_SIGNATURE, setup_sig
> - jne setup_bad
> -
> -# Zero the bss
> - movw $__bss_start, %di
> - movw $_end+3, %cx
> - xorl %eax, %eax
> - subw %di, %cx
> - shrw $2, %cx
> - rep stosl
> -
> -# The C code uses %gs == 0 as invariant
> - movw %ax, %gs
> -
> -# Jump to C code (should not return)
> - calll main
> -
> -# Setup corrupt somehow...
> -setup_bad:
> - movl $setup_corrupt, %eax
> - # Fall through...
> -
> - .globl die
> - .type die, @function
> -die:
> - calll puts
> -1:
> - hlt
> - jmp 1b
> -
> - .size die, .-die
> -
> - .section ".initdata", "a"
> -setup_corrupt:
> - .byte 7
> - .string "No setup signature found...\n"
> -
> - .section ".signature", "a"
> - .balign 4
> -setup_sig:
> - .long SETUP_SIGNATURE
> diff --git a/arch/x86/boot/start16.S b/arch/x86/boot/start16.S
> new file mode 100644
> index 000000000000..3381dc0f4065
> --- /dev/null
> +++ b/arch/x86/boot/start16.S
> @@ -0,0 +1,108 @@
> +/*
> + * start16.S
> + *
> + * Copyright (C) 1991, 1992 Linus Torvalds
> + *
> + * Based on bootsect.S and setup.S
> + * modified by more people than can be counted
> + *
> + * Rewritten as a common file by H. Peter Anvin (Apr 2007)
> + *
> + * BIG FAT NOTE: We're in real mode using 64k segments. Therefore segment
> + * addresses must be multiplied by 16 to obtain their respective linear
> + * addresses. To avoid confusion, linear addresses are written using leading
> + * hex while segment addresses are written as segment:offset.
> + *
> + * This code must *immediately* follow the bzImage header, so DO NOT
> + * add alignment directives anywhere in the .entrytext section!
> + */
> +
> +#include <asm/bootparam.h>
> +#include "boot.h"
> +
> + .code16
> + .section ".entrytext", "ax"
> + .globl start_of_setup
> +start_of_setup:
> +# Force %es = %ds
> + movw %ds, %ax
> + movw %ax, %es
> + cld
> +
> +# Apparently some ancient versions of LILO invoked the kernel with %ss != %ds,
> +# which happened to work by accident for the old code. Recalculate the stack
> +# pointer if %ss is invalid. Otherwise leave it alone, LOADLIN sets up the
> +# stack behind its own code, so we can't blindly put it directly past the heap.
> +
> + movw %ss, %dx
> + cmpw %ax, %dx # %ds == %ss?
> + movw %sp, %dx
> + je 2f # -> assume %sp is reasonably set
> +
> + # Invalid %ss, make up a new stack
> + movw $_end, %dx
> + testb $CAN_USE_HEAP, loadflags
> + jz 1f
> + movw heap_end_ptr, %dx
> +1: addw $STACK_SIZE, %dx
> + jnc 2f
> + xorw %dx, %dx # Prevent wraparound
> +
> +2: # Now %dx should point to the end of our stack space
> + andw $~3, %dx # dword align (might as well...)
> + jnz 3f
> + movw $0xfffc, %dx # Make sure we're not zero
> +3: movw %ax, %ss
> + movzwl %dx, %esp # Clear upper half of %esp
> + sti # Now we should have a working stack
> +
> +# We will have entered with %cs = %ds+0x20, normalize %cs so
> +# it is on par with the other segments.
> + pushw %ds
> + pushw $6f
> + lretw
> +6:
> +
> +# Check signature at end of setup
> +SETUP_SIGNATURE = 0x5a5aaa55
> + cmpl $SETUP_SIGNATURE, setup_sig
> + jne setup_bad
> +
> +# Zero the bss
> + movw $__bss_start, %di
> + movw $_end+3, %cx
> + xorl %eax, %eax
> + subw %di, %cx
> + shrw $2, %cx
> + rep stosl
> +
> +# The C code uses %gs == 0 as invariant
> + movw %ax, %gs
> +
> +# Jump to C code (should not return)
> + calll main
> +
> +# Setup corrupt somehow...
> +setup_bad:
> + movl $setup_corrupt, %eax
> + # Fall through...
> +
> + .globl die
> + .type die, @function
> +die:
> + calll puts
> +1:
> + hlt
> + jmp 1b
> +
> + .size die, .-die
> +
> + .section ".initdata", "a"
> +setup_corrupt:
> + .byte 7
> + .string "No setup signature found...\n"
> +
> + .section ".signature", "a"
> + .balign 4
> +setup_sig:
> + .long SETUP_SIGNATURE
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 12/14] x86/boot: tweak a20.c for better code generation
From: Uros Bizjak @ 2026-01-21 10:10 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-13-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> Do some minor tweaks to arch/x86/boot/a20.c for better code
> generation, made possible by the __seg_fs/__seg_gs changes.
>
> Move the die() call to a20.c itself; there is no reason to push an
> error code upwards just to die() there.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Acked-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/a20.c | 24 +++++++++++-------------
> arch/x86/boot/boot.h | 2 +-
> arch/x86/boot/pm.c | 3 +--
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 52c3fccdcb70..38a1cad8a553 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -53,24 +53,22 @@ static int empty_8042(void)
>
> static int a20_test(int loops)
> {
> - int ok = 0;
> int saved, ctr;
>
> set_fs(0xffff);
>
> saved = ctr = rdgs32(A20_TEST_ADDR);
>
> - while (loops--) {
> + do {
> wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> barrier(); /* Compiler won't know about fs/gs overlap */
> - ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> - if (ok)
> + if (rdfs32(A20_TEST_ADDR+0x10) != ctr)
> break;
> - }
> + } while (--loops);
>
> wrgs32(saved, A20_TEST_ADDR);
> - return ok;
> + return loops;
> }
>
> /* Quick test to see if A20 is already enabled */
> @@ -125,7 +123,7 @@ static void enable_a20_fast(void)
>
> #define A20_ENABLE_LOOPS 255 /* Number of times to try */
>
> -int enable_a20(void)
> +void enable_a20(void)
> {
> int loops = A20_ENABLE_LOOPS;
> int kbc_err;
> @@ -134,30 +132,30 @@ int enable_a20(void)
> /* First, check to see if A20 is already enabled
> (legacy free, etc.) */
> if (a20_test_short())
> - return 0;
> + return;
>
> /* Next, try the BIOS (INT 0x15, AX=0x2401) */
> enable_a20_bios();
> if (a20_test_short())
> - return 0;
> + return;
>
> /* Try enabling A20 through the keyboard controller */
> kbc_err = empty_8042();
>
> if (a20_test_short())
> - return 0; /* BIOS worked, but with delayed reaction */
> + return; /* BIOS worked, but with delayed reaction */
>
> if (!kbc_err) {
> enable_a20_kbc();
> if (a20_test_long())
> - return 0;
> + return;
> }
>
> /* Finally, try enabling the "fast A20 gate" */
> enable_a20_fast();
> if (a20_test_long())
> - return 0;
> + return;
> }
>
> - return -1;
> + die("A20 gate not responding, unable to boot...\n");
> }
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 4d3549ed7987..584c89d0738b 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -167,7 +167,7 @@ void copy_to_fs(addr_t dst, void *src, size_t len);
> void *copy_from_fs(void *dst, addr_t src, size_t len);
>
> /* a20.c */
> -int enable_a20(void);
> +void enable_a20(void);
>
> /* apm.c */
> int query_apm_bios(void);
> diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
> index 3be89ba4b1b3..e39689ed65ea 100644
> --- a/arch/x86/boot/pm.c
> +++ b/arch/x86/boot/pm.c
> @@ -106,8 +106,7 @@ void go_to_protected_mode(void)
> realmode_switch_hook();
>
> /* Enable the A20 gate */
> - if (enable_a20())
> - die("A20 gate not responding, unable to boot...\n");
> + enable_a20();
>
> /* Reset coprocessor (IGNNE#) */
> reset_coprocessor();
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 13/14] x86/boot: simplify x86/boot/cmdline.c by using __seg_fs
From: Uros Bizjak @ 2026-01-21 10:11 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-14-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> arch/x86/boot/cmdline.c is compiled in 16, 32, or 64-bit mode. In the
> 16-bit case it needs to access an out-of-data-segment pointer.
>
> This has been handled in the past by using a *really* ugly wrapper in
> the 32/64-bit code combined with an inline function in the 16-bit code.
>
> Using __seg_fs for the real-mode case allows for a much cleaner way of
> handling this difference. Futhermore, moving the code for typing and
> obtaining the pointer into cmdline.c itself avoids having to create
> and push an extra argument, which is particularly inefficient in the
> real-mode code as it requires a range check and pushes the argument
> count past 3. Furthermore, co-locating this code with the user makes it
> a lot clearer what the constraints are on this code, even though it
> means using #ifdef _SETUP.
>
> Instead of limit-checking the command line to the real-mode segment
> limit, limit-check it to COMMAND_LINE_SIZE (which is always less than
> 64K, and thus automatically incorporates the real-mode segment limit
> check.)
>
> If compiling for 32 bits, trying to add ext_cmd_line_ptr in
> get_cmd_line_ptr() is futile as unsigned long, and pointers, are only
> 32 bits wide. Instead, limit-check the command line pointer and fail
> if it is >= 4 GiB, just as the real-mode code fails if the pointer is
> >= 1 MiB.
>
> Since the kaslr code depends on get_cmd_line_ptr(), retain it as a
> global function.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Acked-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/boot.h | 23 +--------
> arch/x86/boot/cmdline.c | 79 +++++++++++++++++++++++-------
> arch/x86/boot/compressed/cmdline.c | 29 -----------
> 3 files changed, 64 insertions(+), 67 deletions(-)
>
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 584c89d0738b..8512db8b3f8e 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -216,27 +216,8 @@ struct biosregs {
> void intcall(u8 int_no, const struct biosregs *ireg, struct biosregs *oreg);
>
> /* cmdline.c */
> -int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize);
> -int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option);
> -static inline int cmdline_find_option(const char *option, char *buffer, int bufsize)
> -{
> - unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
> -
> - if (cmd_line_ptr >= 0x100000)
> - return -1; /* inaccessible */
> -
> - return __cmdline_find_option(cmd_line_ptr, option, buffer, bufsize);
> -}
> -
> -static inline int cmdline_find_option_bool(const char *option)
> -{
> - unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
> -
> - if (cmd_line_ptr >= 0x100000)
> - return -1; /* inaccessible */
> -
> - return __cmdline_find_option_bool(cmd_line_ptr, option);
> -}
> +int cmdline_find_option(const char *option, char *buffer, int bufsize);
> +int cmdline_find_option_bool(const char *option);
>
> /* cpu.c, cpucheck.c */
> int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
> diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
> index 21d56ae83cdf..3f31fcbed673 100644
> --- a/arch/x86/boot/cmdline.c
> +++ b/arch/x86/boot/cmdline.c
> @@ -11,12 +11,59 @@
> */
>
> #include "boot.h"
> +#include <asm/setup.h>
> +#include <asm/bootparam.h>
>
> static inline int myisspace(u8 c)
> {
> return c <= ' '; /* Close enough approximation */
> }
>
> +#ifdef _SETUP
> +typedef const char __seg_fs *cptr_t;
> +static inline cptr_t get_cptr(void)
> +{
> + /*
> + * Note: there is no reason to check ext_cmd_line_ptr here,
> + * because it falls outside of boot_params.hdr and therefore
> + * will always be zero when entering through the real-mode
> + * entry point.
> + */
> + unsigned long ptr = boot_params.hdr.cmd_line_ptr;
> +
> + /*
> + * The -16 here serves two purposes:
> + * 1. It means the segbase >= 0x100000 check also doubles as
> + * a check for the command line pointer being zero.
> + * 2. It means this routine won't return a NULL pointer for
> + * a valid address; it will always return a pointer in the
> + * range 0x10-0x1f inclusive.
> + */
> + unsigned long segbase = (ptr - 16) & ~15;
> + if (segbase >= 0x100000)
> + return NULL;
> +
> + set_fs(segbase >> 4);
> + return (cptr_t)(ptr - segbase);
> +}
> +#else
> +unsigned long get_cmd_line_ptr(void)
> +{
> + unsigned long ptr = boot_params_ptr->hdr.cmd_line_ptr;
> + if (sizeof(unsigned long) > 4)
> + ptr += (u64)boot_params_ptr->ext_cmd_line_ptr << 32;
> + else if (boot_params_ptr->ext_cmd_line_ptr)
> + return 0; /* Inaccessible due to pointer overflow */
> +
> + return ptr;
> +}
> +typedef const char *cptr_t;
> +static inline cptr_t get_cptr(void)
> +{
> + return (cptr_t)get_cmd_line_ptr();
> +}
> +#endif
> +
> /*
> * Find a non-boolean option, that is, "option=argument". In accordance
> * with standard Linux practice, if this option is repeated, this returns
> @@ -25,9 +72,9 @@ static inline int myisspace(u8 c)
> * Returns the length of the argument (regardless of if it was
> * truncated to fit in the buffer), or -1 on not found.
> */
> -int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize)
> +int cmdline_find_option(const char *option, char *buffer, int bufsize)
> {
> - addr_t cptr;
> + cptr_t cptr, eptr;
> char c;
> int len = -1;
> const char *opptr = NULL;
> @@ -39,13 +86,12 @@ int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *b
> st_bufcpy /* Copying this to buffer */
> } state = st_wordstart;
>
> - if (!cmdline_ptr)
> - return -1; /* No command line */
> + cptr = get_cptr();
> + if (!cptr)
> + return -1; /* No command line or invalid pointer */
> + eptr = cptr + COMMAND_LINE_SIZE - 1;
>
> - cptr = cmdline_ptr & 0xf;
> - set_fs(cmdline_ptr >> 4);
> -
> - while (cptr < 0x10000 && (c = rdfs8(cptr++))) {
> + while (cptr < eptr && (c = *cptr++)) {
> switch (state) {
> case st_wordstart:
> if (myisspace(c))
> @@ -97,9 +143,9 @@ int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *b
> * Returns the position of that option (starts counting with 1)
> * or 0 on not found
> */
> -int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)
> +int cmdline_find_option_bool(const char *option)
> {
> - addr_t cptr;
> + cptr_t cptr, eptr;
> char c;
> int pos = 0, wstart = 0;
> const char *opptr = NULL;
> @@ -109,14 +155,13 @@ int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)
> st_wordskip, /* Miscompare, skip */
> } state = st_wordstart;
>
> - if (!cmdline_ptr)
> - return -1; /* No command line */
> -
> - cptr = cmdline_ptr & 0xf;
> - set_fs(cmdline_ptr >> 4);
> + cptr = get_cptr();
> + if (!cptr)
> + return -1; /* No command line or invalid pointer */
> + eptr = cptr + COMMAND_LINE_SIZE - 1;
>
> - while (cptr < 0x10000) {
> - c = rdfs8(cptr++);
> + while (cptr <= eptr) {
> + c = *cptr++;
> pos++;
>
> switch (state) {
> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
> index e162d7f59cc5..0d9267a21012 100644
> --- a/arch/x86/boot/compressed/cmdline.c
> +++ b/arch/x86/boot/compressed/cmdline.c
> @@ -1,32 +1,3 @@
> // SPDX-License-Identifier: GPL-2.0
> #include "misc.h"
> -
> -#include <asm/bootparam.h>
> -
> -static unsigned long fs;
> -static inline void set_fs(unsigned long seg)
> -{
> - fs = seg << 4; /* shift it back */
> -}
> -typedef unsigned long addr_t;
> -static inline char rdfs8(addr_t addr)
> -{
> - return *((char *)(fs + addr));
> -}
> #include "../cmdline.c"
> -unsigned long get_cmd_line_ptr(void)
> -{
> - unsigned long cmd_line_ptr = boot_params_ptr->hdr.cmd_line_ptr;
> -
> - cmd_line_ptr |= (u64)boot_params_ptr->ext_cmd_line_ptr << 32;
> -
> - return cmd_line_ptr;
> -}
> -int cmdline_find_option(const char *option, char *buffer, int bufsize)
> -{
> - return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize);
> -}
> -int cmdline_find_option_bool(const char *option)
> -{
> - return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
> -}
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1 00/14] x86 boot code cleanups, batch 1
From: Uros Bizjak @ 2026-01-21 10:18 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-1-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
> This patchset also includes the removal of the gcc-specific version of
> the RELOC_HIDE() macro, by Uros Bizjak, as the other changes in this
> patchset makes that possible. Per Uros, this is holding back gcc
> development, so it seems pretty important.
Not that this issue is holding back GCC *development*, the historic
kludge unnecessarily suppresses optimizations that could be applied by
the compiler to produce better code.
Thanks,
Uros.
^ permalink raw reply
* Re: [PATCH v1 12/14] x86/boot: tweak a20.c for better code generation
From: David Laight @ 2026-01-21 11:49 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Uros Bizjak, Petr Mladek, Andrew Morton, Kees Cook,
Peter Zijlstra (Intel), Nathan Chancellor, Kiryl Shutsemau,
Rick Edgecombe, linux-kernel, linux-coco, x86
In-Reply-To: <20260120195407.1163051-13-hpa@zytor.com>
On Tue, 20 Jan 2026 11:54:04 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:
> Do some minor tweaks to arch/x86/boot/a20.c for better code
> generation, made possible by the __seg_fs/__seg_gs changes.
>
> Move the die() call to a20.c itself; there is no reason to push an
> error code upwards just to die() there.
Can't this code just do:
sv = *0000:addr;
val = *ffff:addr+10;
if (sv != val)
return 1;
*0000:addr = ~val;
io_delay();
val ^= *ffff:addr+10;
*0000:addr = sv;
return val != ~0;
That inverts all bits, inverting one is enough.
No loops needed.
David
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> ---
> arch/x86/boot/a20.c | 24 +++++++++++-------------
> arch/x86/boot/boot.h | 2 +-
> arch/x86/boot/pm.c | 3 +--
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 52c3fccdcb70..38a1cad8a553 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -53,24 +53,22 @@ static int empty_8042(void)
>
> static int a20_test(int loops)
> {
> - int ok = 0;
> int saved, ctr;
>
> set_fs(0xffff);
>
> saved = ctr = rdgs32(A20_TEST_ADDR);
>
> - while (loops--) {
> + do {
> wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> barrier(); /* Compiler won't know about fs/gs overlap */
> - ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> - if (ok)
> + if (rdfs32(A20_TEST_ADDR+0x10) != ctr)
> break;
> - }
> + } while (--loops);
>
> wrgs32(saved, A20_TEST_ADDR);
> - return ok;
> + return loops;
> }
>
> /* Quick test to see if A20 is already enabled */
> @@ -125,7 +123,7 @@ static void enable_a20_fast(void)
>
> #define A20_ENABLE_LOOPS 255 /* Number of times to try */
>
> -int enable_a20(void)
> +void enable_a20(void)
> {
> int loops = A20_ENABLE_LOOPS;
> int kbc_err;
> @@ -134,30 +132,30 @@ int enable_a20(void)
> /* First, check to see if A20 is already enabled
> (legacy free, etc.) */
> if (a20_test_short())
> - return 0;
> + return;
>
> /* Next, try the BIOS (INT 0x15, AX=0x2401) */
> enable_a20_bios();
> if (a20_test_short())
> - return 0;
> + return;
>
> /* Try enabling A20 through the keyboard controller */
> kbc_err = empty_8042();
>
> if (a20_test_short())
> - return 0; /* BIOS worked, but with delayed reaction */
> + return; /* BIOS worked, but with delayed reaction */
>
> if (!kbc_err) {
> enable_a20_kbc();
> if (a20_test_long())
> - return 0;
> + return;
> }
>
> /* Finally, try enabling the "fast A20 gate" */
> enable_a20_fast();
> if (a20_test_long())
> - return 0;
> + return;
> }
>
> - return -1;
> + die("A20 gate not responding, unable to boot...\n");
> }
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 4d3549ed7987..584c89d0738b 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -167,7 +167,7 @@ void copy_to_fs(addr_t dst, void *src, size_t len);
> void *copy_from_fs(void *dst, addr_t src, size_t len);
>
> /* a20.c */
> -int enable_a20(void);
> +void enable_a20(void);
>
> /* apm.c */
> int query_apm_bios(void);
> diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
> index 3be89ba4b1b3..e39689ed65ea 100644
> --- a/arch/x86/boot/pm.c
> +++ b/arch/x86/boot/pm.c
> @@ -106,8 +106,7 @@ void go_to_protected_mode(void)
> realmode_switch_hook();
>
> /* Enable the A20 gate */
> - if (enable_a20())
> - die("A20 gate not responding, unable to boot...\n");
> + enable_a20();
>
> /* Reset coprocessor (IGNNE#) */
> reset_coprocessor();
^ permalink raw reply
* Re: [PATCH v1 01/14] x86/realmode: remove I/O port paravirtualization
From: Kiryl Shutsemau @ 2026-01-21 12:27 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Uros Bizjak, Petr Mladek, Andrew Morton, Kees Cook,
Peter Zijlstra (Intel), Nathan Chancellor, Rick Edgecombe,
linux-kernel, linux-coco, x86
In-Reply-To: <20260120195407.1163051-2-hpa@zytor.com>
On Tue, Jan 20, 2026 at 11:53:53AM -0800, H. Peter Anvin wrote:
> In commit:
>
> eb4ea1ae8f45 x86/boot: Port I/O: Allow to hook up alternative helpers
>
> ... paravirtualization hooks were added to (some!) of the port I/O
> functions. However, they were only ever used in the 32/64-bit
> "compressed" directory, and never made any sense in the real-mode
> code, which is notoriously size sensitive.
>
> Without these hooks, <asm/shared/io.h> is usable directly, so mode
> io.h into the compressed/ directory and replace "io.h" with
> <asm/shared/io.h> for the actual real-mode code.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Acked-by: Kiryl Shutsemau <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH] x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
From: Kiryl Shutsemau @ 2026-01-21 12:32 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Paolo Bonzini, linux-kernel, linux-coco, kvm, Kai Huang,
Rick Edgecombe, Yan Zhao, Vishal Annapurve, Ackerley Tng
In-Reply-To: <20260120203937.1447592-1-seanjc@google.com>
On Tue, Jan 20, 2026 at 12:39:37PM -0800, Sean Christopherson wrote:
> Rework the TDX APIs to take the kernel's 1-based pg_level enum, not the
> TDX-Module's 0-based level. The APIs are _kernel_ APIs, not TDX-Module
> APIs, and the kernel (and KVM) uses "enum pg_level" literally everywhere.
>
> Using "enum pg_level" eliminates ambiguity when looking at the APIs (it's
> NOT clear that "int level" refers to the TDX-Module's level), and will
> allow for using existing helpers like page_level_size() when support for
> hugepages is added to the S-EPT APIs.
>
> No functional change intended.
>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Vishal Annapurve <vannapurve@google.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Acked-by: Kiryl Shutsemau <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the real-mode boot code
From: H. Peter Anvin @ 2026-01-21 15:13 UTC (permalink / raw)
To: Uros Bizjak
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <CAFULd4bV6jKqHb-vC47skjNvbctit3VccXpF02oYt0icaq1r-Q@mail.gmail.com>
On January 21, 2026 12:56:39 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> All supported versions of gcc support __seg_fs and __seg_gs now.
>> All supported versions of clang support __seg_fs and __seg_gs too,
>> except for two bugs (as of clang 21, at least):
>>
>> 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
>> 2. An internal compiler error when addressing symbols directly.
>>
>> However, none of these are required in the boot code. Furthermore,
>> this makes it possible to remove the absolute_pointer() hack in the
>> fs/gs access functions.
>>
>> This requires adding a barrier() to a20.c, to prevent the compiler
>> from eliding the load from the aliased memory address.
>>
>> Remove the unused memcmp_[fg]s() functions.
>>
>> Finally, ds() is by necessity constant, so mark the function as such.
>>
>> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
>> ---
>> arch/x86/boot/a20.c | 1 +
>> arch/x86/boot/boot.h | 81 ++++++++++++++------------------------------
>> 2 files changed, 27 insertions(+), 55 deletions(-)
>>
>> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
>> index 3ab6cd8eaa31..52c3fccdcb70 100644
>> --- a/arch/x86/boot/a20.c
>> +++ b/arch/x86/boot/a20.c
>> @@ -63,6 +63,7 @@ static int a20_test(int loops)
>> while (loops--) {
>> wrgs32(++ctr, A20_TEST_ADDR);
>> io_delay(); /* Serialize and make delay constant */
>> + barrier(); /* Compiler won't know about fs/gs overlap */
>> ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
>> if (ok)
>> break;
>
>This particular issue is not due to the compiler not knowing about
>fs/gs overlap (if the compiler determines that write and read are to
>the same non-volatile address, it would simply remove both, load and
>store), but due to the compiler performing load hoisting and store
>sinking from the loop. The compiler considers these two addresses as
>two different addresses (they are also defined in two different named
>address spaces), and optimizes access to them. So, without barrier(),
>it simply loads the value from A20_TEST_ADDR and A20_TEST_ADDR+0x10
>before the loop, resulting in:
>
> 9: 65 8b 0d 00 02 00 00 mov %gs:0x200,%ecx
> ...
> 16: 64 8b 35 10 02 00 00 mov %fs:0x210,%esi
> ...
> 28: 83 ea 01 sub $0x1,%edx
> 2b: 74 0a je 37 <a20_test_ref+0x37>
> 2d: e6 80 out %al,$0x80
> 2f: 89 d9 mov %ebx,%ecx
> 31: 29 d1 sub %edx,%ecx
> 33: 39 ce cmp %ecx,%esi
> 35: 74 f1 je 28 <a20_test_ref+0x28>
>
>The solution with barrier() introduces memory clobber between store
>and load, so the compiler is now forced to load and store the values
>due to the side effects of the barrier() inbetween. This kind of
>works, but is just a workaround for what really happens. In reality,
>the value at the test address changes "behind the compiler back", IOW
>- variable’s value can change in ways the compiler cannot predict.
>
>My proposal is to use a volatile pointer to an absolute address, so
>the unwanted optimizations are suppressed. The generated code is the
>same as with barrier(), but now the code tells the compiler that every
>read and write to this address must happen exactly as written in the
>source code. Before your patch, the accessors were defined with
>volatile asm, and this is the place where volatile qualifier matters.
>So, my proposed code would read:
>
>#define A20_TEST_ADDR (4*0x80)
>
>#define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
>#define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
>
>static int a20_test(int loops)
>{
> int saved, ctr;
>
> set_fs(0xffff);
>
> saved = ctr = A20_TEST_GS;
>
> do {
> A20_TEST_GS = ++ctr;
> io_delay(); /* Make constant delay */
> if (A20_TEST_FS != ctr)
> break;
> } while (--loops);
>
> A20_TEST_GS = saved;
> return loops;
>}
>
>BR,
>Uros.
>
I disagree with that being the preferred solution, and it isn't really the Linux style.
Not only does it require making more changes to the macros, but the barrier() construct is well established in Linux as the way to indicate that a memory variable is subject to examination and/or modification by another system agent, while still being a memory variable.
It also generally produces better code.
So no, your analysis is, in my opinion, incorrect in light of the way the Linux memory model is already used.
^ permalink raw reply
* Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the real-mode boot code
From: Uros Bizjak @ 2026-01-21 16:03 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <4E5A81C0-1938-4FB0-A0ED-2CCF03B9C480@zytor.com>
On Wed, Jan 21, 2026 at 4:14 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >My proposal is to use a volatile pointer to an absolute address, so
> >the unwanted optimizations are suppressed. The generated code is the
> >same as with barrier(), but now the code tells the compiler that every
> >read and write to this address must happen exactly as written in the
> >source code. Before your patch, the accessors were defined with
> >volatile asm, and this is the place where volatile qualifier matters.
> >So, my proposed code would read:
> >
> >#define A20_TEST_ADDR (4*0x80)
> >
> >#define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
> >#define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
> >
> >static int a20_test(int loops)
> >{
> > int saved, ctr;
> >
> > set_fs(0xffff);
> >
> > saved = ctr = A20_TEST_GS;
> >
> > do {
> > A20_TEST_GS = ++ctr;
> > io_delay(); /* Make constant delay */
> > if (A20_TEST_FS != ctr)
> > break;
> > } while (--loops);
> >
> > A20_TEST_GS = saved;
> > return loops;
> >}
> >
> >BR,
> >Uros.
> >
>
> I disagree with that being the preferred solution, and it isn't really the Linux style.
>
> Not only does it require making more changes to the macros, but the barrier() construct is well established in Linux as the way to indicate that a memory variable is subject to examination and/or modification by another system agent, while still being a memory variable.
>
> It also generally produces better code.
>
> So no, your analysis is, in my opinion, incorrect in light of the way the Linux memory model is already used.
Thanks for explaining to me the Linux way! If this is the case, I will
withdraw my proposed solution.
Best regards,
Uros.
^ permalink raw reply
* Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the real-mode boot code
From: Uros Bizjak @ 2026-01-21 16:07 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Petr Mladek, Andrew Morton, Kees Cook, Peter Zijlstra (Intel),
Nathan Chancellor, Kiryl Shutsemau, Rick Edgecombe, linux-kernel,
linux-coco, x86
In-Reply-To: <20260120195407.1163051-12-hpa@zytor.com>
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> All supported versions of gcc support __seg_fs and __seg_gs now.
> All supported versions of clang support __seg_fs and __seg_gs too,
> except for two bugs (as of clang 21, at least):
>
> 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> 2. An internal compiler error when addressing symbols directly.
>
> However, none of these are required in the boot code. Furthermore,
> this makes it possible to remove the absolute_pointer() hack in the
> fs/gs access functions.
>
> This requires adding a barrier() to a20.c, to prevent the compiler
> from eliding the load from the aliased memory address.
>
> Remove the unused memcmp_[fg]s() functions.
>
> Finally, ds() is by necessity constant, so mark the function as such.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
> ---
> arch/x86/boot/a20.c | 1 +
> arch/x86/boot/boot.h | 81 ++++++++++++++------------------------------
> 2 files changed, 27 insertions(+), 55 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 3ab6cd8eaa31..52c3fccdcb70 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -63,6 +63,7 @@ static int a20_test(int loops)
> while (loops--) {
> wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> + barrier(); /* Compiler won't know about fs/gs overlap */
> ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> if (ok)
> break;
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index b4eb8405ba55..4d3549ed7987 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -45,7 +45,7 @@ static inline void io_delay(void)
>
> /* These functions are used to reference data in other segments. */
>
> -static inline u16 ds(void)
> +static inline __attribute_const__ u16 ds(void)
> {
> u16 seg;
> asm("movw %%ds,%0" : "=rm" (seg));
> @@ -54,7 +54,7 @@ static inline u16 ds(void)
>
> static inline void set_fs(u16 seg)
> {
> - asm volatile("movw %0,%%fs" : : "rm" (seg));
> + asm volatile("movw %0,%%fs" : : "rm" (seg) : "memory");
> }
> static inline u16 fs(void)
> {
> @@ -65,7 +65,7 @@ static inline u16 fs(void)
>
> static inline void set_gs(u16 seg)
> {
> - asm volatile("movw %0,%%gs" : : "rm" (seg));
> + asm volatile("movw %0,%%gs" : : "rm" (seg) : "memory");
> }
> static inline u16 gs(void)
> {
> @@ -76,96 +76,67 @@ static inline u16 gs(void)
>
> typedef unsigned int addr_t;
>
> +/*
> + * WARNING: as of clang 21, clang has the following two bugs related
> + * to __seg_fs and __seg_gs:
> + *
> + * 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> + * 2. An internal compiler error when addressing symbols directly.
> + *
> + * Neither of those constructs are currently used in the boot code.
> + * If they ever are, and those bugs still remain, then those bugs will
> + * need to be worked around.
> + */
> static inline u8 rdfs8(addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - u8 v;
> - asm volatile("movb %%fs:%1,%0" : "=q" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u8 *)addr;
> }
> static inline u16 rdfs16(addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - u16 v;
> - asm volatile("movw %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u16 *)addr;
> }
> static inline u32 rdfs32(addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - u32 v;
> - asm volatile("movl %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u32 *)addr;
> }
>
> static inline void wrfs8(u8 v, addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - asm volatile("movb %1,%%fs:%0" : "+m" (*ptr) : "qi" (v));
> + *(__seg_fs u8 *)addr = v;
> }
> static inline void wrfs16(u16 v, addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - asm volatile("movw %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_fs u16 *)addr = v;
> }
> static inline void wrfs32(u32 v, addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - asm volatile("movl %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_fs u32 *)addr = v;
> }
>
> static inline u8 rdgs8(addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - u8 v;
> - asm volatile("movb %%gs:%1,%0" : "=q" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u8 *)addr;
> }
> static inline u16 rdgs16(addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - u16 v;
> - asm volatile("movw %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u16 *)addr;
> }
> static inline u32 rdgs32(addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - u32 v;
> - asm volatile("movl %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u32 *)addr;
> }
>
> static inline void wrgs8(u8 v, addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - asm volatile("movb %1,%%gs:%0" : "+m" (*ptr) : "qi" (v));
> + *(__seg_gs u8 *)addr = v;
> }
> static inline void wrgs16(u16 v, addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - asm volatile("movw %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_gs u16 *)addr = v;
> }
> static inline void wrgs32(u32 v, addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - asm volatile("movl %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
> -}
> -
> -/* Note: these only return true/false, not a signed return value! */
> -static inline bool memcmp_fs(const void *s1, addr_t s2, size_t len)
> -{
> - bool diff;
> - asm volatile("fs repe cmpsb"
> - : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> - return diff;
> -}
> -static inline bool memcmp_gs(const void *s1, addr_t s2, size_t len)
> -{
> - bool diff;
> - asm volatile("gs repe cmpsb"
> - : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> - return diff;
> + *(__seg_gs u32 *)addr = v;
> }
>
> /* Heap -- available for dynamic lists. */
> --
> 2.52.0
>
^ permalink raw reply
* [Invitation] bi-weekly guest_memfd upstream call on 2026-01-22
From: David Hildenbrand (Red Hat) @ 2026-01-21 20:52 UTC (permalink / raw)
To: linux-coco@lists.linux.dev, linux-mm@kvack.org, KVM
Hi,
Our next guest_memfd upstream call is scheduled for Thursday,
2026-01-22 at 8:00 - 9:00am (GMT-08:00) Pacific Time - Vancouver.
Ackerley will lead this meeting as I am still on PTO. I might still
join, though.
We'll be using the following Google meet:
http://meet.google.com/wxp-wtju-jzw
The meeting notes can be found at [1], where we also link recordings and
collect current guest_memfd upstream proposals. If you want an google
calendar invitation that also covers all future meetings, just write me
or Ackerley a mail.
In this meeting, we'll discuss:
(1) Next steps for direct-map removal support
(2) guest_memfd THP timeline.
To put something to discuss onto the agenda, reply to this mail or add
them to the "Topics/questions for next meeting(s)" section in the
meeting notes as a comment.
[1]
https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?usp=sharing
--
Cheers
David
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Sean Christopherson @ 2026-01-21 22:12 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
Xiaoyao Li, Dave Hansen, Yan Y Zhao, Binbin Wu, kas@kernel.org,
linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Isaku Yamahata,
Vishal Annapurve, Chao Gao, bp@alien8.de, x86@kernel.org
In-Reply-To: <24665176b1e6b169441c9f6db9b5d02d073377a4.camel@intel.com>
On Tue, Jan 20, 2026, Rick P Edgecombe wrote:
> Sean, really appreciate you taking a look despite being overbooked.
>
> On Fri, 2026-01-16 at 16:53 -0800, Sean Christopherson wrote:
> > NAK. I kinda sorta get why you did this? But the pages KVM uses for page tables
> > are KVM's, not to be mixed with PAMT pages.
> >
> > Eww. Definitely a hard "no". In tdp_mmu_alloc_sp_for_split(), the allocation
> > comes from KVM:
> >
> > if (mirror) {
> > sp->external_spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> > if (!sp->external_spt) {
> > free_page((unsigned long)sp->spt);
> > kmem_cache_free(mmu_page_header_cache, sp);
> > return NULL;
> > }
> > }
>
> Ah, this is from the TDX huge pages series. There is a bit of fallout from TDX
> /coco's eternal nemesis: stacks of code all being co-designed at once.
>
> Dave has been directing us recently to focus on only the needs of the current
> series. Now that we can test at each incremental step we don't have the same
> problems as before. But of course there is still desire for updated TDX huge
> pages, etc to help with development of all the other WIP stuff.
>
> For this design aspect of how the topup caches work for DPAMT, he asked
> specifically for the DPAMT patches to *not* consider how TDX huge pages will use
> them.
>
> Now the TDX huge pages coverletter asked you to look at some aspects of that,
> and traditionally KVM side has preferred to look at how the code is all going to
> work together. The presentation of this was a bit rushed and confused, but
> looking forward, how do you want to do this?
>
> After the 130 patches ordeal, I'm a bit amenable to Dave's view. What do you
> think?
IMO, it's largely irrelevant for this discussion. Bluntly, the code proposed
here is simply bad. S-EPT hugepage support just makes it worse.
The core issue is that the ownership of the pre-allocation cache is split across
KVM and the TDX subsystem (and within KVM, between tdx.c and the MMU), which makes
it extremely difficult to understand who is responsible for what, which in turn
leads to brittle code, and sets the hugepage series up to fail, e.g. by unnecessarily
mixing S-EPT page allocation with PAMT maintenance.q
That aside, I generally agree with Dave. The only caveat I'll throw in is that
I do think we need to _at least_ consider how things will likely play out when
all is said and done, otherwise we'll probably paint ourselves into a corner.
E.g. we don't need to know exactly how S-EPT hugepage support will interact with
DPAMT, but IMO we do need to be aware that KVM will need to demote pages outside
of vCPU context, and thus will need to pre-allocate pages for PAMT without having
a loaded/running vCPU. That knowledge doesn't require active support in the
DPAMT series, but it most definitely influences design decisions.
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Edgecombe, Rick P @ 2026-01-21 22:34 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <aXFPNbCvKURxby1q@google.com>
On Wed, 2026-01-21 at 14:12 -0800, Sean Christopherson wrote:
> IMO, it's largely irrelevant for this discussion. Bluntly, the code proposed
> here is simply bad.
>
FWIW, I wasn't ecstatic about it, but was starting to doubt we could find a
better solution after a string of failed POCs. Will take this feedback under
consideration for the future.
> S-EPT hugepage support just makes it worse.
>
> The core issue is that the ownership of the pre-allocation cache is split across
> KVM and the TDX subsystem (and within KVM, between tdx.c and the MMU), which makes
> it extremely difficult to understand who is responsible for what, which in turn
> leads to brittle code, and sets the hugepage series up to fail, e.g. by unnecessarily
> mixing S-EPT page allocation with PAMT maintenance.q
...or require more extensive changes with relevant huge page related
justification, right? We are not defining uABI I think.
>
> That aside, I generally agree with Dave.
>
Ok.
> The only caveat I'll throw in is that
> I do think we need to _at least_ consider how things will likely play out when
> all is said and done, otherwise we'll probably paint ourselves into a corner.
Well this is the tricky part then.
> E.g. we don't need to know exactly how S-EPT hugepage support will interact with
> DPAMT, but IMO we do need to be aware that KVM will need to demote pages outside
> of vCPU context, and thus will need to pre-allocate pages for PAMT without having
> a loaded/running vCPU. That knowledge doesn't require active support in the
> DPAMT series, but it most definitely influences design decisions.
I see what you are saying here though. It depends on how you look at it a bit.
^ permalink raw reply
* Re: [PATCH] x86/tdx: Use pg_level in TDX APIs, not the TDX-Module's 0-based level
From: Yan Zhao @ 2026-01-22 7:54 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng
In-Reply-To: <20260120203937.1447592-1-seanjc@google.com>
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
Tested-by: Yan Zhao <yan.y.zhao@intel.com>
On Tue, Jan 20, 2026 at 12:39:37PM -0800, Sean Christopherson wrote:
> Rework the TDX APIs to take the kernel's 1-based pg_level enum, not the
> TDX-Module's 0-based level. The APIs are _kernel_ APIs, not TDX-Module
> APIs, and the kernel (and KVM) uses "enum pg_level" literally everywhere.
>
> Using "enum pg_level" eliminates ambiguity when looking at the APIs (it's
> NOT clear that "int level" refers to the TDX-Module's level), and will
> allow for using existing helpers like page_level_size() when support for
> hugepages is added to the S-EPT APIs.
>
> No functional change intended.
>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Vishal Annapurve <vannapurve@google.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>
> Compile-tested only. Came across this when looking at the S-EPT hugepage
> series, specifically this code:
>
> unsigned long npages = 1 << (level * PTE_SHIFT);
>
> which I was _sure_ was broken, until I realized @level wasn't what I thought
> it was.
>
> arch/x86/include/asm/tdx.h | 14 ++++----------
> arch/x86/kvm/vmx/tdx.c | 11 ++++-------
> arch/x86/virt/vmx/tdx/tdx.c | 26 ++++++++++++++++++--------
> 3 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 6b338d7f01b7..bc0d03e70fd6 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -189,19 +189,13 @@ static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
> return ret;
> }
>
> -static inline int pg_level_to_tdx_sept_level(enum pg_level level)
> -{
> - WARN_ON_ONCE(level == PG_LEVEL_NONE);
> - return level - 1;
> -}
> -
> u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
> u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
> u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2);
> -u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2);
> +u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level, struct page *page, u64 *ext_err1, u64 *ext_err2);
> u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page);
> -u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2);
> -u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, int level, u64 *ext_err1, u64 *ext_err2);
> +u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level, struct page *page, u64 *ext_err1, u64 *ext_err2);
> +u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
> u64 tdh_mng_key_config(struct tdx_td *td);
> u64 tdh_mng_create(struct tdx_td *td, u16 hkid);
> u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp);
> @@ -217,7 +211,7 @@ u64 tdh_vp_rd(struct tdx_vp *vp, u64 field, u64 *data);
> u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask);
> u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_size);
> u64 tdh_mem_track(struct tdx_td *tdr);
> -u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, u64 level, u64 *ext_err1, u64 *ext_err2);
> +u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
> u64 tdh_phymem_cache_wb(bool resume);
> u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td);
> u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page);
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 2d7a4d52ccfb..c47f4de2f19c 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1648,14 +1648,13 @@ static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
> enum pg_level level, kvm_pfn_t pfn)
> {
> - int tdx_level = pg_level_to_tdx_sept_level(level);
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> struct page *page = pfn_to_page(pfn);
> gpa_t gpa = gfn_to_gpa(gfn);
> u64 entry, level_state;
> u64 err;
>
> - err = tdh_mem_page_aug(&kvm_tdx->td, gpa, tdx_level, page, &entry, &level_state);
> + err = tdh_mem_page_aug(&kvm_tdx->td, gpa, level, page, &entry, &level_state);
> if (unlikely(tdx_operand_busy(err)))
> return -EBUSY;
>
> @@ -1699,12 +1698,11 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn,
> static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn,
> enum pg_level level, void *private_spt)
> {
> - int tdx_level = pg_level_to_tdx_sept_level(level);
> gpa_t gpa = gfn_to_gpa(gfn);
> struct page *page = virt_to_page(private_spt);
> u64 err, entry, level_state;
>
> - err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, tdx_level, page, &entry,
> + err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, page, &entry,
> &level_state);
> if (unlikely(tdx_operand_busy(err)))
> return -EBUSY;
> @@ -1788,7 +1786,6 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> enum pg_level level, u64 mirror_spte)
> {
> struct page *page = pfn_to_page(spte_to_pfn(mirror_spte));
> - int tdx_level = pg_level_to_tdx_sept_level(level);
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> gpa_t gpa = gfn_to_gpa(gfn);
> u64 err, entry, level_state;
> @@ -1808,7 +1805,7 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> return;
>
> err = tdh_do_no_vcpus(tdh_mem_range_block, kvm, &kvm_tdx->td, gpa,
> - tdx_level, &entry, &level_state);
> + level, &entry, &level_state);
> if (TDX_BUG_ON_2(err, TDH_MEM_RANGE_BLOCK, entry, level_state, kvm))
> return;
>
> @@ -1824,7 +1821,7 @@ static void tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> * Race with TDH.VP.ENTER due to (0-step mitigation) and Guest TDCALLs.
> */
> err = tdh_do_no_vcpus(tdh_mem_page_remove, kvm, &kvm_tdx->td, gpa,
> - tdx_level, &entry, &level_state);
> + level, &entry, &level_state);
> if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_REMOVE, entry, level_state, kvm))
> return;
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 5ce4ebe99774..22c0f832cb37 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1516,6 +1516,12 @@ static void tdx_clflush_page(struct page *page)
> clflush_cache_range(page_to_virt(page), PAGE_SIZE);
> }
>
> +static int pg_level_to_tdx_sept_level(enum pg_level level)
> +{
> + WARN_ON_ONCE(level == PG_LEVEL_NONE);
> + return level - 1;
> +}
> +
> noinstr u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args)
> {
> args->rcx = td->tdvpr_pa;
> @@ -1556,10 +1562,11 @@ u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mem_page_add);
>
> -u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level,
> + struct page *page, u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> - .rcx = gpa | level,
> + .rcx = gpa | pg_level_to_tdx_sept_level(level),
> .rdx = tdx_tdr_pa(td),
> .r8 = page_to_phys(page),
> };
> @@ -1587,10 +1594,11 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_vp_addcx);
>
> -u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
> + struct page *page, u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> - .rcx = gpa | level,
> + .rcx = gpa | pg_level_to_tdx_sept_level(level),
> .rdx = tdx_tdr_pa(td),
> .r8 = page_to_phys(page),
> };
> @@ -1606,10 +1614,11 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, int level, struct page *page, u
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mem_page_aug);
>
> -u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, int level, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_range_block(struct tdx_td *td, u64 gpa, enum pg_level level,
> + u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> - .rcx = gpa | level,
> + .rcx = gpa | pg_level_to_tdx_sept_level(level),
> .rdx = tdx_tdr_pa(td),
> };
> u64 ret;
> @@ -1822,10 +1831,11 @@ u64 tdh_mem_track(struct tdx_td *td)
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mem_track);
>
> -u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, u64 level, u64 *ext_err1, u64 *ext_err2)
> +u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level,
> + u64 *ext_err1, u64 *ext_err2)
> {
> struct tdx_module_args args = {
> - .rcx = gpa | level,
> + .rcx = gpa | pg_level_to_tdx_sept_level(level),
> .rdx = tdx_tdr_pa(td),
> };
> u64 ret;
>
> base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
> --
> 2.52.0.457.g6b5491de43-goog
>
^ permalink raw reply
* Re: SVSM Development Call January 21, 2026
From: Jörg Rödel @ 2026-01-22 12:55 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <4uhsm4wvzsoto4e6koyccvsdqcg2pber4s4oexclhywps7pz5l@tpkfglmsge45>
Meeting minutes are available here:
https://github.com/coconut-svsm/governance/pull/93
-Joerg
^ 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