RE: [PATCH v2 02/13] vfio_pci: refine user config reference in vfio-pci module

From: Liu, Yi L
Date: Mon Sep 30 2019 - 08:39:12 EST


Hi Alex,

> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
> Sent: Thursday, September 26, 2019 10:36 AM
> To: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Subject: Re: [PATCH v2 02/13] vfio_pci: refine user config reference in vfio-pci
> module
>
> On Thu, 5 Sep 2019 15:59:19 +0800
> Liu Yi L <yi.l.liu@xxxxxxxxx> wrote:
>
> > This patch adds three fields in struct vfio_pci_device to pass the user
> > configs of vfio-pci module to some functions which could be common in
> > future usage.
> >
> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Cc: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> > ---
> > drivers/vfio/pci/vfio_pci.c | 24 +++++++++++++++---------
> > drivers/vfio/pci/vfio_pci_private.h | 9 +++++++--
> > 2 files changed, 22 insertions(+), 11 deletions(-)
>
> A subtle behavioral difference here is that disable_idle_d3 and
> nointxmask are runtime modifiable parameters, if the value is changed
> in sysfs the device will adopt the new behavior on its next
> transition. After this patch, each device operates in the mode defined
> at the time it was probed. Should we maybe refresh the value at key
> points, like the user opening or releasing the device so that it tracks
> the module parameter? I think we could defend not changing the
> behavior of a device while it's in use by a user. Otherwise we might
> want to make the module parameter read-only to avoid the
> inconsistency.

Agreed. I think we can take such assumption that the changing is not
allowed during an open/release cycle. Let me add the updates in the
next version.

>
> >
[...]
> > + vfio_pci_fill_ids(&ids[0]);
>
> Or just 'ids'. Thanks,

yes, let me fix it.

> Alex
>

Regards,
Yi Liu