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

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

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.

It means we must do the same for the guest and implement both cold and 
hot reset.

One of the patches in the interrupt handling serie I sent last wednesday 
zeroize
the queues on starting and stopping the guest so no data leak occurs 
between
host and guests or between guests.

However we should follow the machine specification and zeroize the 
queues between
two guest reset too. i.e. in the "reset" call back. and implement the 
VFIO_DEVICE_RESET
ioctl.


Regards,

Pierre

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