Subject: Re: [PATCH v4 4/4] drivers/vfio: Remove duplicated PE states

From: Alex Williamson <[email-masked]>
Date: Wed, 25 Mar 2015 19:55:41 -0600

On Thu, 2015-03-26 at 11:59 +1100, Gavin Shan wrote:
> On Wed, Mar 25, 2015 at 06:46:28PM -0600, Alex Williamson wrote:
> >On Thu, 2015-03-26 at 10:20 +1100, Gavin Shan wrote:
> >> The set of constants for PE states defined in uapi/linux/vfio.h is
> >> duplicated to uapi/asm/eeh.h. The patch removes the set from the
> >> former.
> >> 
> >> Signed-off-by: Gavin Shan <[email-masked]>
> >> ---
> >>  include/uapi/linux/vfio.h | 5 -----
> >>  1 file changed, 5 deletions(-)
> >> 
> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >> index d81c17f..3fd1e86 100644
> >> --- a/include/uapi/linux/vfio.h
> >> +++ b/include/uapi/linux/vfio.h
> >> @@ -492,11 +492,6 @@ struct vfio_eeh_pe_op {
> >>  #define VFIO_EEH_PE_UNFREEZE_IO		2	/* Enable IO for frozen PE   */
> >>  #define VFIO_EEH_PE_UNFREEZE_DMA	3	/* Enable DMA for frozen PE  */
> >>  #define VFIO_EEH_PE_GET_STATE		4	/* PE state retrieval        */
> >> -#define  VFIO_EEH_PE_STATE_NORMAL	0	/* PE in functional state    */
> >> -#define  VFIO_EEH_PE_STATE_RESET	1	/* PE reset in progress      */
> >> -#define  VFIO_EEH_PE_STATE_STOPPED	2	/* Stopped DMA and IO        */
> >> -#define  VFIO_EEH_PE_STATE_STOPPED_DMA	4	/* Stopped DMA only          */
> >> -#define  VFIO_EEH_PE_STATE_UNAVAIL	5	/* State unavailable         */
> >>  #define VFIO_EEH_PE_RESET_DEACTIVATE	5	/* Deassert PE reset         */
> >>  #define VFIO_EEH_PE_RESET_HOT		6	/* Assert hot reset          */
> >>  #define VFIO_EEH_PE_RESET_FUNDAMENTAL	7	/* Assert fundamental reset  */
> >
> >How do you know that nobody depends on these defines?  I thought the
> >suggestion was to use the EEH_* defines for error injection, not to
> >remove existing VFIO_EEH_* defines.  You could certainly redefine these
> >in terms of EEH_* defines instead.  Thanks,
> >
> 
> QEMU should be the first user to utilize the EEH capability exposed by
> the host kernel, and I believe QEMU doesn't use those constants yet.
> So it's right time to move those constants to uapi/asm/eeh.h. Once some
> one starts to use them, it's impossible to do so.

There are soon to be four kernel versions out there with these defines,
you can't be sure that nobody has already or won't in the future do
VFIO/EEH development on those kernels.  The defines need to stay IMHO.
Thanks,

Alex