1:46 a.m.
From: Ramesh Thomas <ramesh.thomas(a)intel.com>
Duplicate the driver error code related constants defined in idxd.h in
library to avoid dependency on kernel versions.
Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
---
accfg/lib/libaccfg.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
index f21cab5..1a06d9c 100644
--- a/accfg/lib/libaccfg.c
+++ b/accfg/lib/libaccfg.c
@@ -27,7 +27,6 @@
#include <accfg/libaccel_config.h>
#include <fnmatch.h>
#include "private.h"
-#include <linux/idxd.h>
#define MDEV_POSTFIX "mdev_supported_types"
#define IDXD_DRIVER_BIND_PATH "/sys/bus/dsa/drivers/idxd"
@@ -74,6 +73,29 @@ enum {
ACCFG_CMD_STATUS_ERROR = 0x80010000,
};
+/* driver error codes from kernel idxd.h */
+enum idxd_scmd_stat {
+ IDXD_SCMD_DEV_ENABLED = 0x80000010,
+ IDXD_SCMD_DEV_NOT_ENABLED = 0x80000020,
+ IDXD_SCMD_WQ_ENABLED = 0x80000021,
+ IDXD_SCMD_DEV_DMA_ERR = 0x80020000,
+ IDXD_SCMD_WQ_NO_GRP = 0x80030000,
+ IDXD_SCMD_WQ_NO_NAME = 0x80040000,
+ IDXD_SCMD_WQ_NO_SVM = 0x80050000,
+ IDXD_SCMD_WQ_NO_THRESH = 0x80060000,
+ IDXD_SCMD_WQ_PORTAL_ERR = 0x80070000,
+ IDXD_SCMD_WQ_RES_ALLOC_ERR = 0x80080000,
+ IDXD_SCMD_PERCPU_ERR = 0x80090000,
+ IDXD_SCMD_DMA_CHAN_ERR = 0x800a0000,
+ IDXD_SCMD_CDEV_ERR = 0x800b0000,
+ IDXD_SCMD_WQ_NO_SWQ_SUPPORT = 0x800c0000,
+ IDXD_SCMD_WQ_NONE_CONFIGURED = 0x800d0000,
+ IDXD_SCMD_WQ_NO_SIZE = 0x800e0000,
+};
+
+#define IDXD_SCMD_SOFTERR_MASK 0x80000000
+#define IDXD_SCMD_SOFTERR_SHIFT 16
+
#define SCMD_STAT(x) (((x) & ~IDXD_SCMD_SOFTERR_MASK) >> \
IDXD_SCMD_SOFTERR_SHIFT)
--
2.26.3
3:17 p.m.
On 7/16/2021 6:46 PM, ramesh.thomas(a)intel.com wrote:
From: Ramesh Thomas <ramesh.thomas(a)intel.com>
Duplicate the driver error code related constants defined in idxd.h in
library to avoid dependency on kernel versions.
I think instead of replicating certain defines, the suggestion by Vishal
was to make a copy of the latest kernel uapi idxd.h in accel-config.
This is probably the best. Otherwise you'll have conflicts if this is
build with the latest idxd.h having those defines as well.
>
> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
> ---
> accfg/lib/libaccfg.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
> index f21cab5..1a06d9c 100644
> --- a/accfg/lib/libaccfg.c
> +++ b/accfg/lib/libaccfg.c
> @@ -27,7 +27,6 @@
> #include <accfg/libaccel_config.h>
> #include <fnmatch.h>
> #include "private.h"
> -#include <linux/idxd.h>
>
> #define MDEV_POSTFIX "mdev_supported_types"
> #define IDXD_DRIVER_BIND_PATH "/sys/bus/dsa/drivers/idxd"
> @@ -74,6 +73,29 @@ enum {
> ACCFG_CMD_STATUS_ERROR = 0x80010000,
> };
>
> +/* driver error codes from kernel idxd.h */
> +enum idxd_scmd_stat {
> + IDXD_SCMD_DEV_ENABLED = 0x80000010,
> + IDXD_SCMD_DEV_NOT_ENABLED = 0x80000020,
> + IDXD_SCMD_WQ_ENABLED = 0x80000021,
> + IDXD_SCMD_DEV_DMA_ERR = 0x80020000,
> + IDXD_SCMD_WQ_NO_GRP = 0x80030000,
> + IDXD_SCMD_WQ_NO_NAME = 0x80040000,
> + IDXD_SCMD_WQ_NO_SVM = 0x80050000,
> + IDXD_SCMD_WQ_NO_THRESH = 0x80060000,
> + IDXD_SCMD_WQ_PORTAL_ERR = 0x80070000,
> + IDXD_SCMD_WQ_RES_ALLOC_ERR = 0x80080000,
> + IDXD_SCMD_PERCPU_ERR = 0x80090000,
> + IDXD_SCMD_DMA_CHAN_ERR = 0x800a0000,
> + IDXD_SCMD_CDEV_ERR = 0x800b0000,
> + IDXD_SCMD_WQ_NO_SWQ_SUPPORT = 0x800c0000,
> + IDXD_SCMD_WQ_NONE_CONFIGURED = 0x800d0000,
> + IDXD_SCMD_WQ_NO_SIZE = 0x800e0000,
> +};
> +
> +#define IDXD_SCMD_SOFTERR_MASK 0x80000000
> +#define IDXD_SCMD_SOFTERR_SHIFT 16
> +
> #define SCMD_STAT(x) (((x) & ~IDXD_SCMD_SOFTERR_MASK) >> \
> IDXD_SCMD_SOFTERR_SHIFT)
>
5:34 p.m.
I think copying will have the problem of compiler picking different
ones. Library may use the copied version while the app may get the one
in /usr/include. Do you think defining new constants would be safer?
On Mon, Jul 19, 2021 at 08:17:28AM -0700, Dave Jiang wrote:
On 7/16/2021 6:46 PM, ramesh.thomas(a)intel.com wrote:
> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
>
> Duplicate the driver error code related constants defined in idxd.h in
> library to avoid dependency on kernel versions.
I think instead of replicating certain defines, the suggestion by Vishal
was to make a copy of the latest kernel uapi idxd.h in accel-config.
This is probably the best. Otherwise you'll have conflicts if this is
build with the latest idxd.h having those defines as well.
>
> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
> ---
> accfg/lib/libaccfg.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
> index f21cab5..1a06d9c 100644
> --- a/accfg/lib/libaccfg.c
> +++ b/accfg/lib/libaccfg.c
> @@ -27,7 +27,6 @@
> #include <accfg/libaccel_config.h>
> #include <fnmatch.h>
> #include "private.h"
> -#include <linux/idxd.h>
>
> #define MDEV_POSTFIX "mdev_supported_types"
> #define IDXD_DRIVER_BIND_PATH "/sys/bus/dsa/drivers/idxd"
> @@ -74,6 +73,29 @@ enum {
> ACCFG_CMD_STATUS_ERROR = 0x80010000,
> };
>
> +/* driver error codes from kernel idxd.h */
> +enum idxd_scmd_stat {
> + IDXD_SCMD_DEV_ENABLED = 0x80000010,
> + IDXD_SCMD_DEV_NOT_ENABLED = 0x80000020,
> + IDXD_SCMD_WQ_ENABLED = 0x80000021,
> + IDXD_SCMD_DEV_DMA_ERR = 0x80020000,
> + IDXD_SCMD_WQ_NO_GRP = 0x80030000,
> + IDXD_SCMD_WQ_NO_NAME = 0x80040000,
> + IDXD_SCMD_WQ_NO_SVM = 0x80050000,
> + IDXD_SCMD_WQ_NO_THRESH = 0x80060000,
> + IDXD_SCMD_WQ_PORTAL_ERR = 0x80070000,
> + IDXD_SCMD_WQ_RES_ALLOC_ERR = 0x80080000,
> + IDXD_SCMD_PERCPU_ERR = 0x80090000,
> + IDXD_SCMD_DMA_CHAN_ERR = 0x800a0000,
> + IDXD_SCMD_CDEV_ERR = 0x800b0000,
> + IDXD_SCMD_WQ_NO_SWQ_SUPPORT = 0x800c0000,
> + IDXD_SCMD_WQ_NONE_CONFIGURED = 0x800d0000,
> + IDXD_SCMD_WQ_NO_SIZE = 0x800e0000,
> +};
> +
> +#define IDXD_SCMD_SOFTERR_MASK 0x80000000
> +#define IDXD_SCMD_SOFTERR_SHIFT 16
> +
> #define SCMD_STAT(x) (((x) & ~IDXD_SCMD_SOFTERR_MASK) >> \
> IDXD_SCMD_SOFTERR_SHIFT)
>
5:50 p.m.
On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
I think copying will have the problem of compiler picking different
ones. Library may use the copied version while the app may get the one
in /usr/include. Do you think defining new constants would be safer?
I think instead of pulling the /usr/include version we just depend on
the copied version exclusively. Then we will have 1 less dependency and
compile correctly always.
>
> On Mon, Jul 19, 2021 at 08:17:28AM -0700, Dave Jiang wrote:
>> On 7/16/2021 6:46 PM, ramesh.thomas(a)intel.com wrote:
>>> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
>>>
>>> Duplicate the driver error code related constants defined in idxd.h in
>>> library to avoid dependency on kernel versions.
>> I think instead of replicating certain defines, the suggestion by Vishal
>> was to make a copy of the latest kernel uapi idxd.h in accel-config.
>> This is probably the best. Otherwise you'll have conflicts if this is
>> build with the latest idxd.h having those defines as well.
>>
>>
>>> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
>>> ---
>>> accfg/lib/libaccfg.c | 24 +++++++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
>>> index f21cab5..1a06d9c 100644
>>> --- a/accfg/lib/libaccfg.c
>>> +++ b/accfg/lib/libaccfg.c
>>> @@ -27,7 +27,6 @@
>>> #include <accfg/libaccel_config.h>
>>> #include <fnmatch.h>
>>> #include "private.h"
>>> -#include <linux/idxd.h>
>>>
>>> #define MDEV_POSTFIX "mdev_supported_types"
>>> #define IDXD_DRIVER_BIND_PATH "/sys/bus/dsa/drivers/idxd"
>>> @@ -74,6 +73,29 @@ enum {
>>> ACCFG_CMD_STATUS_ERROR = 0x80010000,
>>> };
>>>
>>> +/* driver error codes from kernel idxd.h */
>>> +enum idxd_scmd_stat {
>>> + IDXD_SCMD_DEV_ENABLED = 0x80000010,
>>> + IDXD_SCMD_DEV_NOT_ENABLED = 0x80000020,
>>> + IDXD_SCMD_WQ_ENABLED = 0x80000021,
>>> + IDXD_SCMD_DEV_DMA_ERR = 0x80020000,
>>> + IDXD_SCMD_WQ_NO_GRP = 0x80030000,
>>> + IDXD_SCMD_WQ_NO_NAME = 0x80040000,
>>> + IDXD_SCMD_WQ_NO_SVM = 0x80050000,
>>> + IDXD_SCMD_WQ_NO_THRESH = 0x80060000,
>>> + IDXD_SCMD_WQ_PORTAL_ERR = 0x80070000,
>>> + IDXD_SCMD_WQ_RES_ALLOC_ERR = 0x80080000,
>>> + IDXD_SCMD_PERCPU_ERR = 0x80090000,
>>> + IDXD_SCMD_DMA_CHAN_ERR = 0x800a0000,
>>> + IDXD_SCMD_CDEV_ERR = 0x800b0000,
>>> + IDXD_SCMD_WQ_NO_SWQ_SUPPORT = 0x800c0000,
>>> + IDXD_SCMD_WQ_NONE_CONFIGURED = 0x800d0000,
>>> + IDXD_SCMD_WQ_NO_SIZE = 0x800e0000,
>>> +};
>>> +
>>> +#define IDXD_SCMD_SOFTERR_MASK 0x80000000
>>> +#define IDXD_SCMD_SOFTERR_SHIFT 16
>>> +
>>> #define SCMD_STAT(x) (((x) & ~IDXD_SCMD_SOFTERR_MASK) >> \
>>> IDXD_SCMD_SOFTERR_SHIFT)
>>>
7:35 p.m.
On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
> I think copying will have the problem of compiler picking different
> ones. Library may use the copied version while the app may get the one
> in /usr/include. Do you think defining new constants would be safer?
I think instead of pulling the /usr/include version we just depend on
the copied version exclusively. Then we will have 1 less dependency and
compile correctly always.
I was also concerned about user apps including the /user/include idxd.h.
Even in libaccel we would need to make assumptions that the constants
used will not diverge from the one driver is using. Since the issue is
only about some constants like the sw cmd_status which is not available
in older idxd.h, I think it is better to define them separately and
privately for the library.
8 p.m.
On 7/19/2021 12:35 PM, Thomas, Ramesh wrote:
On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
> On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
>> I think copying will have the problem of compiler picking different
>> ones. Library may use the copied version while the app may get the one
>> in /usr/include. Do you think defining new constants would be safer?
> I think instead of pulling the /usr/include version we just depend on
> the copied version exclusively. Then we will have 1 less dependency and
> compile correctly always.
I was also concerned about user apps including the /user/include idxd.h.
Even in libaccel we would need to make assumptions that the constants
used will not diverge from the one driver is using. Since the issue is
only about some constants like the sw cmd_status which is not available
in older idxd.h, I think it is better to define them separately and
privately for the library.
User ABI is considered constant for Linux and if we are changing it,
there would be ways to provide backward compatibility. I think making
copy of header should be safe. That is why ndctl does that.
>
9:13 p.m.
Noticed driver idxd.h has GPL license. Library is LGPL and I remember
LGPL cannot use GPL code. Even if that is changed in newer drivers I am
not sure if there is any issue with older drivers.
On Mon, Jul 19, 2021 at 01:00:46PM -0700, Dave Jiang wrote:
On 7/19/2021 12:35 PM, Thomas, Ramesh wrote:
> On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
>> On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
>>> I think copying will have the problem of compiler picking different
>>> ones. Library may use the copied version while the app may get the one
>>> in /usr/include. Do you think defining new constants would be safer?
>> I think instead of pulling the /usr/include version we just depend on
>> the copied version exclusively. Then we will have 1 less dependency and
>> compile correctly always.
> I was also concerned about user apps including the /user/include idxd.h.
> Even in libaccel we would need to make assumptions that the constants
> used will not diverge from the one driver is using. Since the issue is
> only about some constants like the sw cmd_status which is not available
> in older idxd.h, I think it is better to define them separately and
> privately for the library.
>
User ABI is considered constant for Linux and if we are changing it,
there would be ways to provide backward compatibility. I think making
copy of header should be safe. That is why ndctl does that.
>
10:14 p.m.
I just noticed that include/uapi/linux/ndctl.h is distributed as LGPL. I
wonder if it's possible to change that given only we (Intel) has touched
this file.
On 7/19/2021 2:13 PM, Thomas, Ramesh wrote:
> Noticed driver idxd.h has GPL license. Library is LGPL and I remember
> LGPL cannot use GPL code. Even if that is changed in newer drivers I am
> not sure if there is any issue with older drivers.
>
> On Mon, Jul 19, 2021 at 01:00:46PM -0700, Dave Jiang wrote:
>> On 7/19/2021 12:35 PM, Thomas, Ramesh wrote:
>>> On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
>>>> On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
>>>>> I think copying will have the problem of compiler picking different
>>>>> ones. Library may use the copied version while the app may get the
one
>>>>> in /usr/include. Do you think defining new constants would be safer?
>>>> I think instead of pulling the /usr/include version we just depend on
>>>> the copied version exclusively. Then we will have 1 less dependency and
>>>> compile correctly always.
>>> I was also concerned about user apps including the /user/include idxd.h.
>>> Even in libaccel we would need to make assumptions that the constants
>>> used will not diverge from the one driver is using. Since the issue is
>>> only about some constants like the sw cmd_status which is not available
>>> in older idxd.h, I think it is better to define them separately and
>>> privately for the library.
>>>
>> User ABI is considered constant for Linux and if we are changing it,
>> there would be ways to provide backward compatibility. I think making
>> copy of header should be safe. That is why ndctl does that.
12:16 a.m.
What should we do while this is resolved to fix the compile issue? Should
I create an idxd.h with LGPL license?
On Mon, Jul 19, 2021 at 03:14:31PM -0700, Dave Jiang wrote:
I just noticed that include/uapi/linux/ndctl.h is distributed as
LGPL. I
wonder if it's possible to change that given only we (Intel) has touched
this file.
On 7/19/2021 2:13 PM, Thomas, Ramesh wrote:
> Noticed driver idxd.h has GPL license. Library is LGPL and I remember
> LGPL cannot use GPL code. Even if that is changed in newer drivers I am
> not sure if there is any issue with older drivers.
>
> On Mon, Jul 19, 2021 at 01:00:46PM -0700, Dave Jiang wrote:
>> On 7/19/2021 12:35 PM, Thomas, Ramesh wrote:
>>> On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
>>>> On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
>>>>> I think copying will have the problem of compiler picking
different
>>>>> ones. Library may use the copied version while the app may get the
one
>>>>> in /usr/include. Do you think defining new constants would be
safer?
>>>> I think instead of pulling the /usr/include version we just depend on
>>>> the copied version exclusively. Then we will have 1 less dependency
and
>>>> compile correctly always.
>>> I was also concerned about user apps including the /user/include idxd.h.
>>> Even in libaccel we would need to make assumptions that the constants
>>> used will not diverge from the one driver is using. Since the issue is
>>> only about some constants like the sw cmd_status which is not available
>>> in older idxd.h, I think it is better to define them separately and
>>> privately for the library.
>>>
>> User ABI is considered constant for Linux and if we are changing it,
>> there would be ways to provide backward compatibility. I think making
>> copy of header should be safe. That is why ndctl does that.
12:24 a.m.
On 7/19/2021 5:16 PM, Thomas, Ramesh wrote:
What should we do while this is resolved to fix the compile issue?
Should
I create an idxd.h with LGPL license?
I'm ok with that.
>
> On Mon, Jul 19, 2021 at 03:14:31PM -0700, Dave Jiang wrote:
>> I just noticed that include/uapi/linux/ndctl.h is distributed as LGPL. I
>> wonder if it's possible to change that given only we (Intel) has touched
>> this file.
>>
>> On 7/19/2021 2:13 PM, Thomas, Ramesh wrote:
>>> Noticed driver idxd.h has GPL license. Library is LGPL and I remember
>>> LGPL cannot use GPL code. Even if that is changed in newer drivers I am
>>> not sure if there is any issue with older drivers.
>>>
>>> On Mon, Jul 19, 2021 at 01:00:46PM -0700, Dave Jiang wrote:
>>>> On 7/19/2021 12:35 PM, Thomas, Ramesh wrote:
>>>>> On Mon, Jul 19, 2021 at 10:50:09AM -0700, Dave Jiang wrote:
>>>>>> On 7/19/2021 10:34 AM, Thomas, Ramesh wrote:
>>>>>>> I think copying will have the problem of compiler picking
different
>>>>>>> ones. Library may use the copied version while the app may
get the one
>>>>>>> in /usr/include. Do you think defining new constants would be
safer?
>>>>>> I think instead of pulling the /usr/include version we just
depend on
>>>>>> the copied version exclusively. Then we will have 1 less
dependency and
>>>>>> compile correctly always.
>>>>> I was also concerned about user apps including the /user/include
idxd.h.
>>>>> Even in libaccel we would need to make assumptions that the
constants
>>>>> used will not diverge from the one driver is using. Since the issue
is
>>>>> only about some constants like the sw cmd_status which is not
available
>>>>> in older idxd.h, I think it is better to define them separately and
>>>>> privately for the library.
>>>>>
>>>> User ABI is considered constant for Linux and if we are changing it,
>>>> there would be ways to provide backward compatibility. I think making
>>>> copy of header should be safe. That is why ndctl does that.
350
days inactive
353
days old
9 comments
3 participants
participants (3)
-
Dave Jiang -
ramesh.thomas＠intel.com -
Thomas, Ramesh