* [PATCH kvmtool 0/4] Makefile and virtio fixes
@ 2022-07-22 14:17 Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 1/4] Makefile: Add missing build dependencies Jean-Philippe Brucker
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-07-22 14:17 UTC (permalink / raw)
To: will; +Cc: kvm, suzuki.poulose, sami.mujawar, Jean-Philippe Brucker
A few small fixes for kvmtool:
Patch 1 fixes an annoying issue when building kvmtool after updating
without a make clean.
Patch 2 enables passing ARCH=i386 and ARCH=x86_64.
Patch 3 fixes an issue with INTx on modern virtio-pci. That code was
written when INTx were edge-triggered, but they are now level-triggered.
Patch 4 fixes an uninitialized allocation error.
Jean-Philippe Brucker (4):
Makefile: Add missing build dependencies
Makefile: Fix ARCH override
virtio/pci: Deassert IRQ line on ISR read
virtio/rng: Zero-initialize the device
Makefile | 7 ++++---
virtio/pci-modern.c | 5 +----
virtio/rng.c | 2 +-
3 files changed, 6 insertions(+), 8 deletions(-)
--
2.37.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH kvmtool 1/4] Makefile: Add missing build dependencies
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
@ 2022-07-22 14:17 ` Jean-Philippe Brucker
2022-07-25 10:26 ` Alexandru Elisei
2022-07-22 14:17 ` [PATCH kvmtool 2/4] Makefile: Fix ARCH override Jean-Philippe Brucker
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-07-22 14:17 UTC (permalink / raw)
To: will; +Cc: kvm, suzuki.poulose, sami.mujawar, Jean-Philippe Brucker
When running kvmtool after updating without doing a make clean, one
might run into strange issues such as:
Warning: Failed init: symbol_init
Fatal: Initialisation failed
or worse. This happens because symbol.o is not automatically rebuilt
after a change of headers, because .symbol.o.d is not in the $(DEPS)
variable. So if the layout of struct kvm_config changes, for example,
symbols.o that was built for an older version will try to read
kvm->vmlinux from the wrong location in struct kvm, and lkvm will die.
Add all .d files to $(DEPS). Also include $(STATIC_DEPS) which was
previously set but not used.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 1f9903d8..f0df76f4 100644
--- a/Makefile
+++ b/Makefile
@@ -383,7 +383,7 @@ comma = ,
# The dependency file for the current target
depfile = $(subst $(comma),_,$(dir $@).$(notdir $@).d)
-DEPS := $(foreach obj,$(OBJS),\
+DEPS := $(foreach obj,$(OBJS) $(OBJS_DYNOPT) $(OTHEROBJS) $(GUEST_OBJS),\
$(subst $(comma),_,$(dir $(obj)).$(notdir $(obj)).d))
DEFINES += -D_FILE_OFFSET_BITS=64
@@ -590,6 +590,7 @@ cscope:
# Escape redundant work on cleaning up
ifneq ($(MAKECMDGOALS),clean)
-include $(DEPS)
+-include $(STATIC_DEPS)
KVMTOOLS-VERSION-FILE:
@$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT)
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH kvmtool 2/4] Makefile: Fix ARCH override
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 1/4] Makefile: Add missing build dependencies Jean-Philippe Brucker
@ 2022-07-22 14:17 ` Jean-Philippe Brucker
2022-07-25 11:06 ` Alexandru Elisei
2022-07-22 14:17 ` [PATCH kvmtool 3/4] virtio/pci: Deassert IRQ line on ISR read Jean-Philippe Brucker
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-07-22 14:17 UTC (permalink / raw)
To: will; +Cc: kvm, suzuki.poulose, sami.mujawar, Jean-Philippe Brucker
Variables set on the command-line are not overridden by normal
assignments. So when passing ARCH=x86_64 on the command-line, build
fails:
Makefile:227: *** This architecture (x86_64) is not supported in kvmtool.
Use the 'override' directive to force the ARCH reassignment.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index f0df76f4..faae0da2 100644
--- a/Makefile
+++ b/Makefile
@@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
-e s/riscv64/riscv/ -e s/riscv32/riscv/)
ifeq ($(ARCH),i386)
- ARCH := x86
+ override ARCH = x86
DEFINES += -DCONFIG_X86_32
endif
ifeq ($(ARCH),x86_64)
- ARCH := x86
+ override ARCH = x86
DEFINES += -DCONFIG_X86_64
ARCH_PRE_INIT = x86/init.S
endif
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH kvmtool 3/4] virtio/pci: Deassert IRQ line on ISR read
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 1/4] Makefile: Add missing build dependencies Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 2/4] Makefile: Fix ARCH override Jean-Philippe Brucker
@ 2022-07-22 14:17 ` Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 4/4] virtio/rng: Zero-initialize the device Jean-Philippe Brucker
2022-08-04 15:02 ` [PATCH kvmtool 0/4] Makefile and virtio fixes Will Deacon
4 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-07-22 14:17 UTC (permalink / raw)
To: will; +Cc: kvm, suzuki.poulose, sami.mujawar, Jean-Philippe Brucker
Since commit 2108c86d0623 ("virtio/pci: Signal INTx interrupts as level
instead of edge"), virtio uses level-triggered IRQs. Bring the modern
device up to date, by deasserting the IRQ line when the guest reads the
interrupt status register.
Fixes: 3bf79498e6d5 ("virtio: Add support for modern virtio-pci")
Reported-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
virtio/pci-modern.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/virtio/pci-modern.c b/virtio/pci-modern.c
index 753e95bd..c5b4bc50 100644
--- a/virtio/pci-modern.c
+++ b/virtio/pci-modern.c
@@ -245,10 +245,7 @@ static bool virtio_pci__isr_read(struct virtio_device *vdev,
return false;
ioport__write8(data, vpci->isr);
- /*
- * Interrupts are edge triggered (yes, going against the PCI and virtio
- * specs), so no need to deassert the IRQ line.
- */
+ kvm__irq_line(vpci->kvm, vpci->legacy_irq_line, VIRTIO_IRQ_LOW);
vpci->isr = 0;
return 0;
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH kvmtool 4/4] virtio/rng: Zero-initialize the device
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
` (2 preceding siblings ...)
2022-07-22 14:17 ` [PATCH kvmtool 3/4] virtio/pci: Deassert IRQ line on ISR read Jean-Philippe Brucker
@ 2022-07-22 14:17 ` Jean-Philippe Brucker
2022-08-04 15:02 ` [PATCH kvmtool 0/4] Makefile and virtio fixes Will Deacon
4 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-07-22 14:17 UTC (permalink / raw)
To: will; +Cc: kvm, suzuki.poulose, sami.mujawar, Jean-Philippe Brucker
Use calloc() to avoid uninitialized fields in the rng device.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
virtio/rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virtio/rng.c b/virtio/rng.c
index f9d607f6..63ab8fce 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -162,7 +162,7 @@ int virtio_rng__init(struct kvm *kvm)
if (!kvm->cfg.virtio_rng)
return 0;
- rdev = malloc(sizeof(*rdev));
+ rdev = calloc(1, sizeof(*rdev));
if (rdev == NULL)
return -ENOMEM;
--
2.37.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 1/4] Makefile: Add missing build dependencies
2022-07-22 14:17 ` [PATCH kvmtool 1/4] Makefile: Add missing build dependencies Jean-Philippe Brucker
@ 2022-07-25 10:26 ` Alexandru Elisei
2022-08-09 9:52 ` Jean-Philippe Brucker
0 siblings, 1 reply; 12+ messages in thread
From: Alexandru Elisei @ 2022-07-25 10:26 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: will, kvm, suzuki.poulose, sami.mujawar
Hi,
On Fri, Jul 22, 2022 at 03:17:29PM +0100, Jean-Philippe Brucker wrote:
> When running kvmtool after updating without doing a make clean, one
> might run into strange issues such as:
>
> Warning: Failed init: symbol_init
> Fatal: Initialisation failed
>
> or worse. This happens because symbol.o is not automatically rebuilt
> after a change of headers, because .symbol.o.d is not in the $(DEPS)
> variable. So if the layout of struct kvm_config changes, for example,
> symbols.o that was built for an older version will try to read
> kvm->vmlinux from the wrong location in struct kvm, and lkvm will die.
>
> Add all .d files to $(DEPS). Also include $(STATIC_DEPS) which was
> previously set but not used.
This makes sense to me. And hopefully it should fix some weird errors that
went away for me by doing a make clean before a rebuild.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 1f9903d8..f0df76f4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -383,7 +383,7 @@ comma = ,
> # The dependency file for the current target
> depfile = $(subst $(comma),_,$(dir $@).$(notdir $@).d)
>
> -DEPS := $(foreach obj,$(OBJS),\
> +DEPS := $(foreach obj,$(OBJS) $(OBJS_DYNOPT) $(OTHEROBJS) $(GUEST_OBJS),\
Checked and those are indeed all the objects.
> $(subst $(comma),_,$(dir $(obj)).$(notdir $(obj)).d))
>
> DEFINES += -D_FILE_OFFSET_BITS=64
> @@ -590,6 +590,7 @@ cscope:
> # Escape redundant work on cleaning up
> ifneq ($(MAKECMDGOALS),clean)
> -include $(DEPS)
> +-include $(STATIC_DEPS)
In the spirit in keeping the makefile as small as possible and reading
fewer files, maybe STATIC_DEPS should be included only if the target is
$(PROGRAM)-static.
Regardless of how you want to handle that, the patch looks correct to me:
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Thanks,
Alex
>
> KVMTOOLS-VERSION-FILE:
> @$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT)
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 2/4] Makefile: Fix ARCH override
2022-07-22 14:17 ` [PATCH kvmtool 2/4] Makefile: Fix ARCH override Jean-Philippe Brucker
@ 2022-07-25 11:06 ` Alexandru Elisei
2022-08-09 9:56 ` Jean-Philippe Brucker
0 siblings, 1 reply; 12+ messages in thread
From: Alexandru Elisei @ 2022-07-25 11:06 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: will, kvm, suzuki.poulose, sami.mujawar
Hi,
Thank you for fixing this, I've come across it several times in the past.
On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote:
> Variables set on the command-line are not overridden by normal
> assignments. So when passing ARCH=x86_64 on the command-line, build
> fails:
>
> Makefile:227: *** This architecture (x86_64) is not supported in kvmtool.
>
> Use the 'override' directive to force the ARCH reassignment.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f0df76f4..faae0da2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> -e s/riscv64/riscv/ -e s/riscv32/riscv/)
>
> ifeq ($(ARCH),i386)
As far as I know, there are several versions of the x86 architecture: i386,
i486, i586 and i686.
It looks rather arbitrary to have i386 as a valid ARCH value, but not the other
ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64,
like the kernel.
Regardless, the patch looks good to me:
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Thanks,
Alex
> - ARCH := x86
> + override ARCH = x86
> DEFINES += -DCONFIG_X86_32
> endif
> ifeq ($(ARCH),x86_64)
> - ARCH := x86
> + override ARCH = x86
> DEFINES += -DCONFIG_X86_64
> ARCH_PRE_INIT = x86/init.S
> endif
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 0/4] Makefile and virtio fixes
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
` (3 preceding siblings ...)
2022-07-22 14:17 ` [PATCH kvmtool 4/4] virtio/rng: Zero-initialize the device Jean-Philippe Brucker
@ 2022-08-04 15:02 ` Will Deacon
2022-08-09 10:05 ` Jean-Philippe Brucker
4 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2022-08-04 15:02 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: catalin.marinas, kernel-team, Will Deacon, sami.mujawar, kvm,
suzuki.poulose
On Fri, 22 Jul 2022 15:17:28 +0100, Jean-Philippe Brucker wrote:
> A few small fixes for kvmtool:
>
> Patch 1 fixes an annoying issue when building kvmtool after updating
> without a make clean.
>
> Patch 2 enables passing ARCH=i386 and ARCH=x86_64.
>
> [...]
I wasn't sure whether you were going to respin patch 2 based on Alexandru's
comment, but I actually ran into the legacy IRQ issue so I went ahead and
applied the series. I can, of course, queue extra stuff on top if you like!
Applied to kvmtool (master), thanks!
[1/4] Makefile: Add missing build dependencies
https://git.kernel.org/will/kvmtool/c/3863f34bd767
[2/4] Makefile: Fix ARCH override
https://git.kernel.org/will/kvmtool/c/ae22ac7a81e5
[3/4] virtio/pci: Deassert IRQ line on ISR read
https://git.kernel.org/will/kvmtool/c/fe2182731b72
[4/4] virtio/rng: Zero-initialize the device
https://git.kernel.org/will/kvmtool/c/6c88c26f701f
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 1/4] Makefile: Add missing build dependencies
2022-07-25 10:26 ` Alexandru Elisei
@ 2022-08-09 9:52 ` Jean-Philippe Brucker
0 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-08-09 9:52 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: will, kvm, suzuki.poulose, sami.mujawar
On Mon, Jul 25, 2022 at 11:26:15AM +0100, Alexandru Elisei wrote:
> > @@ -590,6 +590,7 @@ cscope:
> > # Escape redundant work on cleaning up
> > ifneq ($(MAKECMDGOALS),clean)
> > -include $(DEPS)
> > +-include $(STATIC_DEPS)
>
> In the spirit in keeping the makefile as small as possible and reading
> fewer files, maybe STATIC_DEPS should be included only if the target is
> $(PROGRAM)-static.
Right this adds a measurable overhead, about 2-5% on a make invocation
without anything to do. It's not noticeable since the feature tests still
take most of the time, but optimizing it may be worthwhile
Thanks,
Jean
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 2/4] Makefile: Fix ARCH override
2022-07-25 11:06 ` Alexandru Elisei
@ 2022-08-09 9:56 ` Jean-Philippe Brucker
2022-08-09 15:38 ` Alexandru Elisei
0 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-08-09 9:56 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: will, kvm, suzuki.poulose, sami.mujawar
On Mon, Jul 25, 2022 at 12:06:38PM +0100, Alexandru Elisei wrote:
> Hi,
>
> Thank you for fixing this, I've come across it several times in the past.
>
> On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote:
> > Variables set on the command-line are not overridden by normal
> > assignments. So when passing ARCH=x86_64 on the command-line, build
> > fails:
> >
> > Makefile:227: *** This architecture (x86_64) is not supported in kvmtool.
> >
> > Use the 'override' directive to force the ARCH reassignment.
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> > Makefile | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index f0df76f4..faae0da2 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> > -e s/riscv64/riscv/ -e s/riscv32/riscv/)
> >
> > ifeq ($(ARCH),i386)
>
> As far as I know, there are several versions of the x86 architecture: i386,
> i486, i586 and i686.
The discovery from "uname -m" does "sed -e s/i.86/i386/" to catch those
> It looks rather arbitrary to have i386 as a valid ARCH value, but not the other
> ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64,
> like the kernel.
"i386" does seem to be the conventional way of selecting 32-bit x86 in the
kernel, since it has a i386_defconfig selected by ARCH=i386.
>
> Regardless, the patch looks good to me:
>
> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Thanks!
Jean
>
> Thanks,
> Alex
>
> > - ARCH := x86
> > + override ARCH = x86
> > DEFINES += -DCONFIG_X86_32
> > endif
> > ifeq ($(ARCH),x86_64)
> > - ARCH := x86
> > + override ARCH = x86
> > DEFINES += -DCONFIG_X86_64
> > ARCH_PRE_INIT = x86/init.S
> > endif
> > --
> > 2.37.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 0/4] Makefile and virtio fixes
2022-08-04 15:02 ` [PATCH kvmtool 0/4] Makefile and virtio fixes Will Deacon
@ 2022-08-09 10:05 ` Jean-Philippe Brucker
0 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-08-09 10:05 UTC (permalink / raw)
To: Will Deacon
Cc: catalin.marinas, kernel-team, sami.mujawar, kvm, suzuki.poulose
On Thu, Aug 04, 2022 at 04:02:27PM +0100, Will Deacon wrote:
> On Fri, 22 Jul 2022 15:17:28 +0100, Jean-Philippe Brucker wrote:
> > A few small fixes for kvmtool:
> >
> > Patch 1 fixes an annoying issue when building kvmtool after updating
> > without a make clean.
> >
> > Patch 2 enables passing ARCH=i386 and ARCH=x86_64.
> >
> > [...]
>
> I wasn't sure whether you were going to respin patch 2 based on Alexandru's
> comment, but I actually ran into the legacy IRQ issue so I went ahead and
> applied the series. I can, of course, queue extra stuff on top if you like!
Right I was away, thanks for picking those up. I don't think patch 2 needs
any change but I may send an optimization for patch 1
Thanks,
Jean
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH kvmtool 2/4] Makefile: Fix ARCH override
2022-08-09 9:56 ` Jean-Philippe Brucker
@ 2022-08-09 15:38 ` Alexandru Elisei
0 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2022-08-09 15:38 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: will, kvm, suzuki.poulose, sami.mujawar
Hi,
On Tue, Aug 09, 2022 at 10:56:50AM +0100, Jean-Philippe Brucker wrote:
> On Mon, Jul 25, 2022 at 12:06:38PM +0100, Alexandru Elisei wrote:
> > Hi,
> >
> > Thank you for fixing this, I've come across it several times in the past.
> >
> > On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote:
> > > Variables set on the command-line are not overridden by normal
> > > assignments. So when passing ARCH=x86_64 on the command-line, build
> > > fails:
> > >
> > > Makefile:227: *** This architecture (x86_64) is not supported in kvmtool.
> > >
> > > Use the 'override' directive to force the ARCH reassignment.
> > >
> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > ---
> > > Makefile | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index f0df76f4..faae0da2 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> > > -e s/riscv64/riscv/ -e s/riscv32/riscv/)
> > >
> > > ifeq ($(ARCH),i386)
> >
> > As far as I know, there are several versions of the x86 architecture: i386,
> > i486, i586 and i686.
>
> The discovery from "uname -m" does "sed -e s/i.86/i386/" to catch those
I was referring when the user sets ARCH to something other than i386:
$ make ARCH=i486 clean
Makefile:233: *** This architecture (i486) is not supported in kvmtool. Stop.
Looks like that's because ARCH is assigned to the output of that uname -m
oneliner with the ?= operator, which according to gnu make: "This is called
a conditional variable assignment operator, because it only has an effect
if the variable is not yet defined".
But yeah, I checked for the kernel and only i386 works (never compiled the
kernel for something other than x86_64), so I guess it's OK to leave
kvmtool's Makefile as it is.
Thanks,
Alex
>
> > It looks rather arbitrary to have i386 as a valid ARCH value, but not the other
> > ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64,
> > like the kernel.
>
> "i386" does seem to be the conventional way of selecting 32-bit x86 in the
> kernel, since it has a i386_defconfig selected by ARCH=i386.
>
> >
> > Regardless, the patch looks good to me:
> >
> > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
>
> Thanks!
> Jean
>
> >
> > Thanks,
> > Alex
> >
> > > - ARCH := x86
> > > + override ARCH = x86
> > > DEFINES += -DCONFIG_X86_32
> > > endif
> > > ifeq ($(ARCH),x86_64)
> > > - ARCH := x86
> > > + override ARCH = x86
> > > DEFINES += -DCONFIG_X86_64
> > > ARCH_PRE_INIT = x86/init.S
> > > endif
> > > --
> > > 2.37.1
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-08-09 15:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-22 14:17 [PATCH kvmtool 0/4] Makefile and virtio fixes Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 1/4] Makefile: Add missing build dependencies Jean-Philippe Brucker
2022-07-25 10:26 ` Alexandru Elisei
2022-08-09 9:52 ` Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 2/4] Makefile: Fix ARCH override Jean-Philippe Brucker
2022-07-25 11:06 ` Alexandru Elisei
2022-08-09 9:56 ` Jean-Philippe Brucker
2022-08-09 15:38 ` Alexandru Elisei
2022-07-22 14:17 ` [PATCH kvmtool 3/4] virtio/pci: Deassert IRQ line on ISR read Jean-Philippe Brucker
2022-07-22 14:17 ` [PATCH kvmtool 4/4] virtio/rng: Zero-initialize the device Jean-Philippe Brucker
2022-08-04 15:02 ` [PATCH kvmtool 0/4] Makefile and virtio fixes Will Deacon
2022-08-09 10:05 ` Jean-Philippe Brucker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).