1. Patchew
  2. Xen
  3. View series

[PATCH for-4.17?] test/vpci: enable by default

Roger Pau Monne posted 1 patch 1 year, 6 months ago
Failed in applying to current master (apply log)
tools/tests/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH for-4.17?] test/vpci: enable by default
Posted by Roger Pau Monne 1 year, 6 months ago 
CONFIG_HAS_PCI is not defined for the tools build, and as a result the
vpci harness would never get build.  Fix this by building it
unconditionally, there's nothing arch specific in it.

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
While not strictly a bugfix, I think it's worth adding this change to the
release in order to always build the vpci test hardness and prevent it
from bitrotting.
---
 tools/tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/Makefile b/tools/tests/Makefile
index 33e32730c4..d99146d56a 100644
--- a/tools/tests/Makefile
+++ b/tools/tests/Makefile
@@ -10,7 +10,7 @@ SUBDIRS-$(CONFIG_X86) += x86_emulator
 endif
 SUBDIRS-y += xenstore
 SUBDIRS-y += depriv
-SUBDIRS-$(CONFIG_HAS_PCI) += vpci
+SUBDIRS-y += vpci
 
 .PHONY: all clean install distclean uninstall
 all clean distclean install uninstall: %: subdirs-%
-- 
2.37.3
Re: [PATCH for-4.17?] test/vpci: enable by default
Posted by Andrew Cooper 1 year, 6 months ago 
On 20/10/2022 11:27, Roger Pau Monne wrote:
> CONFIG_HAS_PCI is not defined for the tools build, and as a result the
> vpci harness would never get build.  Fix this by building it
> unconditionally, there's nothing arch specific in it.
>
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> While not strictly a bugfix, I think it's worth adding this change to the
> release in order to always build the vpci test hardness and prevent it
> from bitrotting.
> ---
>  tools/tests/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/tests/Makefile b/tools/tests/Makefile
> index 33e32730c4..d99146d56a 100644
> --- a/tools/tests/Makefile
> +++ b/tools/tests/Makefile
> @@ -10,7 +10,7 @@ SUBDIRS-$(CONFIG_X86) += x86_emulator
>  endif
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += depriv
> -SUBDIRS-$(CONFIG_HAS_PCI) += vpci
> +SUBDIRS-y += vpci

I'm afraid this is only half the fix.  The other half is:

diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
index 5075bc2be28c..336904958f6a 100644
--- a/tools/tests/vpci/Makefile
+++ b/tools/tests/vpci/Makefile
@@ -22,6 +22,8 @@ distclean: clean
 
 .PHONY: install
 install:
+       $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
+       $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN)
 
 vpci.c: $(XEN_ROOT)/xen/drivers/vpci/vpci.c
        # Remove includes and add the test harness header

so it can actually get deployed somewhere useful.

~Andrew
Re: [PATCH for-4.17?] test/vpci: enable by default
Posted by Roger Pau Monné 1 year, 6 months ago 
On Fri, Oct 21, 2022 at 07:01:01PM +0000, Andrew Cooper wrote:
> On 20/10/2022 11:27, Roger Pau Monne wrote:
> > CONFIG_HAS_PCI is not defined for the tools build, and as a result the
> > vpci harness would never get build.  Fix this by building it
> > unconditionally, there's nothing arch specific in it.
> >
> > Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > While not strictly a bugfix, I think it's worth adding this change to the
> > release in order to always build the vpci test hardness and prevent it
> > from bitrotting.
> > ---
> >  tools/tests/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/tests/Makefile b/tools/tests/Makefile
> > index 33e32730c4..d99146d56a 100644
> > --- a/tools/tests/Makefile
> > +++ b/tools/tests/Makefile
> > @@ -10,7 +10,7 @@ SUBDIRS-$(CONFIG_X86) += x86_emulator
> >  endif
> >  SUBDIRS-y += xenstore
> >  SUBDIRS-y += depriv
> > -SUBDIRS-$(CONFIG_HAS_PCI) += vpci
> > +SUBDIRS-y += vpci
> 
> I'm afraid this is only half the fix.  The other half is:
> 
> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
> index 5075bc2be28c..336904958f6a 100644
> --- a/tools/tests/vpci/Makefile
> +++ b/tools/tests/vpci/Makefile
> @@ -22,6 +22,8 @@ distclean: clean
>  
>  .PHONY: install
>  install:
> +       $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
> +       $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN)
>  
>  vpci.c: $(XEN_ROOT)/xen/drivers/vpci/vpci.c
>         # Remove includes and add the test harness header
> 
> so it can actually get deployed somewhere useful.

For now I just wanted to get it to be built by default.  It wasn't
clear to me we want this installed, as it's a standalone unit test
that could be executed as part of the build phase (doesn't require
interaction with any hypercalls).

It's also currently built using HOSTCC, so installing on the target
would be wrong for cross-builds.

Thanks, Roger.
RE: [PATCH for-4.17?] test/vpci: enable by default
Posted by Henry Wang 1 year, 6 months ago 
Hi Roger,

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Subject: [PATCH for-4.17?] test/vpci: enable by default
> 
> CONFIG_HAS_PCI is not defined for the tools build, and as a result the
> vpci harness would never get build.  Fix this by building it
> unconditionally, there's nothing arch specific in it.
> 
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> While not strictly a bugfix, I think it's worth adding this change to the
> release in order to always build the vpci test hardness and prevent it
> from bitrotting.

Good point.

No problem from my side, but I think you need also Anthony's opinion
as he is the toolstack maintainer. If he also likes this idea, feel free to
add my:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

> ---
>  tools/tests/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/tests/Makefile b/tools/tests/Makefile
> index 33e32730c4..d99146d56a 100644
> --- a/tools/tests/Makefile
> +++ b/tools/tests/Makefile
> @@ -10,7 +10,7 @@ SUBDIRS-$(CONFIG_X86) += x86_emulator
>  endif
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += depriv
> -SUBDIRS-$(CONFIG_HAS_PCI) += vpci
> +SUBDIRS-y += vpci
> 
>  .PHONY: all clean install distclean uninstall
>  all clean distclean install uninstall: %: subdirs-%
> --
> 2.37.3
Re: [PATCH for-4.17?] test/vpci: enable by default
Posted by Anthony PERARD 1 year, 6 months ago 
On Thu, Oct 20, 2022 at 10:30:26AM +0000, Henry Wang wrote:
> Hi Roger,
> 
> > -----Original Message-----
> > From: Roger Pau Monne <roger.pau@citrix.com>
> > Subject: [PATCH for-4.17?] test/vpci: enable by default
> > 
> > CONFIG_HAS_PCI is not defined for the tools build, and as a result the
> > vpci harness would never get build.  Fix this by building it
> > unconditionally, there's nothing arch specific in it.
> > 
> > Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > While not strictly a bugfix, I think it's worth adding this change to the
> > release in order to always build the vpci test hardness and prevent it
> > from bitrotting.
> 
> Good point.
> 
> No problem from my side, but I think you need also Anthony's opinion
> as he is the toolstack maintainer.

This sounds fine to me, the risk is that the build could fail. But we
can easily revert the patch and reapply it at the next development
cycle.

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.