Re: [Qemu-devel] [PATCH v5 4/6] s390x/vfio: ap: Introduce VFIO AP device

[Pierre Morel <pmorel@linux.ibm.com>]

On 15/05/2018 17:09, Tony Krowiak wrote:
> On 05/15/2018 03:55 AM, Pierre Morel wrote:
>> On 14/05/2018 21:26, Tony Krowiak wrote:
>>> On 05/11/2018 05:02 AM, Pierre Morel wrote:
>>>> On 10/05/2018 15:10, Tony Krowiak wrote:
>>>>> On 05/09/2018 10:28 AM, Halil Pasic wrote:
>>>>>>
>>>>>>
>>>>>> On 05/08/2018 02:25 PM, Tony Krowiak wrote:
>>>>>>> Introduces a VFIO based AP device. The device is defined via
>>>>>>> the QEMU command line by specifying:
>>>>>>>
>>>>>>>      -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
>>>>>>>
>>>>>>> There may be only one vfio-ap device configured for a guest.
>>>>>>>
>>>>>>> The mediated matrix device is created by the VFIO AP device
>>>>>> [..]
>>>>>>> + * directory.
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <linux/vfio.h>
>>>>>>> +#include <sys/ioctl.h>
>>>>>>> +#include "qemu/osdep.h"
>>>>>>> +#include "qapi/error.h"
>>>>>>> +#include "hw/sysbus.h"
>>>>>>> +#include "hw/vfio/vfio.h"
>>>>>>> +#include "hw/vfio/vfio-common.h"
>>>>>>> +#include "hw/s390x/ap-device.h"
>>>>>>> +#include "qemu/error-report.h"
>>>>>>> +#include "qemu/queue.h"
>>>>>>> +#include "qemu/option.h"
>>>>>>> +#include "qemu/config-file.h"
>>>>>>> +#include "cpu.h"
>>>>>>> +#include "kvm_s390x.h"
>>>>>>> +#include "sysemu/sysemu.h"
>>>>>>> +
>>>>>>> +#define VFIO_AP_DEVICE_TYPE      "vfio-ap"
>>>>>>> +
>>>>>>> +typedef struct VFIOAPDevice {
>>>>>>> +    APDevice apdev;
>>>>>>> +    VFIODevice vdev;
>>>>>>> +    QTAILQ_ENTRY(VFIOAPDevice) sibling;
>>>>>>> +} VFIOAPDevice;
>>>>>>> +
>>>>>>> +VFIOAPDevice *vfio_apdev;
>>>>>>> +
>>>>>>> +static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
>>>>>>> +{
>>>>>>> +    vdev->needs_reset = false;
>>>>>>> +}
>>>>>>> +
>>>>>>> +/*
>>>>>>> + * We don't need vfio_hot_reset_multi and vfio_eoi operations for
>>>>>>> + * vfio-ap-matrix device now.
>>>>>>> + */
>>>>>>> +struct VFIODeviceOps vfio_ap_ops = {
>>>>>>> +    .vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
>>>>>>> +};
>>>>>>> +
>>>>>>
>>>>>> I'm not familiar with the vfio infrastructure, but AFAIR I
>>>>>> haven't seen any substantial reset handling (QEMU or kernel).
>>>>>> Did I miss something?
>>>>>
>>>>> No, you didn't miss anything, there is no reset handling.
>>>>>
>>>>>>
>>>>>> If I did not. I think this is a big problem. We need to at least
>>>>>> zeroize the queues (e.g. on system reset)  to avoid leaking
>>>>>> sensitive information. Without this, there is no sane way to use
>>>>>> ap-passthrough. Or am I wrong?
>>>>>
>>>>> I do not have a definitive answer, I will have to look into it.
>>>>> I'm thinking that since we are using ap-passthrough, the AP bus
>>>>> running on the guest would be responsible for handling reset possibly
>>>>> by resetting or zeroizing its queues. I'll get back to you on this.
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Halil
>>>>>>
>>>>>
>>>> On the host, on system reset or subsystem reset, the queues are 
>>>> zeroized.
>>>
>>> Can you point me to where this is done?
>>
>> This is firmware. See documentation, it is specified that the queues 
>> are zeroized on system or subsystem reset.
>
> The sticking point for me is how does this relate to the guest running 
> under SIE?

No it has nothing to do with the SIE, therefor we must do it explicitly.

Regards,

Pierre


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany