[PATCH] x86/tdx: Fix crash on kexec

[PATCH] x86/tdx: Fix crash on kexec

[Kirill A. Shutemov]

The function tdx_enc_status_changed() was modified to handle vmalloc()
mappings. It now utilizes slow_virt_to_phys() to determine the physical
address of the page by walking page tables and looking for the physical
address in the page table entry.

However, this adjustment conflicted with the enabling of kexec. The
function tdx_kexec_finish() clears the page table entry before calling
tdx_enc_status_changed(), causing a BUG_ON() error in
slow_virt_to_phys().

To address this issue, tdx_enc_status_change() should use __pa() to
obtain physical addresses whenever possible. The virt_addr_valid() check
will handle such cases, while any other scenarios, including vmalloc()
mappings, will resort to slow_virt_to_phys().

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Fixes: e1b8ac3aae58 ("x86/tdx: Support vmalloc() for tdx_enc_status_changed()")
---
 arch/x86/coco/tdx/tdx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index ef8ec2425998..8f471260924f 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -813,8 +813,16 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
 		step = PAGE_SIZE;
 
 	for (addr = start; addr < end; addr += step) {
-		phys_addr_t start_pa = slow_virt_to_phys((void *)addr);
-		phys_addr_t end_pa   = start_pa + step;
+		phys_addr_t start_pa;
+		phys_addr_t end_pa;
+
+		/* The check fails on vmalloc() mappings */
+		if (virt_addr_valid(addr))
+			start_pa = __pa(addr);
+		else
+			start_pa = slow_virt_to_phys((void *)addr);
+
+		end_pa = start_pa + step;
 
 		if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
 			return false;
-- 
2.43.0

Re: [PATCH] x86/tdx: Fix crash on kexec

[Borislav Petkov]

On Sat, Jun 29, 2024 at 04:06:20PM +0300, Kirill A. Shutemov wrote:
> The function tdx_enc_status_changed() was modified to handle vmalloc()
> mappings. It now utilizes slow_virt_to_phys() to determine the physical
> address of the page by walking page tables and looking for the physical
> address in the page table entry.
> 
> However, this adjustment conflicted with the enabling of kexec. The
> function tdx_kexec_finish() clears the page table entry before calling
> tdx_enc_status_changed(), causing a BUG_ON() error in
> slow_virt_to_phys().
> 
> To address this issue, tdx_enc_status_change() should use __pa() to
> obtain physical addresses whenever possible. The virt_addr_valid() check
> will handle such cases, while any other scenarios, including vmalloc()
> mappings, will resort to slow_virt_to_phys().
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Fixes: e1b8ac3aae58 ("x86/tdx: Support vmalloc() for tdx_enc_status_changed()")

I'm going to zap this one from x86/urgent and give you guys ample time to test
thus stuff better and longer.

Also, what is this e1b8ac3aae58 fixing and why is it urgent?

AFAICT, it can go through the normal merge window...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/tdx: Fix crash on kexec

[Kirill A. Shutemov]

On Sat, Jun 29, 2024 at 03:59:33PM +0200, Borislav Petkov wrote:
> On Sat, Jun 29, 2024 at 04:06:20PM +0300, Kirill A. Shutemov wrote:
> > The function tdx_enc_status_changed() was modified to handle vmalloc()
> > mappings. It now utilizes slow_virt_to_phys() to determine the physical
> > address of the page by walking page tables and looking for the physical
> > address in the page table entry.
> > 
> > However, this adjustment conflicted with the enabling of kexec. The
> > function tdx_kexec_finish() clears the page table entry before calling
> > tdx_enc_status_changed(), causing a BUG_ON() error in
> > slow_virt_to_phys().
> > 
> > To address this issue, tdx_enc_status_change() should use __pa() to
> > obtain physical addresses whenever possible. The virt_addr_valid() check
> > will handle such cases, while any other scenarios, including vmalloc()
> > mappings, will resort to slow_virt_to_phys().
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Fixes: e1b8ac3aae58 ("x86/tdx: Support vmalloc() for tdx_enc_status_changed()")
> 
> I'm going to zap this one from x86/urgent and give you guys ample time to test
> thus stuff better and longer.
> 
> Also, what is this e1b8ac3aae58 fixing and why is it urgent?

Daxuan, how urgent is this fix for you?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

RE: [PATCH] x86/tdx: Fix crash on kexec

[Dexuan Cui]

> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Sent: Saturday, June 29, 2024 7:04 AM
> 
> On Sat, Jun 29, 2024 at 03:59:33PM +0200, Borislav Petkov wrote:
> > On Sat, Jun 29, 2024 at 04:06:20PM +0300, Kirill A. Shutemov wrote:
> > > The function tdx_enc_status_changed() was modified to handle vmalloc()
> > > mappings. It now utilizes slow_virt_to_phys() to determine the physical
> > > address of the page by walking page tables and looking for the physical
> > > address in the page table entry.
> > >
> > > However, this adjustment conflicted with the enabling of kexec. The
> > > function tdx_kexec_finish() clears the page table entry before calling
> > > tdx_enc_status_changed(), causing a BUG_ON() error in
> > > slow_virt_to_phys().
> > >
> > > To address this issue, tdx_enc_status_change() should use __pa() to
> > > obtain physical addresses whenever possible. The virt_addr_valid() check
> > > will handle such cases, while any other scenarios, including vmalloc()
> > > mappings, will resort to slow_virt_to_phys().
> > >
> > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Fixes: e1b8ac3aae58 ("x86/tdx: Support vmalloc() for
> tdx_enc_status_changed()")
> >
> > I'm going to zap this one from x86/urgent and give you guys ample time to
> >  test
> > thus stuff better and longer.
> >
> > Also, what is this e1b8ac3aae58 fixing and why is it urgent?
> 
> Daxuan, how urgent is this fix for you?
> 
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

Hi Kirill, Boris,
IMO e1b8ac3aae58  is not urgent and can go through the normal merge window.
It would be great to add e1b8ac3aae58 to the branch x86/tdx.

Thanks,
Dexuan

Re: [PATCH] x86/tdx: Fix crash on kexec

[Borislav Petkov]

On Sat, Jun 29, 2024 at 07:27:57PM +0000, Dexuan Cui wrote:
> It would be great to add e1b8ac3aae58 to the branch x86/tdx.

Sure we will, once it is properly tested. This very thread says otherwise.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH] x86/tdx: Fix crash on kexec

[Dexuan Cui]

> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Saturday, June 29, 2024 12:41 PM
> To: Dexuan Cui <decui@microsoft.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>; Dave Hansen
> <dave.hansen@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> Ingo Molnar <mingo@redhat.com>; x86@kernel.org; H. Peter Anvin
> <hpa@zytor.com>; Michael Kelley <mikelley@microsoft.com>;
> Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@linux.intel.com>; Kai Huang
> <kai.huang@intel.com>; Rick Edgecombe <rick.p.edgecombe@intel.com>;
> linux-coco@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] x86/tdx: Fix crash on kexec
> 
> On Sat, Jun 29, 2024 at 07:27:57PM +0000, Dexuan Cui wrote:
> > It would be great to add e1b8ac3aae58 to the branch x86/tdx.
> 
> Sure we will, once it is properly tested. This very thread says otherwise.
> --
> Regards/Gruss,
>     Boris.

Hi Kirill, Dave,
Do you think if it's a good idea if I post a new patch that combines
    e1b8ac3aae58 ("x86/tdx: Support vmalloc() for tdx_enc_status_changed()")
and
    your patch "[PATCH] x86/tdx: Fix crash on kexec"?
 
Or, maybe Dave can help combine the two patches into one?

Just wanted to make sure e1b8ac3aae58 won't get lost.

Thanks,
Dexuan

Re: [PATCH] x86/tdx: Fix crash on kexec

[Kirill A. Shutemov]

On Wed, Jul 03, 2024 at 07:16:47PM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Borislav Petkov <bp@alien8.de>
> > Sent: Saturday, June 29, 2024 12:41 PM
> > To: Dexuan Cui <decui@microsoft.com>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>; Dave Hansen
> > <dave.hansen@linux.intel.com>; Thomas Gleixner <tglx@linutronix.de>;
> > Ingo Molnar <mingo@redhat.com>; x86@kernel.org; H. Peter Anvin
> > <hpa@zytor.com>; Michael Kelley <mikelley@microsoft.com>;
> > Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppuswamy@linux.intel.com>; Kai Huang
> > <kai.huang@intel.com>; Rick Edgecombe <rick.p.edgecombe@intel.com>;
> > linux-coco@lists.linux.dev; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] x86/tdx: Fix crash on kexec
> > 
> > On Sat, Jun 29, 2024 at 07:27:57PM +0000, Dexuan Cui wrote:
> > > It would be great to add e1b8ac3aae58 to the branch x86/tdx.
> > 
> > Sure we will, once it is properly tested. This very thread says otherwise.
> > --
> > Regards/Gruss,
> >     Boris.
> 
> Hi Kirill, Dave,
> Do you think if it's a good idea if I post a new patch that combines
>     e1b8ac3aae58 ("x86/tdx: Support vmalloc() for tdx_enc_status_changed()")
> and
>     your patch "[PATCH] x86/tdx: Fix crash on kexec"?

Yeah, IIUC, that's what Borislav wanted. After proper testing.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

RE: [PATCH] x86/tdx: Fix crash on kexec

[Dexuan Cui]

> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > [...]
> > Hi Kirill, Dave,
> > Do you think if it's a good idea if I post a new patch that combines
> >     e1b8ac3aae58 ("x86/tdx: Support vmalloc() for
> > tdx_enc_status_changed()")
> > and
> >     your patch "[PATCH] x86/tdx: Fix crash on kexec"?
> 
> Yeah, IIUC, that's what Borislav wanted. After proper testing.
> 
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

Hi Kirill,  
I tested the 2 patches for a Linux VM on Hyper-V and all worked fine.

When you finish testing, please let me know so that I can post
a combined patch; alternatively, it would be better if you can help post
a combined patch.

Thanks,
Dexuan

Re: [PATCH] x86/tdx: Fix crash on kexec

[Kirill A. Shutemov]

On Thu, Jul 04, 2024 at 02:48:49PM +0000, Dexuan Cui wrote:
> > From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > [...]
> > > Hi Kirill, Dave,
> > > Do you think if it's a good idea if I post a new patch that combines
> > >     e1b8ac3aae58 ("x86/tdx: Support vmalloc() for
> > > tdx_enc_status_changed()")
> > > and
> > >     your patch "[PATCH] x86/tdx: Fix crash on kexec"?
> > 
> > Yeah, IIUC, that's what Borislav wanted. After proper testing.
> > 
> > --
> >   Kiryl Shutsemau / Kirill A. Shutemov
> 
> Hi Kirill,  
> I tested the 2 patches for a Linux VM on Hyper-V and all worked fine.
> 
> When you finish testing, please let me know so that I can post
> a combined patch; alternatively, it would be better if you can help post
> a combined patch.

Go ahead, post the new version.

Borislav, could you drop the original patch from tip tree?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/tdx: Fix crash on kexec

[Borislav Petkov]

On Mon, Jul 08, 2024 at 03:34:34PM +0300, Kirill A. Shutemov wrote:
> Borislav, could you drop the original patch from tip tree?

Long gone already.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/tdx: Fix crash on kexec

[Kirill A. Shutemov]

On Mon, Jul 08, 2024 at 03:32:42PM +0200, Borislav Petkov wrote:
> On Mon, Jul 08, 2024 at 03:34:34PM +0300, Kirill A. Shutemov wrote:
> > Borislav, could you drop the original patch from tip tree?
> 
> Long gone already.

Hm. I still see it in tip/x86/cc branch which is merged in tip/master.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/tdx: Fix crash on kexec

[Borislav Petkov]

On Mon, Jul 08, 2024 at 04:51:12PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jul 08, 2024 at 03:32:42PM +0200, Borislav Petkov wrote:
> > On Mon, Jul 08, 2024 at 03:34:34PM +0300, Kirill A. Shutemov wrote:
> > > Borislav, could you drop the original patch from tip tree?
> > 
> > Long gone already.
> 
> Hm. I still see it in tip/x86/cc branch which is merged in tip/master.

Yeah, it should be gone now.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH] x86/tdx: Fix crash on kexec

[Dexuan Cui]

> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Sent: Monday, July 8, 2024 5:35 AM
> > [...]
> Go ahead, post the new version.

I just posted the new version.