Message ID | 20221020102706.29267-1-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [for-4.17?] test/vpci: enable by default | expand |
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
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,
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
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.
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-%
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(-)