From: Joe Richey <[email protected]>
The BIT(n) macro is used in the kernel as an alias for (1 << n).
However, it is not defined in the UAPI headers, which means that any
UAPI header files must be careful not to use it, or else the user
will get a linker error. For example, compiling the following program:
#include <sys/auxv.h>
#include <asm/hwcap2.h>
// Detect if FSGSBASE instructions are enabled
int main() {
unsigned long val = getauxval(AT_HWCAP2);
return !(val & HWCAP2_FSGSBASE);
}
Results in the following likner error:
/usr/bin/ld: /tmp/cceFpAdR.o: in function `main':
gs.c:(.text+0x21): undefined reference to `BIT'
This patch series changes all UAPI uses of BIT() to just be open-coded.
However, there really should be a check for this in checkpatch.pl
Currently, the script actually _encourages_ users to use the BIT macro
even if adding things to UAPI.
Running `rg "BIT\(" **/uapi/**` shows no more usage of BIT() in any
UAPI headers. Tested by building a basic kernel. Changes are trivial.
Joe Richey (6):
x86/elf: Don't use BIT() macro in UAPI headers
KVM: X86: Don't use BIT() macro in UAPI headers
drivers: firmware: psci: Don't use BIT() macro in UAPI headers
uacce: Don't use BIT() macro in UAPI headers
media: vicodec: Don't use BIT() macro in UAPI headers
tools headers UAPI: Sync pkt_sched.h with the kernel sources
arch/x86/include/uapi/asm/hwcap2.h | 2 +-
include/uapi/linux/kvm.h | 4 +-
include/uapi/linux/psci.h | 2 +-
include/uapi/linux/v4l2-controls.h | 22 ++---
include/uapi/misc/uacce/uacce.h | 2 +-
tools/include/uapi/linux/kvm.h | 4 +-
tools/include/uapi/linux/pkt_sched.h | 122 ++++++++++++++++++++++++---
7 files changed, 130 insertions(+), 28 deletions(-)
--
2.31.1
From: Joe Richey <[email protected]>
A previous patch [1] used the BIT() macro to define UACCE_DEV_SVA.
This macro is defined in the kernel but not in the UAPI headers.
[1] https://lore.kernel.org/patchwork/patch/11334877/
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/misc/uacce/uacce.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/misc/uacce/uacce.h b/include/uapi/misc/uacce/uacce.h
index cc7185678f47..a404ec40e000 100644
--- a/include/uapi/misc/uacce/uacce.h
+++ b/include/uapi/misc/uacce/uacce.h
@@ -23,7 +23,7 @@
* Support PASID
* Support device page faults (PCI PRI or SMMU Stall)
*/
-#define UACCE_DEV_SVA BIT(0)
+#define UACCE_DEV_SVA (1 << 0)
/**
* enum uacce_qfrt: queue file region type
--
2.31.1
From: Joe Richey <[email protected]>
It looks like this process isn't automatic for some reason.
This prevents some perf tools warnings and removes use of the
BIT() macro which isn't present for UAPI headers.
Signed-off-by: Joe Richey <[email protected]>
---
tools/include/uapi/linux/pkt_sched.h | 122 ++++++++++++++++++++++++---
1 file changed, 112 insertions(+), 10 deletions(-)
diff --git a/tools/include/uapi/linux/pkt_sched.h b/tools/include/uapi/linux/pkt_sched.h
index 5c903abc9fa5..79a699f106b1 100644
--- a/tools/include/uapi/linux/pkt_sched.h
+++ b/tools/include/uapi/linux/pkt_sched.h
@@ -2,6 +2,7 @@
#ifndef __LINUX_PKT_SCHED_H
#define __LINUX_PKT_SCHED_H
+#include <linux/const.h>
#include <linux/types.h>
/* Logical priority bands not depending on specific packet scheduler.
@@ -255,6 +256,9 @@ enum {
TCA_RED_PARMS,
TCA_RED_STAB,
TCA_RED_MAX_P,
+ TCA_RED_FLAGS, /* bitfield32 */
+ TCA_RED_EARLY_DROP_BLOCK, /* u32 */
+ TCA_RED_MARK_BLOCK, /* u32 */
__TCA_RED_MAX,
};
@@ -267,12 +271,28 @@ struct tc_red_qopt {
unsigned char Wlog; /* log(W) */
unsigned char Plog; /* log(P_max/(qth_max-qth_min)) */
unsigned char Scell_log; /* cell size for idle damping */
+
+ /* This field can be used for flags that a RED-like qdisc has
+ * historically supported. E.g. when configuring RED, it can be used for
+ * ECN, HARDDROP and ADAPTATIVE. For SFQ it can be used for ECN,
+ * HARDDROP. Etc. Because this field has not been validated, and is
+ * copied back on dump, any bits besides those to which a given qdisc
+ * has assigned a historical meaning need to be considered for free use
+ * by userspace tools.
+ *
+ * Any further flags need to be passed differently, e.g. through an
+ * attribute (such as TCA_RED_FLAGS above). Such attribute should allow
+ * passing both recent and historic flags in one value.
+ */
unsigned char flags;
#define TC_RED_ECN 1
#define TC_RED_HARDDROP 2
#define TC_RED_ADAPTATIVE 4
+#define TC_RED_NODROP 8
};
+#define TC_RED_HISTORIC_FLAGS (TC_RED_ECN | TC_RED_HARDDROP | TC_RED_ADAPTATIVE)
+
struct tc_red_xstats {
__u32 early; /* Early drops */
__u32 pdrop; /* Drops due to queue limits */
@@ -894,6 +914,12 @@ enum {
TCA_FQ_CE_THRESHOLD, /* DCTCP-like CE-marking threshold */
+ TCA_FQ_TIMER_SLACK, /* timer slack */
+
+ TCA_FQ_HORIZON, /* time horizon in us */
+
+ TCA_FQ_HORIZON_DROP, /* drop packets beyond horizon, or cap their EDT */
+
__TCA_FQ_MAX
};
@@ -913,6 +939,8 @@ struct tc_fq_qd_stats {
__u32 throttled_flows;
__u32 unthrottle_latency_ns;
__u64 ce_mark; /* packets above ce_threshold */
+ __u64 horizon_drops;
+ __u64 horizon_caps;
};
/* Heavy-Hitter Filter */
@@ -950,19 +978,56 @@ enum {
TCA_PIE_BETA,
TCA_PIE_ECN,
TCA_PIE_BYTEMODE,
+ TCA_PIE_DQ_RATE_ESTIMATOR,
__TCA_PIE_MAX
};
#define TCA_PIE_MAX (__TCA_PIE_MAX - 1)
struct tc_pie_xstats {
- __u32 prob; /* current probability */
- __u32 delay; /* current delay in ms */
- __u32 avg_dq_rate; /* current average dq_rate in bits/pie_time */
- __u32 packets_in; /* total number of packets enqueued */
- __u32 dropped; /* packets dropped due to pie_action */
- __u32 overlimit; /* dropped due to lack of space in queue */
- __u32 maxq; /* maximum queue size */
- __u32 ecn_mark; /* packets marked with ecn*/
+ __u64 prob; /* current probability */
+ __u32 delay; /* current delay in ms */
+ __u32 avg_dq_rate; /* current average dq_rate in
+ * bits/pie_time
+ */
+ __u32 dq_rate_estimating; /* is avg_dq_rate being calculated? */
+ __u32 packets_in; /* total number of packets enqueued */
+ __u32 dropped; /* packets dropped due to pie_action */
+ __u32 overlimit; /* dropped due to lack of space
+ * in queue
+ */
+ __u32 maxq; /* maximum queue size */
+ __u32 ecn_mark; /* packets marked with ecn*/
+};
+
+/* FQ PIE */
+enum {
+ TCA_FQ_PIE_UNSPEC,
+ TCA_FQ_PIE_LIMIT,
+ TCA_FQ_PIE_FLOWS,
+ TCA_FQ_PIE_TARGET,
+ TCA_FQ_PIE_TUPDATE,
+ TCA_FQ_PIE_ALPHA,
+ TCA_FQ_PIE_BETA,
+ TCA_FQ_PIE_QUANTUM,
+ TCA_FQ_PIE_MEMORY_LIMIT,
+ TCA_FQ_PIE_ECN_PROB,
+ TCA_FQ_PIE_ECN,
+ TCA_FQ_PIE_BYTEMODE,
+ TCA_FQ_PIE_DQ_RATE_ESTIMATOR,
+ __TCA_FQ_PIE_MAX
+};
+#define TCA_FQ_PIE_MAX (__TCA_FQ_PIE_MAX - 1)
+
+struct tc_fq_pie_xstats {
+ __u32 packets_in; /* total number of packets enqueued */
+ __u32 dropped; /* packets dropped due to fq_pie_action */
+ __u32 overlimit; /* dropped due to lack of space in queue */
+ __u32 overmemory; /* dropped due to lack of memory in queue */
+ __u32 ecn_mark; /* packets marked with ecn */
+ __u32 new_flow_count; /* count of new flows created by packets */
+ __u32 new_flows_len; /* count of flows in new list */
+ __u32 old_flows_len; /* count of flows in old list */
+ __u32 memory_usage; /* total memory across all queues */
};
/* CBS */
@@ -989,8 +1054,9 @@ struct tc_etf_qopt {
__s32 delta;
__s32 clockid;
__u32 flags;
-#define TC_ETF_DEADLINE_MODE_ON BIT(0)
-#define TC_ETF_OFFLOAD_ON BIT(1)
+#define TC_ETF_DEADLINE_MODE_ON _BITUL(0)
+#define TC_ETF_OFFLOAD_ON _BITUL(1)
+#define TC_ETF_SKIP_SOCK_CHECK _BITUL(2)
};
enum {
@@ -1022,6 +1088,7 @@ enum {
TCA_CAKE_INGRESS,
TCA_CAKE_ACK_FILTER,
TCA_CAKE_SPLIT_GSO,
+ TCA_CAKE_FWMARK,
__TCA_CAKE_MAX
};
#define TCA_CAKE_MAX (__TCA_CAKE_MAX - 1)
@@ -1148,6 +1215,19 @@ enum {
#define TCA_TAPRIO_SCHED_MAX (__TCA_TAPRIO_SCHED_MAX - 1)
+/* The format for the admin sched (dump only):
+ * [TCA_TAPRIO_SCHED_ADMIN_SCHED]
+ * [TCA_TAPRIO_ATTR_SCHED_BASE_TIME]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_CMD]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_GATES]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_INTERVAL]
+ */
+
+#define TCA_TAPRIO_ATTR_FLAG_TXTIME_ASSIST _BITUL(0)
+#define TCA_TAPRIO_ATTR_FLAG_FULL_OFFLOAD _BITUL(1)
+
enum {
TCA_TAPRIO_ATTR_UNSPEC,
TCA_TAPRIO_ATTR_PRIOMAP, /* struct tc_mqprio_qopt */
@@ -1156,9 +1236,31 @@ enum {
TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY, /* single entry */
TCA_TAPRIO_ATTR_SCHED_CLOCKID, /* s32 */
TCA_TAPRIO_PAD,
+ TCA_TAPRIO_ATTR_ADMIN_SCHED, /* The admin sched, only used in dump */
+ TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME, /* s64 */
+ TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME_EXTENSION, /* s64 */
+ TCA_TAPRIO_ATTR_FLAGS, /* u32 */
+ TCA_TAPRIO_ATTR_TXTIME_DELAY, /* u32 */
__TCA_TAPRIO_ATTR_MAX,
};
#define TCA_TAPRIO_ATTR_MAX (__TCA_TAPRIO_ATTR_MAX - 1)
+/* ETS */
+
+#define TCQ_ETS_MAX_BANDS 16
+
+enum {
+ TCA_ETS_UNSPEC,
+ TCA_ETS_NBANDS, /* u8 */
+ TCA_ETS_NSTRICT, /* u8 */
+ TCA_ETS_QUANTA, /* nested TCA_ETS_QUANTA_BAND */
+ TCA_ETS_QUANTA_BAND, /* u32 */
+ TCA_ETS_PRIOMAP, /* nested TCA_ETS_PRIOMAP_BAND */
+ TCA_ETS_PRIOMAP_BAND, /* u8 */
+ __TCA_ETS_MAX,
+};
+
+#define TCA_ETS_MAX (__TCA_ETS_MAX - 1)
+
#endif
--
2.31.1
On 20/05/21 12:43, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> The BIT(n) macro is used in the kernel as an alias for (1 << n).
> However, it is not defined in the UAPI headers, which means that any
> UAPI header files must be careful not to use it, or else the user
> will get a linker error. For example, compiling the following program:
>
> #include <sys/auxv.h>
> #include <asm/hwcap2.h>
>
> // Detect if FSGSBASE instructions are enabled
> int main() {
> unsigned long val = getauxval(AT_HWCAP2);
> return !(val & HWCAP2_FSGSBASE);
> }
>
> Results in the following likner error:
>
> /usr/bin/ld: /tmp/cceFpAdR.o: in function `main':
> gs.c:(.text+0x21): undefined reference to `BIT'
>
> This patch series changes all UAPI uses of BIT() to just be open-coded.
> However, there really should be a check for this in checkpatch.pl
> Currently, the script actually _encourages_ users to use the BIT macro
> even if adding things to UAPI.
>
> Running `rg "BIT\(" **/uapi/**` shows no more usage of BIT() in any
> UAPI headers. Tested by building a basic kernel. Changes are trivial.
>
> Joe Richey (6):
> x86/elf: Don't use BIT() macro in UAPI headers
> KVM: X86: Don't use BIT() macro in UAPI headers
> drivers: firmware: psci: Don't use BIT() macro in UAPI headers
> uacce: Don't use BIT() macro in UAPI headers
> media: vicodec: Don't use BIT() macro in UAPI headers
> tools headers UAPI: Sync pkt_sched.h with the kernel sources
>
> arch/x86/include/uapi/asm/hwcap2.h | 2 +-
> include/uapi/linux/kvm.h | 4 +-
> include/uapi/linux/psci.h | 2 +-
> include/uapi/linux/v4l2-controls.h | 22 ++---
> include/uapi/misc/uacce/uacce.h | 2 +-
> tools/include/uapi/linux/kvm.h | 4 +-
> tools/include/uapi/linux/pkt_sched.h | 122 ++++++++++++++++++++++++---
> 7 files changed, 130 insertions(+), 28 deletions(-)
>
Will queue the KVM one, thanks!
Paolo
From: Joe Richey <[email protected]>
A previous patch [1] used the BIT() macro to define HWCAP2_FSGSBASE.
This macro is defined in the kernel but not in the UAPI headers.
[1] https://lore.kernel.org/patchwork/patch/912068/
Signed-off-by: Joe Richey <[email protected]>
---
arch/x86/include/uapi/asm/hwcap2.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/uapi/asm/hwcap2.h b/arch/x86/include/uapi/asm/hwcap2.h
index 5fdfcb47000f..6d2175b43710 100644
--- a/arch/x86/include/uapi/asm/hwcap2.h
+++ b/arch/x86/include/uapi/asm/hwcap2.h
@@ -6,6 +6,6 @@
#define HWCAP2_RING3MWAIT (1 << 0)
/* Kernel allows FSGSBASE instructions available in Ring 3 */
-#define HWCAP2_FSGSBASE BIT(1)
+#define HWCAP2_FSGSBASE (1 << 1)
#endif
--
2.31.1
On Thu, May 20, 2021 at 03:43:37AM -0700, Joe Richey wrote:
> This patch series changes all UAPI uses of BIT() to just be open-coded.
> However, there really should be a check for this in checkpatch.pl
Wanna add that check too?
> Currently, the script actually _encourages_ users to use the BIT macro
> even if adding things to UAPI.
How so?
This is with your first patch:
$ ./scripts/checkpatch.pl /tmp/bit.01
total: 0 errors, 0 warnings, 7 lines checked
/tmp/bit.01 has no obvious style problems and is ready for submission.
Also, in your commit messages you refer to patches with patchwork links
- please use the respective upstream commit IDs instead. Grep for
"Fixes:" here:
Documentation/process/submitting-patches.rst
for more info.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
> > Currently, the script actually _encourages_ users to use the BIT macro
> > even if adding things to UAPI.
>
> How so?
Running checkpatch.pl with --strict gives:
CHECK: Prefer using the BIT macro
#26: FILE: arch/x86/include/uapi/asm/hwcap2.h:9:
+#define HWCAP2_FSGSBASE (1 << 1)
It should probably just recommend the _BITUL macro.
> Also, in your commit messages you refer to patches with patchwork links
> - please use the respective upstream commit IDs instead. Grep for
> "Fixes:" here:
Ahhhhh, I figured there was a more standard way. Will fix.
From: Joe Richey <[email protected]>
A previous patch [1] used the BIT() macro to define the
KVM_DIRTY_GFN_F_* constants in KVM's UAPI header.
This macro is defined in the kernel but not in the UAPI headers.
[1] https://patchwork.kernel.org/patch/11854393
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/linux/kvm.h | 4 ++--
tools/include/uapi/linux/kvm.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 3fd9a7e9d90c..8f8a0fd7cd65 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1879,8 +1879,8 @@ struct kvm_hyperv_eventfd {
* conversion after harvesting an entry. Also, it must not skip any
* dirty bits, so that dirty bits are always harvested in sequence.
*/
-#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
-#define KVM_DIRTY_GFN_F_RESET BIT(1)
+#define KVM_DIRTY_GFN_F_DIRTY (1 << 0)
+#define KVM_DIRTY_GFN_F_RESET (1 << 1)
#define KVM_DIRTY_GFN_F_MASK 0x3
/*
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 3fd9a7e9d90c..8f8a0fd7cd65 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1879,8 +1879,8 @@ struct kvm_hyperv_eventfd {
* conversion after harvesting an entry. Also, it must not skip any
* dirty bits, so that dirty bits are always harvested in sequence.
*/
-#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
-#define KVM_DIRTY_GFN_F_RESET BIT(1)
+#define KVM_DIRTY_GFN_F_DIRTY (1 << 0)
+#define KVM_DIRTY_GFN_F_RESET (1 << 1)
#define KVM_DIRTY_GFN_F_MASK 0x3
/*
--
2.31.1
From: Joe Richey <[email protected]>
A previous patch [1] used the BIT() macro to define
PSCI_1_0_OS_INITIATED in the UAPI header.
This macro is defined in the kernel but not in the UAPI headers.
[1] https://lore.kernel.org/patchwork/patch/949966/
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/linux/psci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 2fcad1dd0b0e..d7da8059cbbe 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -100,7 +100,7 @@
#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK \
(0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
-#define PSCI_1_0_OS_INITIATED BIT(0)
+#define PSCI_1_0_OS_INITIATED (1 << 0)
#define PSCI_1_0_SUSPEND_MODE_PC 0
#define PSCI_1_0_SUSPEND_MODE_OSI 1
--
2.31.1
On Thu, May 20, 2021, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> The BIT(n) macro is used in the kernel as an alias for (1 << n).
> However, it is not defined in the UAPI headers, which means that any
> UAPI header files must be careful not to use it, or else the user
> will get a linker error. For example, compiling the following program:
>
> #include <sys/auxv.h>
> #include <asm/hwcap2.h>
>
> // Detect if FSGSBASE instructions are enabled
> int main() {
> unsigned long val = getauxval(AT_HWCAP2);
> return !(val & HWCAP2_FSGSBASE);
> }
>
> Results in the following likner error:
>
> /usr/bin/ld: /tmp/cceFpAdR.o: in function `main':
> gs.c:(.text+0x21): undefined reference to `BIT'
>
> This patch series changes all UAPI uses of BIT() to just be open-coded.
> However, there really should be a check for this in checkpatch.pl
Any reason not to provide such a patch in this series? :-)
> Currently, the script actually _encourages_ users to use the BIT macro
> even if adding things to UAPI.
>
> Running `rg "BIT\(" **/uapi/**` shows no more usage of BIT() in any
> UAPI headers. Tested by building a basic kernel. Changes are trivial.
This feedback applies to all patches in this series.
On Thu, May 20, 2021, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> A previous patch
Heh, I think it goes without saying that the code was introduced by a previous
patch, unless you've invented a time machine, in which case we should talk...
> [1] used the BIT() macro to define the
> KVM_DIRTY_GFN_F_* constants in KVM's UAPI header.
>
> This macro is defined in the kernel but not in the UAPI headers.
>
> [1] https://patchwork.kernel.org/patch/11854393
Linking to the patch isn't helpful/desirable in this case because it doesn't
provide any info about when the commit actually landed in the kernel. And
depending on the whims of the maintainer, what was posted may not exactly match
the code that was commited.
What you want is a Fixes: tag that points at the offending commit. The Fixes:
tag will also get the fix picked up for stable kernels, though in KVM we often
explicitly add "Cc: [email protected]" (though IIRC tglx prefers not to have
the explicit Cc).
Anyways, the changelog can simply be something like:
Replace BIT() in KVM's UAPI header with an open coded equivalent. BIT() is
not defined in the UAPI headers and its usage may cause userspace build errors.
Fixes: fb04a1eddb1a ("KVM: X86: Implement ring-based dirty memory tracking")
Signed-off-by: Joe Richey <[email protected]>
On Thu, May 20, 2021 at 04:50:11AM -0700, Joseph Richey wrote:
> Running checkpatch.pl with --strict gives:
Yah, people must *remember* to use --strict.
> It should probably just recommend the _BITUL macro.
Right.
> Ahhhhh, I figured there was a more standard way. Will fix.
Yeah, Fixes: is very useful, as we've come to realize.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
From: Joe Richey <[email protected]>
The BIT(n) macro is used in the kernel as an alias for (1 << n).
However, it is not defined in the UAPI headers, they should instead use
the _BITUL(n) macro. This patch chages all existing usages in UAPI
headers and updates ./scripts/checkpatch.pl to properly reccomend the
correct macro depending on context.
Running the below commands shows no more incorrect macro usages:
rg "BIT\(" **/uapi/**
rg "BIT_ULL\(" **/uapi/**
Tested by building a basic kernel. Changes are trivial.
I encountered this issue when compiling the following program:
#include <sys/auxv.h>
#include <asm/hwcap2.h>
// Detect if FSGSBASE instructions are enabled
int main() {
unsigned long val = getauxval(AT_HWCAP2);
return !(val & HWCAP2_FSGSBASE);
}
Resulting in the following likner error:
/usr/bin/ld: /tmp/cceFpAdR.o: in function `main':
gs.c:(.text+0x21): undefined reference to `BIT'
Changes from V1 to V2:
- Use _BITUL() macro instead of open-coding
- Fixup HWCAP2_RING3MWAIT as well
- Shorten commits and added "Fixes" per reviewer comments
- checkpatch: Broaden UAPI regex
- checkpatch: Reccomend _BITULL()/_BITUL() for UAPI headers
Joe Richey (7):
x86/elf: Use _BITUL() macro in UAPI headers
KVM: X86: Use _BITUL() macro in UAPI headers
drivers: firmware: psci: Use _BITUL() macro in UAPI headers
uacce: Use _BITUL() macro in UAPI headers
media: vicodec: Use _BITUL() macro in UAPI headers
tools headers UAPI: Sync pkt_sched.h with the kernel sources
checkpatch: suggest _BITULL() and _BITUL() for UAPI headers
arch/x86/include/uapi/asm/hwcap2.h | 6 +-
include/uapi/linux/kvm.h | 5 +-
include/uapi/linux/psci.h | 4 +-
include/uapi/linux/v4l2-controls.h | 23 ++---
include/uapi/misc/uacce/uacce.h | 3 +-
scripts/checkpatch.pl | 16 ++--
tools/include/uapi/linux/kvm.h | 5 +-
tools/include/uapi/linux/pkt_sched.h | 122 ++++++++++++++++++++++++---
8 files changed, 148 insertions(+), 36 deletions(-)
--
2.31.1
From: Joe Richey <[email protected]>
Replace BIT() in KVM's UPAI header with _BITUL(). BIT() is not defined
in the UAPI headers and its usage may cause userspace build errors.
Fixes: fb04a1eddb1a ("KVM: X86: Implement ring-based dirty memory tracking")
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/linux/kvm.h | 5 +++--
tools/include/uapi/linux/kvm.h | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 3fd9a7e9d90c..79d9c44d1ad7 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -8,6 +8,7 @@
* Note: you must update KVM_API_VERSION if you change this interface.
*/
+#include <linux/const.h>
#include <linux/types.h>
#include <linux/compiler.h>
#include <linux/ioctl.h>
@@ -1879,8 +1880,8 @@ struct kvm_hyperv_eventfd {
* conversion after harvesting an entry. Also, it must not skip any
* dirty bits, so that dirty bits are always harvested in sequence.
*/
-#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
-#define KVM_DIRTY_GFN_F_RESET BIT(1)
+#define KVM_DIRTY_GFN_F_DIRTY _BITUL(0)
+#define KVM_DIRTY_GFN_F_RESET _BITUL(1)
#define KVM_DIRTY_GFN_F_MASK 0x3
/*
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 3fd9a7e9d90c..79d9c44d1ad7 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -8,6 +8,7 @@
* Note: you must update KVM_API_VERSION if you change this interface.
*/
+#include <linux/const.h>
#include <linux/types.h>
#include <linux/compiler.h>
#include <linux/ioctl.h>
@@ -1879,8 +1880,8 @@ struct kvm_hyperv_eventfd {
* conversion after harvesting an entry. Also, it must not skip any
* dirty bits, so that dirty bits are always harvested in sequence.
*/
-#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
-#define KVM_DIRTY_GFN_F_RESET BIT(1)
+#define KVM_DIRTY_GFN_F_DIRTY _BITUL(0)
+#define KVM_DIRTY_GFN_F_RESET _BITUL(1)
#define KVM_DIRTY_GFN_F_MASK 0x3
/*
--
2.31.1
From: Joe Richey <[email protected]>
Replace BIT() in v4l2's UPAI header with _BITUL(). BIT() is not defined
in the UAPI headers and its usage may cause userspace build errors.
Fixes: 206bc0f6fb94 ("media: vicodec: mark the stateless FWHT API as stable")
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/linux/v4l2-controls.h | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index d43bec5f1afd..5afc19c68704 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -50,6 +50,7 @@
#ifndef __LINUX_V4L2_CONTROLS_H
#define __LINUX_V4L2_CONTROLS_H
+#include <linux/const.h>
#include <linux/types.h>
/* Control classes */
@@ -1602,30 +1603,30 @@ struct v4l2_ctrl_h264_decode_params {
#define V4L2_FWHT_VERSION 3
/* Set if this is an interlaced format */
-#define V4L2_FWHT_FL_IS_INTERLACED BIT(0)
+#define V4L2_FWHT_FL_IS_INTERLACED _BITUL(0)
/* Set if this is a bottom-first (NTSC) interlaced format */
-#define V4L2_FWHT_FL_IS_BOTTOM_FIRST BIT(1)
+#define V4L2_FWHT_FL_IS_BOTTOM_FIRST _BITUL(1)
/* Set if each 'frame' contains just one field */
-#define V4L2_FWHT_FL_IS_ALTERNATE BIT(2)
+#define V4L2_FWHT_FL_IS_ALTERNATE _BITUL(2)
/*
* If V4L2_FWHT_FL_IS_ALTERNATE was set, then this is set if this
* 'frame' is the bottom field, else it is the top field.
*/
-#define V4L2_FWHT_FL_IS_BOTTOM_FIELD BIT(3)
+#define V4L2_FWHT_FL_IS_BOTTOM_FIELD _BITUL(3)
/* Set if the Y' plane is uncompressed */
-#define V4L2_FWHT_FL_LUMA_IS_UNCOMPRESSED BIT(4)
+#define V4L2_FWHT_FL_LUMA_IS_UNCOMPRESSED _BITUL(4)
/* Set if the Cb plane is uncompressed */
-#define V4L2_FWHT_FL_CB_IS_UNCOMPRESSED BIT(5)
+#define V4L2_FWHT_FL_CB_IS_UNCOMPRESSED _BITUL(5)
/* Set if the Cr plane is uncompressed */
-#define V4L2_FWHT_FL_CR_IS_UNCOMPRESSED BIT(6)
+#define V4L2_FWHT_FL_CR_IS_UNCOMPRESSED _BITUL(6)
/* Set if the chroma plane is full height, if cleared it is half height */
-#define V4L2_FWHT_FL_CHROMA_FULL_HEIGHT BIT(7)
+#define V4L2_FWHT_FL_CHROMA_FULL_HEIGHT _BITUL(7)
/* Set if the chroma plane is full width, if cleared it is half width */
-#define V4L2_FWHT_FL_CHROMA_FULL_WIDTH BIT(8)
+#define V4L2_FWHT_FL_CHROMA_FULL_WIDTH _BITUL(8)
/* Set if the alpha plane is uncompressed */
-#define V4L2_FWHT_FL_ALPHA_IS_UNCOMPRESSED BIT(9)
+#define V4L2_FWHT_FL_ALPHA_IS_UNCOMPRESSED _BITUL(9)
/* Set if this is an I Frame */
-#define V4L2_FWHT_FL_I_FRAME BIT(10)
+#define V4L2_FWHT_FL_I_FRAME _BITUL(10)
/* A 4-values flag - the number of components - 1 */
#define V4L2_FWHT_FL_COMPONENTS_NUM_MSK GENMASK(18, 16)
--
2.31.1
From: Joe Richey <[email protected]>
Replace BIT() in uacce's UPAI header with _BITUL(). BIT() is not defined
in the UAPI headers and its usage may cause userspace build errors.
Fixes: 015d239ac014 ("uacce: add uacce driver")
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/misc/uacce/uacce.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/misc/uacce/uacce.h b/include/uapi/misc/uacce/uacce.h
index cc7185678f47..e0b4c8a2d29c 100644
--- a/include/uapi/misc/uacce/uacce.h
+++ b/include/uapi/misc/uacce/uacce.h
@@ -2,6 +2,7 @@
#ifndef _UAPIUUACCE_H
#define _UAPIUUACCE_H
+#include <linux/const.h>
#include <linux/types.h>
#include <linux/ioctl.h>
@@ -23,7 +24,7 @@
* Support PASID
* Support device page faults (PCI PRI or SMMU Stall)
*/
-#define UACCE_DEV_SVA BIT(0)
+#define UACCE_DEV_SVA _BITUL(0)
/**
* enum uacce_qfrt: queue file region type
--
2.31.1
On 2021/5/21 下午4:58, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> Replace BIT() in uacce's UPAI header with _BITUL(). BIT() is not defined
> in the UAPI headers and its usage may cause userspace build errors.
>
> Fixes: 015d239ac014 ("uacce: add uacce driver")
> Signed-off-by: Joe Richey <[email protected]>
Thanks Joe
Acked-by: Zhangfei Gao <[email protected]>
> ---
> include/uapi/misc/uacce/uacce.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/misc/uacce/uacce.h b/include/uapi/misc/uacce/uacce.h
> index cc7185678f47..e0b4c8a2d29c 100644
> --- a/include/uapi/misc/uacce/uacce.h
> +++ b/include/uapi/misc/uacce/uacce.h
> @@ -2,6 +2,7 @@
> #ifndef _UAPIUUACCE_H
> #define _UAPIUUACCE_H
>
> +#include <linux/const.h>
> #include <linux/types.h>
> #include <linux/ioctl.h>
>
> @@ -23,7 +24,7 @@
> * Support PASID
> * Support device page faults (PCI PRI or SMMU Stall)
> */
> -#define UACCE_DEV_SVA BIT(0)
> +#define UACCE_DEV_SVA _BITUL(0)
>
> /**
> * enum uacce_qfrt: queue file region type
On Thu, May 20, 2021, Borislav Petkov wrote:
> On Thu, May 20, 2021 at 03:43:37AM -0700, Joe Richey wrote:
> > This patch series changes all UAPI uses of BIT() to just be open-coded.
> > However, there really should be a check for this in checkpatch.pl
>
> Wanna add that check too?
>
> > Currently, the script actually _encourages_ users to use the BIT macro
> > even if adding things to UAPI.
>
> How so?
>
> This is with your first patch:
>
> $ ./scripts/checkpatch.pl /tmp/bit.01
> total: 0 errors, 0 warnings, 7 lines checked
>
> /tmp/bit.01 has no obvious style problems and is ready for submission.
>
> Also, in your commit messages you refer to patches with patchwork links
> - please use the respective upstream commit IDs instead. Grep for
> "Fixes:" here:
Gah, beat me to the punch. Stupid weird mutt sorting.
From: Joe Richey <[email protected]>
Instead of just ignoring UAPI headers, reccomend the UAPI compatible
macros if a user adds something that looks like (1 << n). Normal kernel
code will continue to get BIT_ULL() and BIT() reccomended.
This change also modifies the $realfile regex to match headers that have
"include/uapi" anywhere in their path so paths like:
tools/include/uapi/linux/kvm.h
arch/x86/include/uapi/asm/hwcap2.h
get recognized as UAPI headers.
Signed-off-by: Joe Richey <[email protected]>
---
scripts/checkpatch.pl | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d65334588eb4..47edc61b74c2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7020,15 +7020,17 @@ sub process {
}
}
-# check for #defines like: 1 << <digit> that could be BIT(digit), it is not exported to uapi
- if ($realfile !~ m@^include/uapi/@ &&
- $line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) {
- my $ull = "";
- $ull = "_ULL" if (defined($1) && $1 =~ /ll/i);
+# check for #defines like: 1 << <digit> that could be BIT(digit) or similar
+ if ($line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) {
+ my $ull = (defined($1) && $1 =~ /ll/i);
+ my $macroname = $ull ? "BIT_ULL" : "BIT";
+ if ($realfile =~ m@include/uapi/@) {
+ $macroname = $ull ? "_BITULL" : "_BITUL";
+ }
if (CHK("BIT_MACRO",
- "Prefer using the BIT$ull macro\n" . $herecurr) &&
+ "Prefer using the $macroname macro\n" . $herecurr) &&
$fix) {
- $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/BIT${ull}($1)/;
+ $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/${macroname}($1)/;
}
}
--
2.31.1
From: Joe Richey <[email protected]>
It looks like this process isn't automatic for some reason.
This prevents some perf tools warnings and removes use of the
BIT() macro which isn't present for UAPI headers.
Signed-off-by: Joe Richey <[email protected]>
---
tools/include/uapi/linux/pkt_sched.h | 122 ++++++++++++++++++++++++---
1 file changed, 112 insertions(+), 10 deletions(-)
diff --git a/tools/include/uapi/linux/pkt_sched.h b/tools/include/uapi/linux/pkt_sched.h
index 5c903abc9fa5..79a699f106b1 100644
--- a/tools/include/uapi/linux/pkt_sched.h
+++ b/tools/include/uapi/linux/pkt_sched.h
@@ -2,6 +2,7 @@
#ifndef __LINUX_PKT_SCHED_H
#define __LINUX_PKT_SCHED_H
+#include <linux/const.h>
#include <linux/types.h>
/* Logical priority bands not depending on specific packet scheduler.
@@ -255,6 +256,9 @@ enum {
TCA_RED_PARMS,
TCA_RED_STAB,
TCA_RED_MAX_P,
+ TCA_RED_FLAGS, /* bitfield32 */
+ TCA_RED_EARLY_DROP_BLOCK, /* u32 */
+ TCA_RED_MARK_BLOCK, /* u32 */
__TCA_RED_MAX,
};
@@ -267,12 +271,28 @@ struct tc_red_qopt {
unsigned char Wlog; /* log(W) */
unsigned char Plog; /* log(P_max/(qth_max-qth_min)) */
unsigned char Scell_log; /* cell size for idle damping */
+
+ /* This field can be used for flags that a RED-like qdisc has
+ * historically supported. E.g. when configuring RED, it can be used for
+ * ECN, HARDDROP and ADAPTATIVE. For SFQ it can be used for ECN,
+ * HARDDROP. Etc. Because this field has not been validated, and is
+ * copied back on dump, any bits besides those to which a given qdisc
+ * has assigned a historical meaning need to be considered for free use
+ * by userspace tools.
+ *
+ * Any further flags need to be passed differently, e.g. through an
+ * attribute (such as TCA_RED_FLAGS above). Such attribute should allow
+ * passing both recent and historic flags in one value.
+ */
unsigned char flags;
#define TC_RED_ECN 1
#define TC_RED_HARDDROP 2
#define TC_RED_ADAPTATIVE 4
+#define TC_RED_NODROP 8
};
+#define TC_RED_HISTORIC_FLAGS (TC_RED_ECN | TC_RED_HARDDROP | TC_RED_ADAPTATIVE)
+
struct tc_red_xstats {
__u32 early; /* Early drops */
__u32 pdrop; /* Drops due to queue limits */
@@ -894,6 +914,12 @@ enum {
TCA_FQ_CE_THRESHOLD, /* DCTCP-like CE-marking threshold */
+ TCA_FQ_TIMER_SLACK, /* timer slack */
+
+ TCA_FQ_HORIZON, /* time horizon in us */
+
+ TCA_FQ_HORIZON_DROP, /* drop packets beyond horizon, or cap their EDT */
+
__TCA_FQ_MAX
};
@@ -913,6 +939,8 @@ struct tc_fq_qd_stats {
__u32 throttled_flows;
__u32 unthrottle_latency_ns;
__u64 ce_mark; /* packets above ce_threshold */
+ __u64 horizon_drops;
+ __u64 horizon_caps;
};
/* Heavy-Hitter Filter */
@@ -950,19 +978,56 @@ enum {
TCA_PIE_BETA,
TCA_PIE_ECN,
TCA_PIE_BYTEMODE,
+ TCA_PIE_DQ_RATE_ESTIMATOR,
__TCA_PIE_MAX
};
#define TCA_PIE_MAX (__TCA_PIE_MAX - 1)
struct tc_pie_xstats {
- __u32 prob; /* current probability */
- __u32 delay; /* current delay in ms */
- __u32 avg_dq_rate; /* current average dq_rate in bits/pie_time */
- __u32 packets_in; /* total number of packets enqueued */
- __u32 dropped; /* packets dropped due to pie_action */
- __u32 overlimit; /* dropped due to lack of space in queue */
- __u32 maxq; /* maximum queue size */
- __u32 ecn_mark; /* packets marked with ecn*/
+ __u64 prob; /* current probability */
+ __u32 delay; /* current delay in ms */
+ __u32 avg_dq_rate; /* current average dq_rate in
+ * bits/pie_time
+ */
+ __u32 dq_rate_estimating; /* is avg_dq_rate being calculated? */
+ __u32 packets_in; /* total number of packets enqueued */
+ __u32 dropped; /* packets dropped due to pie_action */
+ __u32 overlimit; /* dropped due to lack of space
+ * in queue
+ */
+ __u32 maxq; /* maximum queue size */
+ __u32 ecn_mark; /* packets marked with ecn*/
+};
+
+/* FQ PIE */
+enum {
+ TCA_FQ_PIE_UNSPEC,
+ TCA_FQ_PIE_LIMIT,
+ TCA_FQ_PIE_FLOWS,
+ TCA_FQ_PIE_TARGET,
+ TCA_FQ_PIE_TUPDATE,
+ TCA_FQ_PIE_ALPHA,
+ TCA_FQ_PIE_BETA,
+ TCA_FQ_PIE_QUANTUM,
+ TCA_FQ_PIE_MEMORY_LIMIT,
+ TCA_FQ_PIE_ECN_PROB,
+ TCA_FQ_PIE_ECN,
+ TCA_FQ_PIE_BYTEMODE,
+ TCA_FQ_PIE_DQ_RATE_ESTIMATOR,
+ __TCA_FQ_PIE_MAX
+};
+#define TCA_FQ_PIE_MAX (__TCA_FQ_PIE_MAX - 1)
+
+struct tc_fq_pie_xstats {
+ __u32 packets_in; /* total number of packets enqueued */
+ __u32 dropped; /* packets dropped due to fq_pie_action */
+ __u32 overlimit; /* dropped due to lack of space in queue */
+ __u32 overmemory; /* dropped due to lack of memory in queue */
+ __u32 ecn_mark; /* packets marked with ecn */
+ __u32 new_flow_count; /* count of new flows created by packets */
+ __u32 new_flows_len; /* count of flows in new list */
+ __u32 old_flows_len; /* count of flows in old list */
+ __u32 memory_usage; /* total memory across all queues */
};
/* CBS */
@@ -989,8 +1054,9 @@ struct tc_etf_qopt {
__s32 delta;
__s32 clockid;
__u32 flags;
-#define TC_ETF_DEADLINE_MODE_ON BIT(0)
-#define TC_ETF_OFFLOAD_ON BIT(1)
+#define TC_ETF_DEADLINE_MODE_ON _BITUL(0)
+#define TC_ETF_OFFLOAD_ON _BITUL(1)
+#define TC_ETF_SKIP_SOCK_CHECK _BITUL(2)
};
enum {
@@ -1022,6 +1088,7 @@ enum {
TCA_CAKE_INGRESS,
TCA_CAKE_ACK_FILTER,
TCA_CAKE_SPLIT_GSO,
+ TCA_CAKE_FWMARK,
__TCA_CAKE_MAX
};
#define TCA_CAKE_MAX (__TCA_CAKE_MAX - 1)
@@ -1148,6 +1215,19 @@ enum {
#define TCA_TAPRIO_SCHED_MAX (__TCA_TAPRIO_SCHED_MAX - 1)
+/* The format for the admin sched (dump only):
+ * [TCA_TAPRIO_SCHED_ADMIN_SCHED]
+ * [TCA_TAPRIO_ATTR_SCHED_BASE_TIME]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_CMD]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_GATES]
+ * [TCA_TAPRIO_ATTR_SCHED_ENTRY_INTERVAL]
+ */
+
+#define TCA_TAPRIO_ATTR_FLAG_TXTIME_ASSIST _BITUL(0)
+#define TCA_TAPRIO_ATTR_FLAG_FULL_OFFLOAD _BITUL(1)
+
enum {
TCA_TAPRIO_ATTR_UNSPEC,
TCA_TAPRIO_ATTR_PRIOMAP, /* struct tc_mqprio_qopt */
@@ -1156,9 +1236,31 @@ enum {
TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY, /* single entry */
TCA_TAPRIO_ATTR_SCHED_CLOCKID, /* s32 */
TCA_TAPRIO_PAD,
+ TCA_TAPRIO_ATTR_ADMIN_SCHED, /* The admin sched, only used in dump */
+ TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME, /* s64 */
+ TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME_EXTENSION, /* s64 */
+ TCA_TAPRIO_ATTR_FLAGS, /* u32 */
+ TCA_TAPRIO_ATTR_TXTIME_DELAY, /* u32 */
__TCA_TAPRIO_ATTR_MAX,
};
#define TCA_TAPRIO_ATTR_MAX (__TCA_TAPRIO_ATTR_MAX - 1)
+/* ETS */
+
+#define TCQ_ETS_MAX_BANDS 16
+
+enum {
+ TCA_ETS_UNSPEC,
+ TCA_ETS_NBANDS, /* u8 */
+ TCA_ETS_NSTRICT, /* u8 */
+ TCA_ETS_QUANTA, /* nested TCA_ETS_QUANTA_BAND */
+ TCA_ETS_QUANTA_BAND, /* u32 */
+ TCA_ETS_PRIOMAP, /* nested TCA_ETS_PRIOMAP_BAND */
+ TCA_ETS_PRIOMAP_BAND, /* u8 */
+ __TCA_ETS_MAX,
+};
+
+#define TCA_ETS_MAX (__TCA_ETS_MAX - 1)
+
#endif
--
2.31.1
From: Joe Richey <[email protected]>
Replace BIT() in x86's UPAI header with _BITUL(). BIT() is not defined
in the UAPI headers and its usage may cause userspace build errors.
Fixes: 742c45c3ecc9 ("x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2")
Signed-off-by: Joe Richey <[email protected]>
---
arch/x86/include/uapi/asm/hwcap2.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/uapi/asm/hwcap2.h b/arch/x86/include/uapi/asm/hwcap2.h
index 5fdfcb47000f..054604aba9f0 100644
--- a/arch/x86/include/uapi/asm/hwcap2.h
+++ b/arch/x86/include/uapi/asm/hwcap2.h
@@ -2,10 +2,12 @@
#ifndef _ASM_X86_HWCAP2_H
#define _ASM_X86_HWCAP2_H
+#include <linux/const.h>
+
/* MONITOR/MWAIT enabled in Ring 3 */
-#define HWCAP2_RING3MWAIT (1 << 0)
+#define HWCAP2_RING3MWAIT _BITUL(0)
/* Kernel allows FSGSBASE instructions available in Ring 3 */
-#define HWCAP2_FSGSBASE BIT(1)
+#define HWCAP2_FSGSBASE _BITUL(1)
#endif
--
2.31.1
From: Joe Richey <[email protected]>
Replace BIT() in psci's UPAI header with _BITUL(). BIT() is not defined
in the UAPI headers and its usage may cause userspace build errors.
Fixes: 60dd1ead65e8 ("drivers: firmware: psci: Announce support for OS initiated suspend mode")
Signed-off-by: Joe Richey <[email protected]>
---
include/uapi/linux/psci.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 2fcad1dd0b0e..87afdeb95096 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -12,6 +12,8 @@
#ifndef _UAPI_LINUX_PSCI_H
#define _UAPI_LINUX_PSCI_H
+#include <linux/const.h>
+
/*
* PSCI v0.1 interface
*
@@ -100,7 +102,7 @@
#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK \
(0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
-#define PSCI_1_0_OS_INITIATED BIT(0)
+#define PSCI_1_0_OS_INITIATED _BITUL(0)
#define PSCI_1_0_SUSPEND_MODE_PC 0
#define PSCI_1_0_SUSPEND_MODE_OSI 1
--
2.31.1
On Fri, May 21, 2021 at 01:58:44AM -0700, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> Replace BIT() in psci's UPAI header with _BITUL(). BIT() is not defined
> in the UAPI headers and its usage may cause userspace build errors.
>
> Fixes: 60dd1ead65e8 ("drivers: firmware: psci: Announce support for OS initiated suspend mode")
> Signed-off-by: Joe Richey <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Mark.
> ---
> include/uapi/linux/psci.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> index 2fcad1dd0b0e..87afdeb95096 100644
> --- a/include/uapi/linux/psci.h
> +++ b/include/uapi/linux/psci.h
> @@ -12,6 +12,8 @@
> #ifndef _UAPI_LINUX_PSCI_H
> #define _UAPI_LINUX_PSCI_H
>
> +#include <linux/const.h>
> +
> /*
> * PSCI v0.1 interface
> *
> @@ -100,7 +102,7 @@
> #define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK \
> (0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
>
> -#define PSCI_1_0_OS_INITIATED BIT(0)
> +#define PSCI_1_0_OS_INITIATED _BITUL(0)
> #define PSCI_1_0_SUSPEND_MODE_PC 0
> #define PSCI_1_0_SUSPEND_MODE_OSI 1
>
> --
> 2.31.1
>
On Fri, 2021-05-21 at 01:58 -0700, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> Instead of just ignoring UAPI headers, reccomend the UAPI compatible
> macros if a user adds something that looks like (1 << n). Normal kernel
> code will continue to get BIT_ULL() and BIT() reccomended.
>
> This change also modifies the $realfile regex to match headers that have
> "include/uapi" anywhere in their path so paths like:
> ????tools/include/uapi/linux/kvm.h
> ????arch/x86/include/uapi/asm/hwcap2.h
> get recognized as UAPI headers.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -7020,15 +7020,17 @@ sub process {
> ? }
> ? }
> ?
>
> -# check for #defines like: 1 << <digit> that could be BIT(digit), it is not exported to uapi
> - if ($realfile !~ m@^include/uapi/@ &&
> - $line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) {
> - my $ull = "";
> - $ull = "_ULL" if (defined($1) && $1 =~ /ll/i);
> +# check for #defines like: 1 << <digit> that could be BIT(digit) or similar
> + if ($line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) {
> + my $ull = (defined($1) && $1 =~ /ll/i);
> + my $macroname = $ull ? "BIT_ULL" : "BIT";
> + if ($realfile =~ m@include/uapi/@) {
Likely better with \b
if ($realfile =~ m@\binclude/uapi/@) {
> + $macroname = $ull ? "_BITULL" : "_BITUL";
> + }
> ? if (CHK("BIT_MACRO",
> - "Prefer using the BIT$ull macro\n" . $herecurr) &&
> + "Prefer using the $macroname macro\n" . $herecurr) &&
> ? $fix) {
> - $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/BIT${ull}($1)/;
> + $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/${macroname}($1)/;
Doesn't need braces
$fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/$macroname($1)/;
Otherwise, fine by me.
On Thu, May 20, 2021 at 03:43:37AM -0700, Joe Richey wrote:
> This patch series changes all UAPI uses of BIT() to just be open-coded.
> However, there really should be a check for this in checkpatch.pl
> Currently, the script actually _encourages_ users to use the BIT macro
> even if adding things to UAPI.
Yes. In fact it should warn about BIT() in general. It is totally
pointless obsfucation that doesn't even save any typing at all.
On 21/05/21 10:58, Joe Richey wrote:
> From: Joe Richey <[email protected]>
>
> Replace BIT() in KVM's UPAI header with _BITUL(). BIT() is not defined
> in the UAPI headers and its usage may cause userspace build errors.
>
> Fixes: fb04a1eddb1a ("KVM: X86: Implement ring-based dirty memory tracking")
> Signed-off-by: Joe Richey <[email protected]>
> ---
> include/uapi/linux/kvm.h | 5 +++--
> tools/include/uapi/linux/kvm.h | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 3fd9a7e9d90c..79d9c44d1ad7 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -8,6 +8,7 @@
> * Note: you must update KVM_API_VERSION if you change this interface.
> */
>
> +#include <linux/const.h>
> #include <linux/types.h>
> #include <linux/compiler.h>
> #include <linux/ioctl.h>
> @@ -1879,8 +1880,8 @@ struct kvm_hyperv_eventfd {
> * conversion after harvesting an entry. Also, it must not skip any
> * dirty bits, so that dirty bits are always harvested in sequence.
> */
> -#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
> -#define KVM_DIRTY_GFN_F_RESET BIT(1)
> +#define KVM_DIRTY_GFN_F_DIRTY _BITUL(0)
> +#define KVM_DIRTY_GFN_F_RESET _BITUL(1)
> #define KVM_DIRTY_GFN_F_MASK 0x3
>
> /*
> diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
> index 3fd9a7e9d90c..79d9c44d1ad7 100644
> --- a/tools/include/uapi/linux/kvm.h
> +++ b/tools/include/uapi/linux/kvm.h
> @@ -8,6 +8,7 @@
> * Note: you must update KVM_API_VERSION if you change this interface.
> */
>
> +#include <linux/const.h>
> #include <linux/types.h>
> #include <linux/compiler.h>
> #include <linux/ioctl.h>
> @@ -1879,8 +1880,8 @@ struct kvm_hyperv_eventfd {
> * conversion after harvesting an entry. Also, it must not skip any
> * dirty bits, so that dirty bits are always harvested in sequence.
> */
> -#define KVM_DIRTY_GFN_F_DIRTY BIT(0)
> -#define KVM_DIRTY_GFN_F_RESET BIT(1)
> +#define KVM_DIRTY_GFN_F_DIRTY _BITUL(0)
> +#define KVM_DIRTY_GFN_F_RESET _BITUL(1)
> #define KVM_DIRTY_GFN_F_MASK 0x3
>
> /*
>
Queued thi sone, thanks.
Paolo
On Mon, May 24, 2021 at 12:46:26PM +0100, Christoph Hellwig wrote:
> On Thu, May 20, 2021 at 03:43:37AM -0700, Joe Richey wrote:
> > This patch series changes all UAPI uses of BIT() to just be open-coded.
> > However, there really should be a check for this in checkpatch.pl
> > Currently, the script actually _encourages_ users to use the BIT macro
> > even if adding things to UAPI.
>
> Yes. In fact it should warn about BIT() in general. It is totally
> pointless obsfucation that doesn't even save any typing at all.
That's not quite true; the point is that if you use BIT() consistently,
then even when you refer to bits 32 to 63 you won't accidentally
introduce shifts of more than the width of int, and the definition will
work equally well for assembly and C, which isn't true if you use `1UL`
in the definition.
With that in mind it's shorter and clearer than its functional
equivalent:
BIT(x)
(UL(1) << (x))
So IMO it's preferable to use BIT() generally, or _BITUL() in uapi
headers.
Thanks,
Mark.
From: Mark Rutland
> Sent: 24 May 2021 13:29
>
> On Mon, May 24, 2021 at 12:46:26PM +0100, Christoph Hellwig wrote:
> > On Thu, May 20, 2021 at 03:43:37AM -0700, Joe Richey wrote:
> > > This patch series changes all UAPI uses of BIT() to just be open-coded.
> > > However, there really should be a check for this in checkpatch.pl
> > > Currently, the script actually _encourages_ users to use the BIT macro
> > > even if adding things to UAPI.
> >
> > Yes. In fact it should warn about BIT() in general. It is totally
> > pointless obsfucation that doesn't even save any typing at all.
>
> That's not quite true; the point is that if you use BIT() consistently,
> then even when you refer to bits 32 to 63 you won't accidentally
> introduce shifts of more than the width of int, and the definition will
> work equally well for assembly and C, which isn't true if you use `1UL`
> in the definition.
>
> With that in mind it's shorter and clearer than its functional
> equivalent:
>
> BIT(x)
> (UL(1) << (x))
>
> So IMO it's preferable to use BIT() generally, or _BITUL() in uapi
> headers.
And then, suddenly the compiler warns about truncation of the
high bits when ~BIT(x) is used to mask a 32bit value on 64bit systems.
Once the C standard committee had decided to change from K&R's
'sign preserving' integer promotions to 'value preserving'
you always lose somewhere.
Personally I prefer hex constants - I can't count bits at all.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)