On 2022/10/11 07:20, Dave Hansen wrote:
> On 9/19/22 23:39, Zhiquan Li wrote:
>> + if (flags & MF_ACTION_REQUIRED) {
>> + /*
>> + * Provide extra info to the task so that it can make further
>> + * decision but not simply kill it. This is quite useful for
>> + * virtualization case.
>> + */
>
> "Quite useful to the virtualization case", eh?
>
> This code can only even be *run* in the virtualization case.
>
OK, I'll change it in V10.
>> + if (page->flags & SGX_EPC_PAGE_KVM_GUEST) {
>> + /*
>> + * The 'encl_owner' field is repurposed, when allocating EPC
>> + * page it was assigned to the virtual address of virtual EPC
>> + * page.
>> + */
>
> That comment is talking about history, which is kinda silly. Let's just
> focus on what the code is doing today.
>
How about move the comment above to here? Because it's talking about
what the code is doing today.
Then this comment can be removed.
>> + vaddr = (void *)((unsigned long)page->vepc_vaddr & PAGE_MASK);
>
> First, why even align this? What does it matter?
>
> Second, is there a reason not to use PTR_ALIGN() here?
>
You're right. The align is unnecessary, host kernel just reports the
error address, no matter if it's aligned, guest kernel MCA will convert
it to PFN.
Best Regards,
Zhiquan
>> + ret = force_sig_mceerr(BUS_MCEERR_AR, vaddr, PAGE_SHIFT);
>> + if (ret < 0)
>> + pr_err("Memory failure: Error sending signal to %s:%d: %d\n",
>> + current->comm, current->pid, ret);
>> + } else
>> + force_sig(SIGBUS);
>> + }
>
