| Message ID | 1427325637-14345-5-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <gwshan@linux.vnet.ibm.com> > --- > 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, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gwshan@linux.vnet.ibm.com> >> --- >> 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. Thanks, Gavin >Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gwshan@linux.vnet.ibm.com> > > --- > > 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, Yeah, since they're already exported, these can't be just removed, but should be redefined in terms of the new exported EEH defines. I also think this should be folded into 1/1.
On Thu, Mar 26, 2015 at 12:01:57PM +1100, David Gibson 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 <gwshan@linux.vnet.ibm.com> >> > --- >> > 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, > >Yeah, since they're already exported, these can't be just removed, but >should be redefined in terms of the new exported EEH defines. > I just explained to Alex.W with something as follows. Are you sure to keep this set of defines in vfio.h? That way, the EEH error constants are all defined in uapi/asm/eeh.h, but the EEH PE state constatns will be distributed in vfio.h and uapi/asm/eeh.h at the same time. Actually, I believe it's safe to move the PE state defines from vfio.h to uapi/asm/eeh.h and now is the right time to do so :) --- 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. >I also think this should be folded into 1/1. > The reason I didn't fold it to PATCH[1/4]: I was afraid the changs will be taken via different trees (ppc-next and vfio-next). Thanks, Gavin >-- >David Gibson | I'll have my music baroque, and my code >david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! >http://www.ozlabs.org/~dgibson -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 26, 2015 at 11:59:03AM +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 <gwshan@linux.vnet.ibm.com> > >> --- > >> 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. That's a good point, actually. This seems like a reasonable risk under the cirucmstances to me.
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 <gwshan@linux.vnet.ibm.com> > >> --- > >> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 25, 2015 at 07:55:41PM -0600, Alex Williamson wrote: >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 <gwshan@linux.vnet.ibm.com> >> >> --- >> >> 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, > Right, it's the point to keep them. I'll simply drop PATCH[4/4] and amend the commit log of PATCH[1/4] in next revision, which will be sent out soon. Thanks, Gavin >Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
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 <gwshan@linux.vnet.ibm.com> --- include/uapi/linux/vfio.h | 5 ----- 1 file changed, 5 deletions(-)