Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ie3994620:Fastmail
Date:   Sat, 25 Mar 2023 03:51:57 +0300
From:   "Kirill A. Shutemov" <kirill@shutemov.name>
To:     Borislav Petkov <bp@alien8.de>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Sean Christopherson <seanjc@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Joerg Roedel <jroedel@suse.de>,
        Ard Biesheuvel <ardb@kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Ingo Molnar <mingo@redhat.com>,
        Dario Faggioli <dfaggioli@suse.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Mike Rapoport <rppt@kernel.org>,
        David Hildenbrand <david@redhat.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        marcelo.cerri@canonical.com, tim.gardner@canonical.com,
        khalid.elmously@canonical.com, philip.cox@canonical.com,
        aarcange@redhat.com, peterx@redhat.com, x86@kernel.org,
        linux-mm@kvack.org, linux-coco@lists.linux.dev,
        linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv8 06/14] efi/x86: Implement support for unaccepted memory
Message-ID: <20230325005157.37b4alnuf6p6mh66@box.shutemov.name>
References: <20221207014933.8435-1-kirill.shutemov@linux.intel.com>
 <20221207014933.8435-7-kirill.shutemov@linux.intel.com>
 <Y7Q5x7qV/Cmc/p8s@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y7Q5x7qV/Cmc/p8s@zn.tnic>
Precedence: bulk

On Tue, Jan 03, 2023 at 03:20:55PM +0100, Borislav Petkov wrote:
> > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> > index 6787ed8dfacf..8aa8adf0bcb5 100644
> > --- a/drivers/firmware/efi/Kconfig
> > +++ b/drivers/firmware/efi/Kconfig
> > @@ -314,6 +314,20 @@ config EFI_COCO_SECRET
> >  	  virt/coco/efi_secret module to access the secrets, which in turn
> >  	  allows userspace programs to access the injected secrets.
> >  
> > +config UNACCEPTED_MEMORY
> > +	bool
> > +	depends on EFI_STUB
> 
> This still doesn't make a whole lotta sense. If I do "make menuconfig" I don't
> see the help text because that bool doesn't have a string prompt. So who is that
> help text for?

It is a form of documentation for a developer. The same happens for other
options. For instance, BOOT_VESA_SUPPORT or ARCH_HAS_CURRENT_STACK_POINTER.

Yes, it is not visible user, but I still think it is helpful for a
developer to understand what the option does.

> Then, in the last patch you have
> 
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -888,6 +888,8 @@ config INTEL_TDX_GUEST
>         select ARCH_HAS_CC_PLATFORM
>         select X86_MEM_ENCRYPT
>         select X86_MCE
> +       select UNACCEPTED_MEMORY
> +       select EFI_STUB
> 
> I guess you want to select UNACCEPTED_MEMORY only.

I had to rework it as

config INTEL_TDX_GUEST
	...
	depends on EFI_STUB
	select UNACCEPTED_MEMORY

Naked select UNACCEPTED_MEMORY doesn't work if EFI and EFI_STUB is
disabled:

WARNING: unmet direct dependencies detected for UNACCEPTED_MEMORY
  Depends on [n]: EFI [=n] && EFI_STUB [=n]
  Selected by [y]:
  - INTEL_TDX_GUEST [=y] && HYPERVISOR_GUEST [=y] && X86_64 [=y] && CPU_SUP_INTEL [=y] && X86_X2APIC [=y]

IIUC, the alternative is to have selects all the way down the option tree.

> 
> And I've already mentioned this whole mess:
> 
> https://lore.kernel.org/r/Yt%2BnOeLMqRxjObbx@zn.tnic
> 
> Please incorporate all review comments before sending a new version of
> your patch.
> 
> Ignoring review feedback is a very unfriendly thing to do:
> 
> - if you agree with the feedback, you work it in in the next revision
> 
> - if you don't agree, you *say* *why* you don't

Sorry, it was not my intention. I misread your comment and focused on
build issues around the option.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov