On 05.03.20 13:24, Janosch Frank wrote:
> On 3/5/20 1:04 PM, Janosch Frank wrote:
>> On 3/4/20 6:04 PM, David Hildenbrand wrote:
>>> On 04.03.20 12:42, Janosch Frank wrote:
>>>> For diag308 subcodes 8 - 10 we have a new ipib of type 5. The ipib
>>>> holds the address and length of the secure execution header, as well
>>>> as a list of guest components.
>>>>
>>>> Each component is a block of memory, for example kernel or initrd,
>>>> which needs to be decrypted by the Ultravisor in order to run a
>>>> protected VM. The secure execution header instructs the Ultravisor on
>>>> how to handle the protected VM and its components.
>>>>
>>>> Subcodes 8 and 9 are similiar to 5 and 6 and subcode 10 will finally
>>>> start the protected guest.
>>>>
>>>> Subcodes 8-10 are not valid in protected mode, we have to do a subcode
>>>> 3 and then the 8 and 10 combination for a protected reboot.
>>>>
>>>> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
>>>> ---
>>>> hw/s390x/ipl.c | 47 ++++++++++++++++++++++++++++++++++++++++++---
>>>> hw/s390x/ipl.h | 32 ++++++++++++++++++++++++++++++
>>>> target/s390x/diag.c | 26 ++++++++++++++++++++++---
>>>> 3 files changed, 99 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>>>> index 9c1ecd423c..80c6ab233a 100644
>>>> --- a/hw/s390x/ipl.c
>>>> +++ b/hw/s390x/ipl.c
>>>> @@ -538,15 +538,55 @@ static bool is_virtio_scsi_device(IplParameterBlock
>>>> *iplb)
>>>> return is_virtio_ccw_device_of_type(iplb, VIRTIO_ID_SCSI);
>>>> }
>>>>
>>>> +int s390_ipl_pv_check_components(IplParameterBlock *iplb)
>>>
>>> What about making this
>>>
>>> bool s390_ipl_pv_valid(IplParameterBlock *iplb)
>>>
>>> and return true/false?
>>
>> We already have iplb_valid_pv() and ipl->iplb_valid_pv.
>> Do you have any other more expressive name we could use?
>
> I think it makes more sense to rip out these tiny functions and
> consolidate them like this:
>
> +static inline bool iplb_valid(IplParameterBlock *iplb)
> {
> - return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> - iplb->pbt == S390_IPL_TYPE_FCP;
> + switch (iplb->pbt) {
> + case S390_IPL_TYPE_FCP:
> + return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> + iplb->pbt == S390_IPL_TYPE_FCP);
> + case S390_IPL_TYPE_CCW:
> + return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN &&
> + iplb->pbt == S390_IPL_TYPE_CCW);
> + case S390_IPL_TYPE_PV:
> + if(be32_to_cpu(iplb->len) < S390_IPLB_MIN_PV_LEN ||
> + iplb->pbt != S390_IPL_TYPE_PV) {
> + return false;
> + }
> + return s390_ipl_pv_check_components(iplb);
yeah, and maybe even inline s390_ipl_pv_check_components().
--
Thanks,
David / dhildenb
