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;
Date:   Thu, 26 May 2022 23:36:19 +0300
From:   "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
To:     Dave Hansen <dave.hansen@intel.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        luto@kernel.org, peterz@infradead.org, ak@linux.intel.com,
        dan.j.williams@intel.com, david@redhat.com, hpa@zytor.com,
        linux-kernel@vger.kernel.org,
        sathyanarayanan.kuppuswamy@linux.intel.com, seanjc@google.com,
        thomas.lendacky@amd.com, x86@kernel.org
Subject: Re: [PATCHv3 3/3] x86/tdx: Handle load_unaligned_zeropad()
 page-cross to a shared page
Message-ID: <20220526203619.gpyyl67ygk622e5g@black.fi.intel.com>
References: <20220524221012.62332-1-kirill.shutemov@linux.intel.com>
 <20220524221012.62332-4-kirill.shutemov@linux.intel.com>
 <6350b927-4b3f-6f43-aa62-f8db19fa8d5b@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6350b927-4b3f-6f43-aa62-f8db19fa8d5b@intel.com>
Precedence: bulk

On Thu, May 26, 2022 at 09:20:56AM -0700, Dave Hansen wrote:
> On 5/24/22 15:10, Kirill A. Shutemov wrote:
> > +	/*
> > +	 * MMIO accesses suppose to be naturally aligned and therefore never
> > +	 * cross a page boundary. Seeing unaligned accesses indicates a bug or
> > +	 * load_unaligned_zeropad() that steps into unmapped shared page.
> 
> Wait a sec though...
> 
> We've been talking all along about how MMIO accesses are in some cases
> just plain old compiler-generated memory accesses.  It's *probably* bad
> code that does this, but it's not necessarily a bug.

Compiler-generated memory accesses tend to be aligned too. You need to do
something make them unalinged, like __packed or pointer trickery.

> It's kinda like the split lock detection patches.  Those definitely
> found some stupid stuff, but it wasn't anything that I would have called
> an outright bug.  Plus, in those cases, folks had explicitly opted in to
> more crashes on stupid stuff.
> 
> That stupid stuff _might_ be rare enough that it's still OK to just punt
> on it and not emulate the instruction (aka. crash).  Or, to say that TDX
> guests are opting in to being more fragile, just like with split lock
> detection.

I think it is reasonable to expect that TDX user value its data security
higher than uptime. 

And I'm not sure that compare unaligned MMIO access to split-lock is fair.
Split-lock is performance hit, but semantics is defined. In unalgined MMIO
case, I think the behaviour is not defined: it is not clear what memory
reqested should be issued on the memory bus in case of byte-algined 4-byte
access. It can make a difference on device side.

> But, either of those would call for a very different comment.

Fair enough.

-- 
 Kirill A. Shutemov