On Tue, 4 Feb 2020 13:48:47 +0100
Thomas Huth <thuth@xxxxxxxxxx> wrote:
> 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, ¶ms);
> + 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?
basically, any error value that is not handled by uv_make_secure is
passed as-is from make_secure_pte directly to the caller of
uv_make_secure .
any RC value that is not explicitly handled here will
result in -EINVAL. The caller has then to check for -EINVAL and check
the RC value, like do_non_secure_storage_access does.
so anything else that goes wrong is passed as-is to the caller.
for some things, like interrupt handlers, we simply don't care; if we
need to try again, we will try again, if we notice we can't continue,
the VM will get killed.
