diff mbox series

[PATCHv11,09/19] x86/tdx: Account shared memory

Message ID 20240528095522.509667-10-kirill.shutemov@linux.intel.com
State Superseded
Headers show
Series x86/tdx: Add kexec support | expand

Commit Message

Kirill A. Shutemov May 28, 2024, 9:55 a.m. UTC 
The kernel will convert all shared memory back to private during kexec.
The direct mapping page tables will provide information on which memory
is shared.

It is extremely important to convert all shared memory. If a page is
missed, it will cause the second kernel to crash when it accesses it.

Keep track of the number of shared pages. This will allow for
cross-checking against the shared information in the direct mapping and
reporting if the shared bit is lost.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Tao Liu <ltao@redhat.com>
---
 arch/x86/coco/tdx/tdx.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Dave Hansen June 4, 2024, 4:08 p.m. UTC | #1 
On 5/28/24 02:55, Kirill A. Shutemov wrote:
> Keep track of the number of shared pages. This will allow for 
> cross-checking against the shared information in the direct mapping
> and reporting if the shared bit is lost.

It's probably also worth mentioning that conversions are slow and
relatively rare and even though a global atomic isn't really scalable,
it also isn't worth doing anything fancier.
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 26fa47db5782..979891e97d83 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -38,6 +38,8 @@
>  
>  #define TDREPORT_SUBTYPE_0	0
>  
> +static atomic_long_t nr_shared;

Doesn't this technically need to be:

	static atomic_long_t nr_shared = ATOMIC_LONG_INIT(0);

?  I thought we had some architectures where the 0 logical value wasn't
actually all 0's.
Kirill A. Shutemov June 4, 2024, 4:24 p.m. UTC | #2 
On Tue, Jun 04, 2024 at 09:08:25AM -0700, Dave Hansen wrote:
> On 5/28/24 02:55, Kirill A. Shutemov wrote:
> > Keep track of the number of shared pages. This will allow for 
> > cross-checking against the shared information in the direct mapping
> > and reporting if the shared bit is lost.
> 
> It's probably also worth mentioning that conversions are slow and
> relatively rare and even though a global atomic isn't really scalable,
> it also isn't worth doing anything fancier.

Okay, will do.

> > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> > index 26fa47db5782..979891e97d83 100644
> > --- a/arch/x86/coco/tdx/tdx.c
> > +++ b/arch/x86/coco/tdx/tdx.c
> > @@ -38,6 +38,8 @@
> >  
> >  #define TDREPORT_SUBTYPE_0	0
> >  
> > +static atomic_long_t nr_shared;
> 
> Doesn't this technically need to be:
> 
> 	static atomic_long_t nr_shared = ATOMIC_LONG_INIT(0);
> 
> ?  I thought we had some architectures where the 0 logical value wasn't
> actually all 0's.

Hm. I am not aware of such requirement. I see plenty uninitilized
atomic_long_t in generic code. For instance, invalid_kread_bytes.

And I doubt TDX will ever be built for non-x86 :P
diff mbox series

Patch

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 26fa47db5782..979891e97d83 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -38,6 +38,8 @@ 
 
 #define TDREPORT_SUBTYPE_0	0
 
+static atomic_long_t nr_shared;
+
 /* Called from __tdx_hypercall() for unrecoverable failure */
 noinstr void __noreturn __tdx_hypercall_failed(void)
 {
@@ -821,6 +823,11 @@  static int tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
 	if (!enc && !tdx_enc_status_changed(vaddr, numpages, enc))
 		return -EIO;
 
+	if (enc)
+		atomic_long_sub(numpages, &nr_shared);
+	else
+		atomic_long_add(numpages, &nr_shared);
+
 	return 0;
 }