Re: [PATCH] zcrypt: handle AP Info notification from CHSC SEI command

[Tony Krowiak <akrowiak@linux.ibm.com>]

On 2/4/19 5:15 AM, Harald Freudenberger wrote:
> On 01.02.19 15:35, Cornelia Huck wrote:
>> On Thu, 31 Jan 2019 18:50:57 -0500
>> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>>
>>> On 1/31/19 4:55 AM, Cornelia Huck wrote:
>>>> On Wed, 30 Jan 2019 12:48:46 -0500
>>>> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>>>> Two questions:
>>>> - Does the event cover _any_ change to the AP configuration, or can the
>>>>     periodic scan detect changes that are not signaled?
>>> It can detect any change, such as a change to the CRYCB masks.
>> Nice. I suppose we can not rely on those messages being generated,
>> though, and therefore need to keep the periodic scan...
> As you wrote, I am not sure if the ap bus code can rely on this to
> cover all changes. For kvm guests I think it is currently not working
> as there is no such notification generated at all. So I'd like to
> have the periodic scan in place.

I verified that a dynamic change to the CRYCB (via the SE) is not
signaled by CHSC Adjunct Processor changed notification. Apparently only
configuration changes are signaled, so we will definitely need to keep
the scan unless somebody knows otherwise.

Tony K

>>
>>>> - Do we want to generate such an event in QEMU on plugging/unplugging
>>>>     the vfio-ap device?
>>> We've discussed this quite a bit internally and decided not to implement
>>> that at this time. We will address it as a future enhancement.
>> Ok, but I think it would be nice to have.
>>
>>>>> diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
>>>>> index a0baee25134c..dccccc337078 100644
>>>>> --- a/drivers/s390/cio/chsc.c
>>>>> +++ b/drivers/s390/cio/chsc.c
>>>>> @@ -586,6 +586,15 @@ static void chsc_process_sei_scm_avail(struct chsc_sei_nt0_area *sei_area)
>>>>>    			      " failed (rc=%d).\n", ret);
>>>>>    }
>>>>>    
>>>>> +static void chsc_process_sei_ap_cfg_chg(struct chsc_sei_nt0_area *sei_area)
>>>>> +{
>>>>> +	CIO_CRW_EVENT(3, "chsc: ap config changed\n");
>>>>> +	if (sei_area->rs != 5)
>>>>> +		return;
>>>> I'm guessing that a reporting source of 5 means ap, right? (The code is
>>>> silent on all those magic rs values :/)
>>> The 5 indicates the accessibility of one or more adjunct processors has
>>> changed. The reason this gets called is because the CC sent with the
>>> instruction indicates the AP configuration has changed, so the reporting
>>> belongs where it is. There is only one RS associated with it.
>> So if we'd ever get there anything but rs == 5, it would be a hardware
>> or hypervisor bug? Then the code makes sense, I guess.
>>
>>>> If so, should the debug logging be moved after the check?
>>> covered in the response above.
>>>
>>>>    
>>>>> +
>>>>> +	ap_bus_cfg_chg();
>>>>> +}
>>>>> +
>