Received-SPF: pass (google.com: domain of linux-kernel+bounces-169977-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 6 May 2024 16:21:20 +0200
From: Borislav Petkov <bp@alien8.de>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: adrian.hunter@intel.com, ashish.kalra@amd.com, bhe@redhat.com,
	dave.hansen@linux.intel.com, elena.reshetova@intel.com,
	jun.nakajima@intel.com, kai.huang@intel.com,
	kexec@lists.infradead.org, linux-coco@lists.linux.dev,
	linux-kernel@vger.kernel.org, ltao@redhat.com, mingo@redhat.com,
	nik.borisov@suse.com, peterz@infradead.org, rafael@kernel.org,
	rick.p.edgecombe@intel.com,
	sathyanarayanan.kuppuswamy@linux.intel.com, seanjc@google.com,
	tglx@linutronix.de, thomas.lendacky@amd.com, x86@kernel.org
Subject: Re: [PATCHv10.1 09/18] x86/mm: Adding callbacks to prepare encrypted
 memory for kexec
Message-ID: <20240506142120.GKZjjnYGMcZkuTLlzG@fat_crate.local>
References: <20240427164747.GCZi0sM6HBCBYtgWqF@fat_crate.local>
 <20240427170634.2397725-1-kirill.shutemov@linux.intel.com>
 <20240502134506.GDZjOY4guvlKH9-73J@fat_crate.local>
 <ch244dd4k5nu5rcryuwqp4pztl4dduhciqunin5drr7a3yls2h@siisliaoc2f6>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <ch244dd4k5nu5rcryuwqp4pztl4dduhciqunin5drr7a3yls2h@siisliaoc2f6>

On Mon, May 06, 2024 at 04:22:02PM +0300, Kirill A. Shutemov wrote:
> I do. See comment just above enc_kexec_stop_conversion() call.

If you mean this:

        /*
         * Call enc_kexec_stop_conversion() while all CPUs are still active and
         * interrupts are enabled. This will allow all in-flight memory
         * conversions to finish cleanly.
         */
        if (kexec_in_progress)
                x86_platform.guest.enc_kexec_stop_conversion(false);

then no, this is not enough.

I mean this:

/**
 * struct x86_guest - Functions used by misc guest incarnations like SEV, TDX, etc.
 *
 * @enc_status_change_prepare   Notify HV before the encryption status of a range is changed
 * @enc_status_change_finish    Notify HV after the encryption status of a range is changed
 * @enc_tlb_flush_required      Returns true if a TLB flush is needed before changing page encryption status
 * @enc_cache_flush_required    Returns true if a cache flush is needed before changing page encryption status
 * @enc_kexec_begin		Begin the two-step process of stopping
 * 				page conversion... <insert reason why it
 * 				needs to happen this way, blabla>
 * @enc_kexec_finish		...
 */
struct x86_guest {
        int (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc);
        int (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc);
        bool (*enc_tlb_flush_required)(bool enc);
        bool (*enc_cache_flush_required)(void);
        void (*enc_kexec_begin)(bool crash);
        void (*enc_kexec_finish)(void);

And calling them a _begin and _finish makes a lot more sense to me:
_begin starts the kexec process for encrypted guests and _finish
finishes it.

Just from the names you now know what needs to happen and in which
order.

Thx.

-- 
Regards/Gruss,
    Boris.

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