From: Denis Mukhin <dmukhin@ford.com>
Introduce some basic infrastructure for doing domain ID allocation unit tests,
and add a few tests that ensure correctness of the domain ID allocator.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v12:
- fixed Makefile
- dropped unused symbols/includes from the test harness header
- s/printk/printf/g in the test code
---
tools/tests/Makefile | 2 +-
tools/tests/domid/.gitignore | 2 +
tools/tests/domid/Makefile | 48 ++++++++++
tools/tests/domid/include/xen/domain.h | 126 +++++++++++++++++++++++++
tools/tests/domid/test-domid.c | 78 +++++++++++++++
5 files changed, 255 insertions(+), 1 deletion(-)
create mode 100644 tools/tests/domid/.gitignore
create mode 100644 tools/tests/domid/Makefile
create mode 100644 tools/tests/domid/include/xen/domain.h
create mode 100644 tools/tests/domid/test-domid.c
diff --git a/tools/tests/Makefile b/tools/tests/Makefile
index 36928676a666..ff1666425436 100644
--- a/tools/tests/Makefile
+++ b/tools/tests/Makefile
@@ -1,7 +1,7 @@
XEN_ROOT = $(CURDIR)/../..
include $(XEN_ROOT)/tools/Rules.mk
-SUBDIRS-y :=
+SUBDIRS-y := domid
SUBDIRS-y += resource
SUBDIRS-$(CONFIG_X86) += cpu-policy
SUBDIRS-$(CONFIG_X86) += tsx
diff --git a/tools/tests/domid/.gitignore b/tools/tests/domid/.gitignore
new file mode 100644
index 000000000000..70e306b3c074
--- /dev/null
+++ b/tools/tests/domid/.gitignore
@@ -0,0 +1,2 @@
+*.o
+test-domid
diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
new file mode 100644
index 000000000000..08fbad096aec
--- /dev/null
+++ b/tools/tests/domid/Makefile
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Unit tests for domain ID allocator.
+#
+# Copyright 2025 Ford Motor Company
+
+XEN_ROOT=$(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+TESTS := test-domid
+
+vpath domid.c $(XEN_ROOT)/xen/common/
+
+.PHONY: all
+all: $(TESTS)
+
+.PHONY: run
+run: $(TESTS)
+ $(foreach t,$(TESTS),./$(t);)
+
+.PHONY: clean
+clean:
+ $(RM) -- *.o $(TESTS) $(DEPS_RM)
+
+.PHONY: distclean
+distclean: clean
+ $(RM) -- *~
+
+.PHONY: install
+install: all
+ $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests
+ $(INSTALL_PROG) test-domid $(DESTDIR)$(LIBEXEC)/tests
+
+.PHONY: uninstall
+uninstall:
+ $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/test-domid
+
+CFLAGS += -D__XEN_TOOLS__
+CFLAGS += $(APPEND_CFLAGS)
+CFLAGS += $(CFLAGS_xeninclude)
+CFLAGS += -I./include/
+
+LDFLAGS += $(APPEND_LDFLAGS)
+
+test-domid: domid.o test-domid.o
+ $(CC) $^ -o $@ $(LDFLAGS)
+
+-include $(DEPS_INCLUDE)
diff --git a/tools/tests/domid/include/xen/domain.h b/tools/tests/domid/include/xen/domain.h
new file mode 100644
index 000000000000..e5db0235445e
--- /dev/null
+++ b/tools/tests/domid/include/xen/domain.h
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Unit test harness for domain ID allocator.
+ *
+ * Copyright 2025 Ford Motor Company
+ */
+
+#ifndef _TEST_HARNESS_
+#define _TEST_HARNESS_
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+
+#include <xen-tools/common-macros.h>
+
+#define BUG_ON(x) assert(!(x))
+#define ASSERT(x) assert(x)
+
+#define DOMID_FIRST_RESERVED (10)
+#define DOMID_INVALID (11)
+
+#define DEFINE_SPINLOCK(x) unsigned long *(x)
+#define spin_lock(x) ((*(x))++)
+#define spin_unlock(x) ((*(x))--)
+
+#define printk printf
+
+#define BITS_PER_LONG sizeof(unsigned long)
+#define BITS_PER_WORD (8U * BITS_PER_LONG)
+#define BITS_TO_LONGS(bits) \
+ (((bits) + BITS_PER_LONG - 1) / BITS_PER_LONG)
+#define DECLARE_BITMAP(name, bits) \
+ unsigned long name[BITS_TO_LONGS(bits)]
+
+static inline int __test_and_set_bit(unsigned int nr, unsigned long *addr)
+{
+ unsigned long mask = 1UL << (nr % BITS_PER_WORD);
+ unsigned long *p = addr + (nr / BITS_PER_WORD);
+ int old = (*p & mask) != 0;
+
+ *p |= mask;
+
+ return old;
+}
+
+static inline int __test_and_clear_bit(unsigned int nr, unsigned long *addr)
+{
+ unsigned long mask = 1UL << (nr % BITS_PER_WORD);
+ unsigned long *p = addr + (nr / BITS_PER_WORD);
+ int old = (*p & mask) != 0;
+
+ *p &= ~mask;
+
+ return old;
+}
+
+static inline void __set_bit(unsigned int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = 1UL << (nr % BITS_PER_WORD);
+ unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
+
+ *p |= mask;
+}
+
+static inline void __clear_bit(unsigned int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = 1UL << (nr % BITS_PER_WORD);
+ unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
+
+ *p &= ~mask;
+}
+
+static inline unsigned long find_next_zero_bit(const unsigned long *addr,
+ unsigned long size,
+ unsigned long offset)
+{
+ unsigned long idx = offset / BITS_PER_WORD;
+ unsigned long bit = offset % BITS_PER_WORD;
+
+ if (offset >= size)
+ return size;
+
+ while (offset < size)
+ {
+ unsigned long val = addr[idx] | (~0UL >> (BITS_PER_WORD - bit));
+
+ if (~val)
+ {
+ unsigned long pos = __builtin_ffsl(~val);
+
+ if (pos > 0)
+ {
+ unsigned long rc = idx * BITS_PER_WORD + (pos - 1);
+
+ if (rc < size)
+ return rc;
+ }
+ }
+
+ offset = (idx + 1) * BITS_PER_WORD;
+ idx++;
+ bit = 0;
+ }
+
+ return size;
+}
+
+typedef bool spinlock_t;
+typedef uint16_t domid_t;
+
+/* See include/xen/domain.h */
+extern domid_t domid_alloc(domid_t domid);
+extern void domid_free(domid_t domid);
+
+#endif /* _TEST_HARNESS_ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/tests/domid/test-domid.c b/tools/tests/domid/test-domid.c
new file mode 100644
index 000000000000..d52eaf5f1f55
--- /dev/null
+++ b/tools/tests/domid/test-domid.c
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Unit tests for domain ID allocator.
+ *
+ * Copyright 2025 Ford Motor Company
+ */
+
+/* Local test include replicating hypervisor includes. */
+#include <xen/domain.h>
+
+int main(int argc, char **argv)
+{
+ domid_t expected, allocated;
+
+ printf("DOMID_FIRST_RESERVED=%u DOMID_INVALID=%u\n",
+ DOMID_FIRST_RESERVED, DOMID_INVALID);
+
+ /* Test ID#0 cannot be allocated twice. */
+ allocated = domid_alloc(0);
+ printf("TEST 1: expected %u allocated %u\n", 0, allocated);
+ ASSERT(allocated == 0);
+ allocated = domid_alloc(0);
+ printf("TEST 1: expected %u allocated %u\n", DOMID_INVALID, allocated);
+ ASSERT(allocated == DOMID_INVALID);
+
+ /* Ensure ID is not allocated. */
+ domid_free(0);
+
+ /*
+ * Test that that two consecutive calls of domid_alloc(DOMID_INVALID)
+ * will never return the same ID.
+ * NB: ID#0 is reserved and shall not be allocated by
+ * domid_alloc(DOMID_INVALID).
+ */
+ for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
+ {
+ allocated = domid_alloc(DOMID_INVALID);
+ printf("TEST 2: expected %u allocated %u\n", expected, allocated);
+ ASSERT(allocated == expected);
+ }
+ for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
+ {
+ allocated = domid_alloc(DOMID_INVALID);
+ printf("TEST 3: expected %u allocated %u\n", DOMID_INVALID, allocated);
+ ASSERT(allocated == DOMID_INVALID);
+ }
+
+ /* Re-allocate first ID from [1..DOMID_FIRST_RESERVED - 1]. */
+ expected = 1;
+ domid_free(1);
+ allocated = domid_alloc(DOMID_INVALID);
+ printf("TEST 4: expected %u allocated %u\n", expected, allocated);
+ ASSERT(allocated == expected);
+
+ /* Re-allocate last ID from [1..DOMID_FIRST_RESERVED - 1]. */
+ expected = DOMID_FIRST_RESERVED - 1;
+ domid_free(DOMID_FIRST_RESERVED - 1);
+ allocated = domid_alloc(DOMID_INVALID);
+ printf("TEST 5: expected %u allocated %u\n", expected, allocated);
+ ASSERT(allocated == expected);
+
+ /* Allocate an invalid ID. */
+ expected = DOMID_INVALID;
+ allocated = domid_alloc(DOMID_FIRST_RESERVED);
+ printf("TEST 6: expected %u allocated %u\n", expected, allocated);
+ ASSERT(allocated == expected);
+
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.34.1On Wed, Jul 30, 2025 at 05:41:00PM +0000, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Introduce some basic infrastructure for doing domain ID allocation unit tests,
> and add a few tests that ensure correctness of the domain ID allocator.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v12:
> - fixed Makefile
> - dropped unused symbols/includes from the test harness header
> - s/printk/printf/g in the test code
> ---
> tools/tests/Makefile | 2 +-
> tools/tests/domid/.gitignore | 2 +
> tools/tests/domid/Makefile | 48 ++++++++++
> tools/tests/domid/include/xen/domain.h | 126 +++++++++++++++++++++++++
> tools/tests/domid/test-domid.c | 78 +++++++++++++++
> 5 files changed, 255 insertions(+), 1 deletion(-)
> create mode 100644 tools/tests/domid/.gitignore
> create mode 100644 tools/tests/domid/Makefile
> create mode 100644 tools/tests/domid/include/xen/domain.h
> create mode 100644 tools/tests/domid/test-domid.c
>
> diff --git a/tools/tests/Makefile b/tools/tests/Makefile
> index 36928676a666..ff1666425436 100644
> --- a/tools/tests/Makefile
> +++ b/tools/tests/Makefile
> @@ -1,7 +1,7 @@
> XEN_ROOT = $(CURDIR)/../..
> include $(XEN_ROOT)/tools/Rules.mk
>
> -SUBDIRS-y :=
> +SUBDIRS-y := domid
> SUBDIRS-y += resource
> SUBDIRS-$(CONFIG_X86) += cpu-policy
> SUBDIRS-$(CONFIG_X86) += tsx
> diff --git a/tools/tests/domid/.gitignore b/tools/tests/domid/.gitignore
> new file mode 100644
> index 000000000000..70e306b3c074
> --- /dev/null
> +++ b/tools/tests/domid/.gitignore
> @@ -0,0 +1,2 @@
> +*.o
> +test-domid
> diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
> new file mode 100644
> index 000000000000..08fbad096aec
> --- /dev/null
> +++ b/tools/tests/domid/Makefile
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Unit tests for domain ID allocator.
> +#
> +# Copyright 2025 Ford Motor Company
> +
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +TESTS := test-domid
> +
> +vpath domid.c $(XEN_ROOT)/xen/common/
> +
> +.PHONY: all
> +all: $(TESTS)
> +
> +.PHONY: run
> +run: $(TESTS)
> + $(foreach t,$(TESTS),./$(t);)
> +
> +.PHONY: clean
> +clean:
> + $(RM) -- *.o $(TESTS) $(DEPS_RM)
> +
> +.PHONY: distclean
> +distclean: clean
> + $(RM) -- *~
> +
> +.PHONY: install
> +install: all
> + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests
> + $(INSTALL_PROG) test-domid $(DESTDIR)$(LIBEXEC)/tests
> +
> +.PHONY: uninstall
> +uninstall:
> + $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/test-domid
> +
> +CFLAGS += -D__XEN_TOOLS__
> +CFLAGS += $(APPEND_CFLAGS)
> +CFLAGS += $(CFLAGS_xeninclude)
> +CFLAGS += -I./include/
> +
> +LDFLAGS += $(APPEND_LDFLAGS)
> +
> +test-domid: domid.o test-domid.o
> + $(CC) $^ -o $@ $(LDFLAGS)
> +
> +-include $(DEPS_INCLUDE)
> diff --git a/tools/tests/domid/include/xen/domain.h b/tools/tests/domid/include/xen/domain.h
> new file mode 100644
> index 000000000000..e5db0235445e
> --- /dev/null
> +++ b/tools/tests/domid/include/xen/domain.h
> @@ -0,0 +1,126 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Unit test harness for domain ID allocator.
> + *
> + * Copyright 2025 Ford Motor Company
> + */
> +
> +#ifndef _TEST_HARNESS_
> +#define _TEST_HARNESS_
> +
> +#include <assert.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +#include <xen-tools/common-macros.h>
> +
> +#define BUG_ON(x) assert(!(x))
> +#define ASSERT(x) assert(x)
> +
> +#define DOMID_FIRST_RESERVED (10)
> +#define DOMID_INVALID (11)
> +
> +#define DEFINE_SPINLOCK(x) unsigned long *(x)
I think this shouldn't be a pointer? As you otherwise trigger a NULL
pointer dereference in the increases and decreases done below?
> +#define spin_lock(x) ((*(x))++)
> +#define spin_unlock(x) ((*(x))--)
FWIW, I would use a plain bool:
#define DEFINE_SPINLOCK(l) bool l
#define spin_lock(l) (*(l) = true)
#define spin_unlock(l) (*(l) = false)
As you don't expect concurrency tests, you could even assert the lock
is in the expected state before taking/releasing it.
> +
> +#define printk printf
> +
> +#define BITS_PER_LONG sizeof(unsigned long)
That's BYTES_PER_LONG, BITS_PER_LONG would be (sizeof(unsigned long) * 8).
> +#define BITS_PER_WORD (8U * BITS_PER_LONG)
> +#define BITS_TO_LONGS(bits) \
> + (((bits) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> +#define DECLARE_BITMAP(name, bits) \
> + unsigned long name[BITS_TO_LONGS(bits)]
> +
> +static inline int __test_and_set_bit(unsigned int nr, unsigned long *addr)
> +{
> + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> + unsigned long *p = addr + (nr / BITS_PER_WORD);
> + int old = (*p & mask) != 0;
> +
> + *p |= mask;
> +
> + return old;
> +}
> +
> +static inline int __test_and_clear_bit(unsigned int nr, unsigned long *addr)
> +{
> + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> + unsigned long *p = addr + (nr / BITS_PER_WORD);
> + int old = (*p & mask) != 0;
> +
> + *p &= ~mask;
> +
> + return old;
> +}
Could you somehow use the generic__test_and_set_bit() and
generic__test_and_clear_bit() implementations in bitops.h?
> +
> +static inline void __set_bit(unsigned int nr, volatile unsigned long *addr)
> +{
> + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> + unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
Why do you need the cast to drop the volatile here?
> +
> + *p |= mask;
> +}
I think you could possibly simplify to a single line:
((unsigned int *)addr)[nr >> 5] |= (1u << (nr & 31));
That's the implementation of constant_set_bit() in x86.
> +
> +static inline void __clear_bit(unsigned int nr, volatile unsigned long *addr)
> +{
> + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> + unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
> +
> + *p &= ~mask;
> +}
I don't think you need __clear_bit()? It's not used by domid.c AFAICT.
> +
> +static inline unsigned long find_next_zero_bit(const unsigned long *addr,
> + unsigned long size,
> + unsigned long offset)
> +{
> + unsigned long idx = offset / BITS_PER_WORD;
> + unsigned long bit = offset % BITS_PER_WORD;
> +
> + if (offset >= size)
> + return size;
> +
> + while (offset < size)
> + {
> + unsigned long val = addr[idx] | (~0UL >> (BITS_PER_WORD - bit));
> +
> + if (~val)
> + {
> + unsigned long pos = __builtin_ffsl(~val);
> +
> + if (pos > 0)
> + {
> + unsigned long rc = idx * BITS_PER_WORD + (pos - 1);
> +
> + if (rc < size)
> + return rc;
> + }
> + }
> +
> + offset = (idx + 1) * BITS_PER_WORD;
> + idx++;
> + bit = 0;
> + }
> +
> + return size;
> +}
Hm, you need a full find_next_zero_bit() implementation here because
addr can be arbitrarily long. Could you somehow include
xen/lib/find-next-bit.c and set the right defines so only the
implementation of find_next_bit() is included?
> +
> +typedef bool spinlock_t;
You want to put this ahead, so that DEFINE_SPINLOCK can be:
#define DEFINE_SPINLOCK(l) spinlock_t l
> +typedef uint16_t domid_t;
> +
> +/* See include/xen/domain.h */
> +extern domid_t domid_alloc(domid_t domid);
> +extern void domid_free(domid_t domid);
> +
> +#endif /* _TEST_HARNESS_ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/tests/domid/test-domid.c b/tools/tests/domid/test-domid.c
> new file mode 100644
> index 000000000000..d52eaf5f1f55
> --- /dev/null
> +++ b/tools/tests/domid/test-domid.c
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Unit tests for domain ID allocator.
> + *
> + * Copyright 2025 Ford Motor Company
> + */
> +
> +/* Local test include replicating hypervisor includes. */
> +#include <xen/domain.h>
I think this is a difficult to maintain position. Right now domid.c
only includes xen/domain.h, so you can easily replace this in
user-space. However if/when domid.c starts including more headers,
replicating this in user-space will be cumbersome IMO.
I would just guard the includes in domid.c with #ifdef __XEN__ for the
preprocessor to remove them when domid.c is compiled as part of the
unit-tests harness.
I usually include a harness.h that contains the glue to make the
imported code build (much like what you have placed in the test
harness xen/domain.h header.
> +
> +int main(int argc, char **argv)
> +{
> + domid_t expected, allocated;
> +
> + printf("DOMID_FIRST_RESERVED=%u DOMID_INVALID=%u\n",
> + DOMID_FIRST_RESERVED, DOMID_INVALID);
> +
> + /* Test ID#0 cannot be allocated twice. */
> + allocated = domid_alloc(0);
> + printf("TEST 1: expected %u allocated %u\n", 0, allocated);
> + ASSERT(allocated == 0);
> + allocated = domid_alloc(0);
> + printf("TEST 1: expected %u allocated %u\n", DOMID_INVALID, allocated);
> + ASSERT(allocated == DOMID_INVALID);
> +
> + /* Ensure ID is not allocated. */
> + domid_free(0);
> +
> + /*
> + * Test that that two consecutive calls of domid_alloc(DOMID_INVALID)
> + * will never return the same ID.
> + * NB: ID#0 is reserved and shall not be allocated by
> + * domid_alloc(DOMID_INVALID).
> + */
> + for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
> + {
> + allocated = domid_alloc(DOMID_INVALID);
> + printf("TEST 2: expected %u allocated %u\n", expected, allocated);
> + ASSERT(allocated == expected);
> + }
> + for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
> + {
> + allocated = domid_alloc(DOMID_INVALID);
> + printf("TEST 3: expected %u allocated %u\n", DOMID_INVALID, allocated);
> + ASSERT(allocated == DOMID_INVALID);
> + }
> +
> + /* Re-allocate first ID from [1..DOMID_FIRST_RESERVED - 1]. */
> + expected = 1;
> + domid_free(1);
> + allocated = domid_alloc(DOMID_INVALID);
> + printf("TEST 4: expected %u allocated %u\n", expected, allocated);
> + ASSERT(allocated == expected);
> +
> + /* Re-allocate last ID from [1..DOMID_FIRST_RESERVED - 1]. */
> + expected = DOMID_FIRST_RESERVED - 1;
> + domid_free(DOMID_FIRST_RESERVED - 1);
> + allocated = domid_alloc(DOMID_INVALID);
> + printf("TEST 5: expected %u allocated %u\n", expected, allocated);
> + ASSERT(allocated == expected);
> +
> + /* Allocate an invalid ID. */
> + expected = DOMID_INVALID;
> + allocated = domid_alloc(DOMID_FIRST_RESERVED);
> + printf("TEST 6: expected %u allocated %u\n", expected, allocated);
> + ASSERT(allocated == expected);
I would make this a bit less chatty maybe?
I think you only need to print on errors, and you probably don't want
to ASSERT() on failure, and rather try to finish all the tests in
order to report multiple failures in a single run.
Thanks, Roger.
On Tue, Aug 05, 2025 at 04:15:28PM +0200, Roger Pau Monné wrote:
> On Wed, Jul 30, 2025 at 05:41:00PM +0000, dmkhn@proton.me wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Introduce some basic infrastructure for doing domain ID allocation unit tests,
> > and add a few tests that ensure correctness of the domain ID allocator.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v12:
> > - fixed Makefile
> > - dropped unused symbols/includes from the test harness header
> > - s/printk/printf/g in the test code
> > ---
> > tools/tests/Makefile | 2 +-
> > tools/tests/domid/.gitignore | 2 +
> > tools/tests/domid/Makefile | 48 ++++++++++
> > tools/tests/domid/include/xen/domain.h | 126 +++++++++++++++++++++++++
> > tools/tests/domid/test-domid.c | 78 +++++++++++++++
> > 5 files changed, 255 insertions(+), 1 deletion(-)
> > create mode 100644 tools/tests/domid/.gitignore
> > create mode 100644 tools/tests/domid/Makefile
> > create mode 100644 tools/tests/domid/include/xen/domain.h
> > create mode 100644 tools/tests/domid/test-domid.c
> >
> > diff --git a/tools/tests/Makefile b/tools/tests/Makefile
> > index 36928676a666..ff1666425436 100644
> > --- a/tools/tests/Makefile
> > +++ b/tools/tests/Makefile
> > @@ -1,7 +1,7 @@
> > XEN_ROOT = $(CURDIR)/../..
> > include $(XEN_ROOT)/tools/Rules.mk
> >
> > -SUBDIRS-y :=
> > +SUBDIRS-y := domid
> > SUBDIRS-y += resource
> > SUBDIRS-$(CONFIG_X86) += cpu-policy
> > SUBDIRS-$(CONFIG_X86) += tsx
> > diff --git a/tools/tests/domid/.gitignore b/tools/tests/domid/.gitignore
> > new file mode 100644
> > index 000000000000..70e306b3c074
> > --- /dev/null
> > +++ b/tools/tests/domid/.gitignore
> > @@ -0,0 +1,2 @@
> > +*.o
> > +test-domid
> > diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
> > new file mode 100644
> > index 000000000000..08fbad096aec
> > --- /dev/null
> > +++ b/tools/tests/domid/Makefile
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Unit tests for domain ID allocator.
> > +#
> > +# Copyright 2025 Ford Motor Company
> > +
> > +XEN_ROOT=$(CURDIR)/../../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +TESTS := test-domid
> > +
> > +vpath domid.c $(XEN_ROOT)/xen/common/
> > +
> > +.PHONY: all
> > +all: $(TESTS)
> > +
> > +.PHONY: run
> > +run: $(TESTS)
> > + $(foreach t,$(TESTS),./$(t);)
> > +
> > +.PHONY: clean
> > +clean:
> > + $(RM) -- *.o $(TESTS) $(DEPS_RM)
> > +
> > +.PHONY: distclean
> > +distclean: clean
> > + $(RM) -- *~
> > +
> > +.PHONY: install
> > +install: all
> > + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests
> > + $(INSTALL_PROG) test-domid $(DESTDIR)$(LIBEXEC)/tests
> > +
> > +.PHONY: uninstall
> > +uninstall:
> > + $(RM) -- $(DESTDIR)$(LIBEXEC)/tests/test-domid
> > +
> > +CFLAGS += -D__XEN_TOOLS__
> > +CFLAGS += $(APPEND_CFLAGS)
> > +CFLAGS += $(CFLAGS_xeninclude)
> > +CFLAGS += -I./include/
> > +
> > +LDFLAGS += $(APPEND_LDFLAGS)
> > +
> > +test-domid: domid.o test-domid.o
> > + $(CC) $^ -o $@ $(LDFLAGS)
> > +
> > +-include $(DEPS_INCLUDE)
> > diff --git a/tools/tests/domid/include/xen/domain.h b/tools/tests/domid/include/xen/domain.h
> > new file mode 100644
> > index 000000000000..e5db0235445e
> > --- /dev/null
> > +++ b/tools/tests/domid/include/xen/domain.h
> > @@ -0,0 +1,126 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Unit test harness for domain ID allocator.
> > + *
> > + * Copyright 2025 Ford Motor Company
> > + */
> > +
> > +#ifndef _TEST_HARNESS_
> > +#define _TEST_HARNESS_
> > +
> > +#include <assert.h>
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +
> > +#include <xen-tools/common-macros.h>
> > +
> > +#define BUG_ON(x) assert(!(x))
> > +#define ASSERT(x) assert(x)
> > +
> > +#define DOMID_FIRST_RESERVED (10)
> > +#define DOMID_INVALID (11)
> > +
> > +#define DEFINE_SPINLOCK(x) unsigned long *(x)
>
> I think this shouldn't be a pointer? As you otherwise trigger a NULL
> pointer dereference in the increases and decreases done below?
Sorry, this bitops integration is very raw.
Thanks for all your suggestions, I reworked it.
>
> > +#define spin_lock(x) ((*(x))++)
> > +#define spin_unlock(x) ((*(x))--)
>
> FWIW, I would use a plain bool:
>
> #define DEFINE_SPINLOCK(l) bool l
> #define spin_lock(l) (*(l) = true)
> #define spin_unlock(l) (*(l) = false)
>
> As you don't expect concurrency tests, you could even assert the lock
> is in the expected state before taking/releasing it.
>
> > +
> > +#define printk printf
> > +
> > +#define BITS_PER_LONG sizeof(unsigned long)
>
> That's BYTES_PER_LONG, BITS_PER_LONG would be (sizeof(unsigned long) * 8).
>
> > +#define BITS_PER_WORD (8U * BITS_PER_LONG)
> > +#define BITS_TO_LONGS(bits) \
> > + (((bits) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> > +#define DECLARE_BITMAP(name, bits) \
> > + unsigned long name[BITS_TO_LONGS(bits)]
> > +
> > +static inline int __test_and_set_bit(unsigned int nr, unsigned long *addr)
> > +{
> > + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> > + unsigned long *p = addr + (nr / BITS_PER_WORD);
> > + int old = (*p & mask) != 0;
> > +
> > + *p |= mask;
> > +
> > + return old;
> > +}
> > +
> > +static inline int __test_and_clear_bit(unsigned int nr, unsigned long *addr)
> > +{
> > + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> > + unsigned long *p = addr + (nr / BITS_PER_WORD);
> > + int old = (*p & mask) != 0;
> > +
> > + *p &= ~mask;
> > +
> > + return old;
> > +}
>
> Could you somehow use the generic__test_and_set_bit() and
> generic__test_and_clear_bit() implementations in bitops.h?
I tried that originally, and it pulls a lot of dependencies from xen/bitops.h;
that will be a mini project to compile xen/bitops.h for the host, which I
think I can skip doing for the purpose of this test.
I followed another approach as discussed offline in matrix: re-purpose
tools/libs/ctrl/xc_bitops.h which seems to be working nice!
>
> > +
> > +static inline void __set_bit(unsigned int nr, volatile unsigned long *addr)
> > +{
> > + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> > + unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
>
> Why do you need the cast to drop the volatile here?
>
> > +
> > + *p |= mask;
> > +}
>
> I think you could possibly simplify to a single line:
>
> ((unsigned int *)addr)[nr >> 5] |= (1u << (nr & 31));
>
> That's the implementation of constant_set_bit() in x86.
>
> > +
> > +static inline void __clear_bit(unsigned int nr, volatile unsigned long *addr)
> > +{
> > + unsigned long mask = 1UL << (nr % BITS_PER_WORD);
> > + unsigned long *p = (unsigned long *)addr + (nr / BITS_PER_WORD);
> > +
> > + *p &= ~mask;
> > +}
>
> I don't think you need __clear_bit()? It's not used by domid.c AFAICT.
Overlooked, thanks.
>
> > +
> > +static inline unsigned long find_next_zero_bit(const unsigned long *addr,
> > + unsigned long size,
> > + unsigned long offset)
> > +{
> > + unsigned long idx = offset / BITS_PER_WORD;
> > + unsigned long bit = offset % BITS_PER_WORD;
> > +
> > + if (offset >= size)
> > + return size;
> > +
> > + while (offset < size)
> > + {
> > + unsigned long val = addr[idx] | (~0UL >> (BITS_PER_WORD - bit));
> > +
> > + if (~val)
> > + {
> > + unsigned long pos = __builtin_ffsl(~val);
> > +
> > + if (pos > 0)
> > + {
> > + unsigned long rc = idx * BITS_PER_WORD + (pos - 1);
> > +
> > + if (rc < size)
> > + return rc;
> > + }
> > + }
> > +
> > + offset = (idx + 1) * BITS_PER_WORD;
> > + idx++;
> > + bit = 0;
> > + }
> > +
> > + return size;
> > +}
>
> Hm, you need a full find_next_zero_bit() implementation here because
> addr can be arbitrarily long. Could you somehow include
> xen/lib/find-next-bit.c and set the right defines so only the
> implementation of find_next_bit() is included?
That's a good idea!
xen/lib/find-next-bit.c seems to be integrating pretty simple.
Thanks for the hint!
>
> > +
> > +typedef bool spinlock_t;
>
> You want to put this ahead, so that DEFINE_SPINLOCK can be:
>
> #define DEFINE_SPINLOCK(l) spinlock_t l
>
> > +typedef uint16_t domid_t;
> > +
> > +/* See include/xen/domain.h */
> > +extern domid_t domid_alloc(domid_t domid);
> > +extern void domid_free(domid_t domid);
> > +
> > +#endif /* _TEST_HARNESS_ */
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/tools/tests/domid/test-domid.c b/tools/tests/domid/test-domid.c
> > new file mode 100644
> > index 000000000000..d52eaf5f1f55
> > --- /dev/null
> > +++ b/tools/tests/domid/test-domid.c
> > @@ -0,0 +1,78 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Unit tests for domain ID allocator.
> > + *
> > + * Copyright 2025 Ford Motor Company
> > + */
> > +
> > +/* Local test include replicating hypervisor includes. */
> > +#include <xen/domain.h>
>
> I think this is a difficult to maintain position. Right now domid.c
> only includes xen/domain.h, so you can easily replace this in
> user-space. However if/when domid.c starts including more headers,
> replicating this in user-space will be cumbersome IMO.
>
> I would just guard the includes in domid.c with #ifdef __XEN__ for the
> preprocessor to remove them when domid.c is compiled as part of the
> unit-tests harness.
>
> I usually include a harness.h that contains the glue to make the
> imported code build (much like what you have placed in the test
> harness xen/domain.h header.
I like that there's no need to modify the tested code.
I reworked this slightly differently: local include/xen/domain.h is a symlink
to local harness.h, and all future dependendent files will be symlinks to
harness.h as well.
>
> > +
> > +int main(int argc, char **argv)
> > +{
> > + domid_t expected, allocated;
> > +
> > + printf("DOMID_FIRST_RESERVED=%u DOMID_INVALID=%u\n",
> > + DOMID_FIRST_RESERVED, DOMID_INVALID);
> > +
> > + /* Test ID#0 cannot be allocated twice. */
> > + allocated = domid_alloc(0);
> > + printf("TEST 1: expected %u allocated %u\n", 0, allocated);
> > + ASSERT(allocated == 0);
> > + allocated = domid_alloc(0);
> > + printf("TEST 1: expected %u allocated %u\n", DOMID_INVALID, allocated);
> > + ASSERT(allocated == DOMID_INVALID);
> > +
> > + /* Ensure ID is not allocated. */
> > + domid_free(0);
> > +
> > + /*
> > + * Test that that two consecutive calls of domid_alloc(DOMID_INVALID)
> > + * will never return the same ID.
> > + * NB: ID#0 is reserved and shall not be allocated by
> > + * domid_alloc(DOMID_INVALID).
> > + */
> > + for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
> > + {
> > + allocated = domid_alloc(DOMID_INVALID);
> > + printf("TEST 2: expected %u allocated %u\n", expected, allocated);
> > + ASSERT(allocated == expected);
> > + }
> > + for ( expected = 1; expected < DOMID_FIRST_RESERVED; expected++ )
> > + {
> > + allocated = domid_alloc(DOMID_INVALID);
> > + printf("TEST 3: expected %u allocated %u\n", DOMID_INVALID, allocated);
> > + ASSERT(allocated == DOMID_INVALID);
> > + }
> > +
> > + /* Re-allocate first ID from [1..DOMID_FIRST_RESERVED - 1]. */
> > + expected = 1;
> > + domid_free(1);
> > + allocated = domid_alloc(DOMID_INVALID);
> > + printf("TEST 4: expected %u allocated %u\n", expected, allocated);
> > + ASSERT(allocated == expected);
> > +
> > + /* Re-allocate last ID from [1..DOMID_FIRST_RESERVED - 1]. */
> > + expected = DOMID_FIRST_RESERVED - 1;
> > + domid_free(DOMID_FIRST_RESERVED - 1);
> > + allocated = domid_alloc(DOMID_INVALID);
> > + printf("TEST 5: expected %u allocated %u\n", expected, allocated);
> > + ASSERT(allocated == expected);
> > +
> > + /* Allocate an invalid ID. */
> > + expected = DOMID_INVALID;
> > + allocated = domid_alloc(DOMID_FIRST_RESERVED);
> > + printf("TEST 6: expected %u allocated %u\n", expected, allocated);
> > + ASSERT(allocated == expected);
>
> I would make this a bit less chatty maybe?
Ack.
>
> I think you only need to print on errors, and you probably don't want
> to ASSERT() on failure, and rather try to finish all the tests in
> order to report multiple failures in a single run.
I thought about it originally, but my "tests" depend on each other, so I'll
keep failing on the first error as is, if there's no strong objection.
>
> Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.