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

[Janosch Frank <frankja@linux.ibm.com>]

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.