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

[Janosch Frank]

On 1/17/23 10:09, Thomas Huth wrote:
> On 17/01/2023 09.40, Janosch Frank wrote:
> > On 1/16/23 18:46, 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.
> >
> > Didn't we establish that you were missing the IOMMU flag so this statement
> > isn't correct anymore?
> >
> >
> > I haven't yet fully ingested my coffee, but from what I understand you would
> > block a switch into PV mode if cgs is not set. Which would mean that PV KVM
> > unit tests wouldn't start anymore as well as any VMs that have the unpack
> > feature but not cgs.
> >
> > And that's not something that we want.
> >
> > You can start a PV VM without cgs if unpack is in the CPU model. The ONLY
> > requirement that we should fail on is unpack.
>
> So would it make sense to check for S390_FEAT_UNPACK (or something else?)
> here, or should the patch completely be dropped instead?

Subcodes 8 - 10 already result in a spec PGM for !unpack and if memory 
serves me right then Marc will catch that and print a warning in the 
genprotimg kernel:

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

So other than shuffling code around I see no benefit in this patch.