Re: [RFCv2 06/37] s390: add (non)secure page access exceptions handlers

[Thomas Huth <thuth@xxxxxxxxxx>]

On 04/02/2020 12.41, Claudio Imbrenda wrote:
> On Tue, 4 Feb 2020 11:37:42 +0100
> Thomas Huth <thuth@xxxxxxxxxx> wrote:
> 
> [...]
> 
>>> ---
>>>  arch/s390/kernel/pgm_check.S |  4 +-
>>>  arch/s390/mm/fault.c         | 87
>>> ++++++++++++++++++++++++++++++++++++ 2 files changed, 89
>>> insertions(+), 2 deletions(-)  
>> [...]
>>> +void do_non_secure_storage_access(struct pt_regs *regs)
>>> +{
>>> +	unsigned long gaddr = regs->int_parm_long &
>>> __FAIL_ADDR_MASK;
>>> +	struct gmap *gmap = (struct gmap *)S390_lowcore.gmap;
>>> +	struct uv_cb_cts uvcb = {
>>> +		.header.cmd = UVC_CMD_CONV_TO_SEC_STOR,
>>> +		.header.len = sizeof(uvcb),
>>> +		.guest_handle = gmap->se_handle,
>>> +		.gaddr = gaddr,
>>> +	};
>>> +	int rc;
>>> +
>>> +	if (get_fault_type(regs) != GMAP_FAULT) {
>>> +		do_fault_error(regs, VM_READ | VM_WRITE,
>>> VM_FAULT_BADMAP);
>>> +		WARN_ON_ONCE(1);
>>> +		return;
>>> +	}
>>> +
>>> +	rc = uv_make_secure(gmap, gaddr, &uvcb, 0);
>>> +	if (rc == -EINVAL && uvcb.header.rc != 0x104)
>>> +		send_sig(SIGSEGV, current, 0);
>>> +}  
>>
>> What about the other rc beside 0x104 that could happen here? They go
>> unnoticed?
> 
> no, they are handled in the uv_make_secure, and return an appropriate
> error code. 
Hmm, in patch 05/37, I basically see:

+static int make_secure_pte(pte_t *ptep, unsigned long addr, void *data)
+{
[...]
+	rc = uv_call(0, (u64)params->uvcb);
+	page_ref_unfreeze(page, expected);
+	if (rc)
+		rc = (params->uvcb->rc == 0x10a) ? -ENXIO : -EINVAL;
+	return rc;
+}

+int uv_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb,
int pins)
+{
[...]
+	lock_page(params.page);
+	rc = apply_to_page_range(gmap->mm, uaddr, PAGE_SIZE, make_secure_pte,
&params);
+	unlock_page(params.page);
+out:
+	up_read(&gmap->mm->mmap_sem);
+
+	if (rc == -EBUSY) {
+		if (local_drain) {
+			lru_add_drain_all();
+			return -EAGAIN;
+		}
+		lru_add_drain();
+		local_drain = 1;
+		goto again;
+	} else if (rc == -ENXIO) {
+		if (gmap_fault(gmap, gaddr, FAULT_FLAG_WRITE))
+			return -EFAULT;
+		return -EAGAIN;
+	}
+	return rc;
+}

So 0x10a result in -ENXIO and is handled ==> OK.
And 0x104 is handled in do_non_secure_storage_access ==> OK.

But what about the other possible error codes? make_secure_pte() returns
-EINVAL in that case, but uv_make_secure() does not care about that
error code, and do_non_secure_storage_access() only cares if
uvcb.header.rc was 0x104 ... what did I miss?

 Thomas