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;
MIME-Version: 1.0
References: <20230724230329.3970-1-kirill.shutemov@linux.intel.com>
In-Reply-To: <20230724230329.3970-1-kirill.shutemov@linux.intel.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 25 Jul 2023 14:57:35 -0700
Message-ID: <CAHk-=wipev18s9sErt+iNO_rzgyvGTce38fr1CYO0U_hGVGy2Q@mail.gmail.com>
Subject: Re: [PATCH] x86/traps: Fix load_unaligned_zeropad() handling for
 shared TDX memory
To:     "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc:     dave.hansen@intel.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, x86@kernel.org, linux-kernel@vger.kernel.org,
        Michael Kelley <mikelley@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 24 Jul 2023 at 16:03, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
>
> Commit c4e34dd99f2e ("x86: simplify load_unaligned_zeropad()
> implementation") changes how exceptions around load_unaligned_zeropad()
> handled. Kernel now relies on fault_address in fixup_exception() to
> detect that the exception happened due to load_unaligned_zeropad().
>
> It works fine for #PF, but breaks on #VE since no fault address is
> passed down to fixup_exception().
>
> Propagating ve_info.gla down to fixup_exception() resolves the issue.

Ahh.

The faulting address was really only used as a sanity check, so I
guess another option would have been to just remove it.

We do need to get the "real" address anyway just to get the right
offset, and that comes from

        addr = (unsigned long) insn_get_addr_ref(&insn, regs);

and all we do is to then check they "yep, the faulting address is the
start of the next aligned word".

The faulting address in itself was never sufficient, since it has lost
the original offset in the previous page, and typically just points to
the first word in the next page.

That said, your patch looks fine to me, even if it might have been
easier to just remove the fault_addr thing entirely. And the
fault_addr verification is nice in that it validates our instruction
decoder result, so maybe it is all better this way.

I can apply this directly (having written that ex_handler_zeropad())
or wait for it to come from the x86 tree?

                 Linus