Re: [PATCH v2 3/4] s390x/pv: Introduce a s390_pv_check() helper for runtime

[Janosch Frank]

On 1/9/23 10:27, Cédric Le Goater wrote:
> On 1/9/23 10:04, Janosch Frank wrote:
> > On 1/6/23 08:53, Cédric Le Goater wrote:
> > > From: Cédric Le Goater <clg@redhat.com>
> > >
> > > If a secure kernel is started in a non-protected VM, the OS will hang
> > > during boot without giving a proper error message to the user.
> >
> > Most of the time you see nothing in the console because libvirt is too slow. If 
> > you start the VM in paused mode, attach a console and then resume it, then 
> > you'll see a nice error message.
>
> If you wait long enough, the VM fails to mount / and falls into the dracut
> initrams.

I have the feeling that we're not talking about the same thing here.

A PV VM always starts out as a non-PV VM and is put into PV mode via two 
diag308 subcodes (8 & 10). ALL PV subcodes (8 - 10) are spec exceptions 
if the host isn't enabled for PV.

There is no way to run a secure image in a non-PV environment. What's 
being run is a non-secure bootloader that initiates the switch into 
secure mode.

Either the switch fails and we return with DIAG_308_RC_INVAL_FOR_PV to 
the non-PV bootloader or we start running in PV mode and __never__ 
return to the bootloader without a reboot.

> > > Perform the checks on Confidential Guest support at runtime with an
> > > helper called from the service call switching the guest to protected
> > > mode.
> >
> > If we don't have PV support then the subcodes >=8 are a specification exception
> > so this is never executed AFAIK.
>
> It is. The test on huge page size was added just above this change.

And the huge page test only applies if the PV feature is in the cpumodel 
which also means that the host is PV enabled since it's based on the 
capability:

if (subcode >= DIAG308_PV_SET && !s390_has_feat(S390_FEAT_UNPACK)) {
        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
        return;
}


> Thanks,
>
> C.
>
> > >        if (env->psw.mask & PSW_MASK_PSTATE) {
> > >            s390_program_interrupt(env, PGM_PRIVILEGED, ra);
> > > @@ -176,6 +177,12 @@ out:
> > >                return;
> > >            }
> > > +        if (!s390_pv_check(&local_err)) {
> > > +            error_report_err(local_err);
> > > +            env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;
> > > +            return;
> > > +        }
> > > +
> > >            s390_ipl_reset_request(cs, S390_RESET_PV);
> > >            break;
> > >        default: