lkml.org 
[lkml]   [2022]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCHv7 14/14] x86/tdx: Add unaccepted memory support
    On Tue, Jul 26, 2022 at 04:51:16PM +0200, Borislav Petkov wrote:
    > 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?

    This code called from EFI stub which runs before decompressor code and
    therefore before early_tdx_detect().

    > > 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

    Okay, will look into this.

    > > + 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.

    Okay.

    --
    Kiryl Shutsemau / Kirill A. Shutemov

    \
     
     \ /
      Last update: 2022-08-09 13:43    [W:4.252 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site