On Tue, Jun 14, 2022 at 03:02:31PM +0300, Kirill A. Shutemov wrote:
> +static bool is_tdx_guest(void)
> +{
> + static bool once;
> + static bool is_tdx;
> +
> + if (!IS_ENABLED(CONFIG_INTEL_TDX_GUEST))
> + return false;
> +
> + if (!once) {
> + u32 eax, sig[3];
> +
> + cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax,
> + &sig[0], &sig[2], &sig[1]);
> + is_tdx = !memcmp(TDX_IDENT, sig, sizeof(sig));
> + once = true;
> + }
> +
> + return is_tdx;
> +}
early_tdx_detect() already calls this CPUID function. It assigns
function pointers too.
So why can't you assign an accept_memory() function pointer there and
get rid of this sprinkled if (tdx) everywhere?
> diff --git a/arch/x86/boot/compressed/tdx.c b/arch/x86/boot/compressed/tdx.c
> index 918a7606f53c..8518a75e5dd5 100644
> --- a/arch/x86/boot/compressed/tdx.c
> +++ b/arch/x86/boot/compressed/tdx.c
> @@ -3,12 +3,15 @@
> #include "../cpuflags.h"
> #include "../string.h"
> #include "../io.h"
> +#include "align.h"
> #include "error.h"
> +#include "pgtable_types.h"
>
> #include <vdso/limits.h>
> #include <uapi/asm/vmx.h>
>
> #include <asm/shared/tdx.h>
> +#include <asm/page_types.h>
>
> /* Called from __tdx_hypercall() for unrecoverable failure */
> void __tdx_hypercall_failed(void)
> @@ -75,3 +78,78 @@ void early_tdx_detect(void)
> pio_ops.f_outb = tdx_outb;
> pio_ops.f_outw = tdx_outw;
> }
> +
> +static unsigned long try_accept_one(phys_addr_t start, unsigned long len,
> + enum pg_level level)
That's pretty much a copy of the same function in arch/x86/coco/tdx/tdx.c.
Yeah, you need a tdx-shared.c which you include in both places just like
it is done with sev-shared.c
...
> +void tdx_accept_memory(phys_addr_t start, phys_addr_t end)
That one too.
> +{
> + /*
> + * Notify the VMM about page mapping conversion. More info about ABI
> + * can be found in TDX Guest-Host-Communication Interface (GHCI),
> + * section "TDG.VP.VMCALL<MapGPA>"
> + */
> + if (_tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0))
> + error("Accepting memory failed\n");
> +
> + /*
> + * For shared->private conversion, accept the page using
> + * TDX_ACCEPT_PAGE TDX module call.
> + */
> + while (start < end) {
> + unsigned long len = end - start;
> + unsigned long accept_size;
> +
> + /*
> + * Try larger accepts first. It gives chance to VMM to keep
> + * 1G/2M Secure EPT entries where possible and speeds up
> + * process by cutting number of hypercalls (if successful).
> + */
> +
> + accept_size = try_accept_one(start, len, PG_LEVEL_1G);
> + if (!accept_size)
> + accept_size = try_accept_one(start, len, PG_LEVEL_2M);
> + if (!accept_size)
> + accept_size = try_accept_one(start, len, PG_LEVEL_4K);
> + if (!accept_size)
> + error("Accepting memory failed\n");
> + start += accept_size;
This series of calls to try_accept_one() appear in at least three
places. Please carve them out into a separate function can put it in
tdx-shared.c.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
