Re: [PATCH v9 3/3] x86/sgx: Fine grained SGX MCA behavior for virtualization

From: "Huang, Kai" <kai.huang@intel.com>
To: "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"Luck, Tony" <tony.luck@intel.com>,
	"Li, Zhiquan1" <zhiquan1.li@intel.com>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"jarkko@kernel.org" <jarkko@kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>
Cc: "tglx@linutronix.de" <tglx@linutronix.de>,
	"Du, Fan" <fan.du@intel.com>, "Christopherson,,
	Sean" <seanjc@google.com>, "Zhang, Cathy" <cathy.zhang@intel.com>,
	"bp@suse.de" <bp@suse.de>
Subject: Re: [PATCH v9 3/3] x86/sgx: Fine grained SGX MCA behavior for virtualization
Date: Thu, 13 Oct 2022 23:40:25 +0000	[thread overview]
Message-ID: <c8e7cff6f8d66ce4f8575caaf69f02b491ed7d90.camel@intel.com> (raw)
In-Reply-To: <4930999a-888f-88bc-a05c-86762504f059@intel.com>

On Thu, 2022-10-13 at 15:28 -0700, Dave Hansen wrote:
> On 10/13/22 15:15, Huang, Kai wrote:
> > On Thu, 2022-10-13 at 15:02 -0700, Hansen, Dave wrote:
> > > Specifically, with this machine check SIGBUS implementation, one EPC
> > > page can only have on host virtual address.  But, it's possible to
> > > mmap() a single VEPC page in multiple processes at multiple host virtual
> > > addresses.
> > > 
> > > So, the implementation only allows one host virtual address.  But, we
> > > can imagine a scenario where there are *TWO* valid virtual addresses
> > > where the VEPC is mapped.
> > > 
> > > What might happen in this case?  What's the fallout?
> > 
> > My understanding is there can be two #MC happening on two virtual addresses.
> > 
> > Each #MC will be injected to the VM which triggers that #MC to handle.
> 
> OK but which address with each of them see in siginfo->si_addr?  There's
> only one ->vepc_vaddr.
> 
> It seems to me like this might result in the siginfo getting populated
> with the ->si_addr from the *OTHER* process and *OTHER* virtual address.
>  Basically, whoever allocates the page sets up *their* virtual address.
>  Anyone else who gets a machine check on that page will see a virtual
> address that has nothing to do with its virtual address space.
> 
> Is that problematic?

Thanks Dave.  I see the problem now.  I already forgot the reason that I said
"whether to add a comment is good enough" in my original reply.

So with allowing fork(), putting the 'virtual address' to the owner doesn't
seems right.  I think perhaps we have two options:

1) to enforce in kernel that virtual EPC doesn't support fork(), and page's
owner can be used to carry 'virtual address'.
2) in arch_memory_failure, we find out the virtual address based on the current
process, but I am not entirely sure whether it can work, i.e. it's not called
from interrupt context?

What's your suggestion?