Introduce an install target, like it's used by other tests.  This
allows running the test on the installed systems, which is easier than
running it during the build phase when dealing with automated testing.
Strictly speaking the vpci test doesn't require to be run on a Xen
host currently, but that allows easier integration with logic that
runs the rest of the tests.

While there also adjust the makefile to use $(RM), and rename the
resulting binary to use a dash instead of an underscore (again to
match the rest of the tests).

Since the resulting test binary is now part of the distribution CC
must be used instead of HOSTCC, together with CFLAGS.  The usage of
CFLAGS requires removing two local unused variables and converting
pci_get_pdev() into a function in order for the 'd' local variable
in vpci_{read,write} to be used, or else the build of the test
fails.

Finally adjust the `run` target to only run the test if HOSTCC is CC,
else print a warning message.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
XenRT has recently gained the ability to run the tests in tools/tests
that are installed, so the install target is needed for that use-case.
---
Changes since v1:
 - Adjust run target to take HOSTCC vs CC into account.
 - Use CFLAGS.
 - Remove two unused variables.
 - Convert pci_get_pdev() into a function.
---
 tools/tests/vpci/Makefile | 18 ++++++++++++++----
 tools/tests/vpci/emul.h   |  7 ++++++-
 tools/tests/vpci/main.c   |  2 --
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
index 5075bc2be2..11f1ee7126 100644
--- a/tools/tests/vpci/Makefile
+++ b/tools/tests/vpci/Makefile
@@ -1,27 +1,37 @@
 XEN_ROOT=$(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-TARGET := test_vpci
+TARGET := test-vpci
 
 .PHONY: all
 all: $(TARGET)
 
 .PHONY: run
 run: $(TARGET)
+ifeq ($(CC),$(HOSTCC))
 	./$(TARGET)
+else
+	$(warning HOSTCC != CC, cannot run test)
+endif
 
 $(TARGET): vpci.c vpci.h list.h main.c emul.h
-	$(HOSTCC) -g -o $@ vpci.c main.c
+	$(CC) $(CFLAGS) -o $@ vpci.c main.c
 
 .PHONY: clean
 clean:
-	rm -rf $(TARGET) *.o *~ vpci.h vpci.c list.h
+	$(RM) -- $(TARGET) *.o *~ vpci.h vpci.c list.h
 
 .PHONY: distclean
 distclean: clean
 
 .PHONY: install
-install:
+install: all
+	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
+	$(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN)
+
+.PHONY: uninstall
+uninstall:
+	$(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET)
 
 vpci.c: $(XEN_ROOT)/xen/drivers/vpci/vpci.c
 	# Remove includes and add the test harness header
diff --git a/tools/tests/vpci/emul.h b/tools/tests/vpci/emul.h
index f03e3a56d1..6baab20de5 100644
--- a/tools/tests/vpci/emul.h
+++ b/tools/tests/vpci/emul.h
@@ -92,7 +92,12 @@ typedef union {
 #define xmalloc(type) ((type *)malloc(sizeof(type)))
 #define xfree(p) free(p)
 
-#define pci_get_pdev(...) (&test_pdev)
+const static inline struct pci_dev *pci_get_pdev(const struct domain *d,
+                                                 pci_sbdf_t sbdf)
+{
+    return &test_pdev;
+}
+
 #define pci_get_ro_map(...) NULL
 
 #define test_bit(...) false
diff --git a/tools/tests/vpci/main.c b/tools/tests/vpci/main.c
index b9a0a6006b..e3335a0962 100644
--- a/tools/tests/vpci/main.c
+++ b/tools/tests/vpci/main.c
@@ -154,8 +154,6 @@ main(int argc, char **argv)
     uint16_t r20[2] = { };
     uint32_t r24 = 0;
     uint8_t r28, r30;
-    unsigned int i;
-    int rc;
 
     INIT_LIST_HEAD(&vpci.handlers);
     spin_lock_init(&vpci.lock);
-- 
2.39.0

On Mon, Mar 13, 2023 at 01:12:26PM +0100, Roger Pau Monne wrote:
> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
> index 5075bc2be2..11f1ee7126 100644
> --- a/tools/tests/vpci/Makefile
> +++ b/tools/tests/vpci/Makefile
> @@ -1,27 +1,37 @@
>  XEN_ROOT=$(CURDIR)/../../..
>  include $(XEN_ROOT)/tools/Rules.mk
>  
> -TARGET := test_vpci
> +TARGET := test-vpci
>  
>  .PHONY: all
>  all: $(TARGET)
>  
>  .PHONY: run
>  run: $(TARGET)
> +ifeq ($(CC),$(HOSTCC))
>  	./$(TARGET)
> +else
> +	$(warning HOSTCC != CC, cannot run test)
> +endif
>  
>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
> -	$(HOSTCC) -g -o $@ vpci.c main.c
> +	$(CC) $(CFLAGS) -o $@ vpci.c main.c

This now needs $(CFLAGS_xeninclude) to build, so
"CFLAGS += $(CFLAGS_xeninclude)" somewhere in the file.


Also, there's another change needed as we've got this error:

vpci.c:344:29: error: ‘dom_xen’ undeclared (first use in this function)
  344 |         pdev = pci_get_pdev(dom_xen, sbdf);


Otherwise, patch looks fine to me.

Thanks,

-- 
Anthony PERARD