From:   Thomas Gleixner <tglx@linutronix.de>
To:     Paolo Bonzini <pbonzini@redhat.com>, isaku.yamahata@intel.com,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, erdemaktas@google.com,
        Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc:     isaku.yamahata@gmail.com, Xiaoyao Li <xiaoyao.li@intel.com>
Subject: Re: [RFC PATCH v3 54/59] KVM: X86: Introduce initial_tsc_khz in
 struct kvm_arch
In-Reply-To: <741df444-5cd0-2049-f93a-c2521e4f426d@redhat.com>
References: <cover.1637799475.git.isaku.yamahata@intel.com>
 <5ba3573c8b82fcbdc3f3994f6d4d2a3c40445be9.1637799475.git.isaku.yamahata@intel.com>
 <875ysghrp8.ffs@tglx> <741df444-5cd0-2049-f93a-c2521e4f426d@redhat.com>
Date:   Fri, 26 Nov 2021 00:26:45 +0100
Message-ID: <87tufzhl56.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Paolo,

On Thu, Nov 25 2021 at 23:13, Paolo Bonzini wrote:
> On 11/25/21 22:05, Thomas Gleixner wrote:
> If there are some patches that are actually independent, go ahead and 
> submit them early.  But more practically, for the bulk of the changes 
> what you need to do is:
>
> 1) incorporate into patch 55 a version of tdx.c that essentially does 
> KVM_BUG_ON or WARN_ON for each function.  Temporarily keep the same huge 
> patch that adds the remaining 2000 lines of tdx.c

There is absolutely no reason to populate anything upfront at all.

Why?

Simply because that whole muck cannot work until all pieces are in place.

That's the classic PoC vs. usable code situation. I.e. you know what
needs to be done from a PoC point of view and you have to get there by
building up the infrastructure piece by piece.

So why would you provide:

handle_A(...) { BUG(); }
..
handle_Z(...) { BUG(); }

with all the invocations in the common emulation dispatcher:

  handle_stuff(reason)
  {
        switch(reason) {
        case A: return handle_A();
        ...
        case Z: return handle_Z();
        default: BUG();
        }
  };

in the first place instead of providing:

 1)
        handle_stuff(reason)
        {
                switch(reason) {
                default: BUG();
                };
        }

 2)
        handle_A(...)
        {
                return do_something_understandable()
        }
        
        handle_stuff(reason)
        {
                switch(reason) {
                case A: return handle_A();
                default: BUG();
        }

 $Z)
        handle_Z(...)
        {
                return do_something_understandable()
        }
        
        handle_stuff(reason)
        {
                switch(reason) {
                case A: return handle_A();
                ...
                case Z: return handle_Z();
                default: BUG();
        }

where each of (1..$Z) is a single patch doing exactly _ONE_ thing at a
time and if there is some extra magic required like an additional export
for handle_Y() then this export patch is either part of the patch
dealing with $Y or just before that.

In both scenarious you cannot boot a TDX guest until you reached $Z, but
in the gradual one you and the reviewers have the pleasure of looking at
one thing at a time.

No?

Thanks,

        tglx