[PATCH v2 0/4] Various convenience fixes

[PATCH v2 0/4] Various convenience fixes

[Andre Przywara]

As I found myself using kvmtool more often in the last weeks, I got more
and more annoyed by some smaller "ticks" that kvmtool shows.

So this is an attempt to post various smaller fixes I gathered up over
the years, but never found worth enough to send out.
Version 2 drops the two patches that have been merged already, and
amends the remaining ones.

The patches address:
- add /chosen/stdout-path node in .dtb
- honour make -s switch
- remove pointless kvmtool version number
- introduce autodetection of supported GIC type

Changelog v1..v2:
- stdout-path: Let hw/serial.c set the node path and use an alias
- make -s: adapt to newest version from the Linux kernel
- kvmtool "version" number: unchanged, but verified to be compatible
  with the Debian package
- GIC autodetection: ignore --force-pci, always try to have an MSI
  controller

Please have a look!

Cheers,
Andre.

Andre Przywara (4):
  arm: fdt: add stdout-path to /chosen node
  Makefile: support -s switch
  Makefile: Remove echoing of kvmtools version file
  arm: Auto-detect guest GIC type

 Makefile                     |  6 +++++-
 arm/fdt.c                    | 10 ++++++++++
 arm/gic.c                    | 25 +++++++++++++++++++++++++
 arm/include/arm-common/gic.h |  1 +
 hw/serial.c                  |  9 +++++++++
 include/kvm/fdt.h            |  2 ++
 util/KVMTOOLS-VERSION-GEN    |  1 -
 7 files changed, 52 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] arm: fdt: add stdout-path to /chosen node

[Andre Przywara]

The DT spec describes the stdout-path property in the /chosen node to
contain the DT path for a default device usable for outputting characters.
The Linux kernel uses this for earlycon (without further parameters),
other DT users might rely on this as well.

Add a stdout-path property pointing to the "serial0" alias, then add an
aliases node at the end of the FDT, containing the actual path. This
allows the FDT generation code in hw/serial.c to set this string.

Even when we use the virtio console, the serial console is still there
and works, so we can expose this unconditionally. Putting the virtio
console path in there will not work anyway.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/fdt.c         | 10 ++++++++++
 hw/serial.c       |  9 +++++++++
 include/kvm/fdt.h |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/arm/fdt.c b/arm/fdt.c
index 7c50464a..624dbace 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -145,6 +145,7 @@ static int setup_fdt(struct kvm *kvm)
 					 kvm->cfg.real_cmdline));
 
 	_FDT(fdt_property_u64(fdt, "kaslr-seed", kvm->cfg.arch.kaslr_seed));
+	_FDT(fdt_property_string(fdt, "stdout-path", "serial0"));
 
 	/* Initrd */
 	if (kvm->arch.initrd_size != 0) {
@@ -210,6 +211,15 @@ static int setup_fdt(struct kvm *kvm)
 	_FDT(fdt_property_cell(fdt, "migrate", fns->migrate));
 	_FDT(fdt_end_node(fdt));
 
+	if (fdt_stdout_path) {
+		_FDT(fdt_begin_node(fdt, "aliases"));
+		_FDT(fdt_property_string(fdt, "serial0", fdt_stdout_path));
+		_FDT(fdt_end_node(fdt));
+
+		free(fdt_stdout_path);
+		fdt_stdout_path = NULL;
+	}
+
 	/* Finalise. */
 	_FDT(fdt_end_node(fdt));
 	_FDT(fdt_finish(fdt));
diff --git a/hw/serial.c b/hw/serial.c
index 2f19ba80..13c4663e 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -366,6 +366,9 @@ static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
 }
 
 #ifdef CONFIG_HAS_LIBFDT
+
+char *fdt_stdout_path = NULL;
+
 #define DEVICE_NAME_MAX_LEN 32
 static
 void serial8250_generate_fdt_node(struct ioport *ioport, void *fdt,
@@ -383,6 +386,12 @@ void serial8250_generate_fdt_node(struct ioport *ioport, void *fdt,
 
 	snprintf(dev_name, DEVICE_NAME_MAX_LEN, "U6_16550A@%llx", addr);
 
+	if (!fdt_stdout_path) {
+		fdt_stdout_path = malloc(strlen(dev_name) + 2);
+		/* Assumes that this node is a child of the root node. */
+		sprintf(fdt_stdout_path, "/%s", dev_name);
+	}
+
 	_FDT(fdt_begin_node(fdt, dev_name));
 	_FDT(fdt_property_string(fdt, "compatible", "ns16550a"));
 	_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
index beadc7f3..4e615725 100644
--- a/include/kvm/fdt.h
+++ b/include/kvm/fdt.h
@@ -25,6 +25,8 @@ enum irq_type {
 	IRQ_TYPE_LEVEL_MASK	= (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH),
 };
 
+extern char *fdt_stdout_path;
+
 /* Helper for the various bits of code that generate FDT nodes */
 #define _FDT(exp)							\
 	do {								\
-- 
2.17.1

[PATCH v2 2/4] Makefile: support -s switch

[Andre Przywara]

"make -s" suppresses normal output, just shows warnings and errors.
But since we explicitly override the make output with our fancy concise
version, we miss out on this feature.

Do as the kernel does and explicitly suppress every normal output when -s
is given. This helps to spot warnings that scroll out of the terminal
window too quickly.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c4faff66..ec75cd99 100644
--- a/Makefile
+++ b/Makefile
@@ -3,7 +3,11 @@
 #
 
 ifeq ($(strip $(V)),)
-	E = @echo
+	ifeq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
+		E = @echo
+	else
+		E = @\#
+	endif
 	Q = @
 else
 	E = @\#
-- 
2.17.1

[PATCH v2 3/4] Makefile: Remove echoing of kvmtools version file

[Andre Przywara]

On every build we report the kvmtool "version" number, which isn't
meaningful at all anymore.

Remove the line from the KVMTOOLS-VERSION-GEN script to drop a
pointless message.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 util/KVMTOOLS-VERSION-GEN | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util/KVMTOOLS-VERSION-GEN b/util/KVMTOOLS-VERSION-GEN
index f0dcfdea..91ee2c2f 100755
--- a/util/KVMTOOLS-VERSION-GEN
+++ b/util/KVMTOOLS-VERSION-GEN
@@ -35,6 +35,5 @@ else
 	VC=unset
 fi
 test "$VN" = "$VC" || {
-	echo >&2 "KVMTOOLS_VERSION = $VN"
 	echo "KVMTOOLS_VERSION = $VN" >$GVF
 }
-- 
2.17.1

[PATCH v2 4/4] arm: Auto-detect guest GIC type

[Andre Przywara]

At the moment kvmtool always tries to instantiate a virtual GICv2 for
the guest, and fails with some scary error message if that doesn't work.
The user has then to manually specify "--irqchip=gicv3", which is not
really obvious.
With the advent of more GICv3-only machines, let's try to be more
clever and implement some auto-detection of the GIC type needed:
We try gicv3-its, gicv3, gicv2m and gicv2, in that order. That first one
succeeding wins.
For GICv2 machines the first two will always fail. For GICv2-backwards
compatible GICv3 machines GICv3 is probably the better choice these days.

This algorithm is in effect is there is no explicit --irqchip parameter
on the command line. We still allow the GIC type to be set explicitly.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c                    | 25 +++++++++++++++++++++++++
 arm/include/arm-common/gic.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index abcbcc09..a86da20e 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -182,6 +182,8 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 		gic_device.type = KVM_DEV_TYPE_ARM_VGIC_V3;
 		dist_attr.attr  = KVM_VGIC_V3_ADDR_TYPE_DIST;
 		break;
+	case IRQCHIP_AUTO:
+		return -ENODEV;
 	}
 
 	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
@@ -199,6 +201,8 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 	case IRQCHIP_GICV3:
 		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &redist_attr);
 		break;
+	case IRQCHIP_AUTO:
+		return -ENODEV;
 	}
 	if (err)
 		goto out_err;
@@ -249,9 +253,30 @@ static int gic__create_irqchip(struct kvm *kvm)
 
 int gic__create(struct kvm *kvm, enum irqchip_type type)
 {
+	enum irqchip_type try;
 	int err;
 
 	switch (type) {
+	case IRQCHIP_AUTO:
+		try = IRQCHIP_GICV3_ITS;
+		err = gic__create(kvm, try);
+		if (err) {
+			try = IRQCHIP_GICV3;
+			err = gic__create(kvm, try);
+		}
+		if (err) {
+			try = IRQCHIP_GICV2M;
+			err = gic__create(kvm, try);
+		}
+		if (err) {
+			try = IRQCHIP_GICV2;
+			err = gic__create(kvm, try);
+		}
+		if (err)
+			return err;
+
+		kvm->cfg.arch.irqchip = try;
+		return 0;
 	case IRQCHIP_GICV2M:
 		gic_msi_size = KVM_VGIC_V2M_SIZE;
 		gic_msi_base = ARM_GIC_CPUI_BASE - gic_msi_size;
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index 1125d601..ec9cf31a 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -24,6 +24,7 @@
 #define KVM_VGIC_V2M_SIZE		0x1000
 
 enum irqchip_type {
+	IRQCHIP_AUTO,
 	IRQCHIP_GICV2,
 	IRQCHIP_GICV2M,
 	IRQCHIP_GICV3,
-- 
2.17.1

Re: [PATCH v2 4/4] arm: Auto-detect guest GIC type

[Will Deacon]

On Fri, Feb 01, 2019 at 12:37:16PM +0000, Andre Przywara wrote:
> At the moment kvmtool always tries to instantiate a virtual GICv2 for
> the guest, and fails with some scary error message if that doesn't work.
> The user has then to manually specify "--irqchip=gicv3", which is not
> really obvious.
> With the advent of more GICv3-only machines, let's try to be more
> clever and implement some auto-detection of the GIC type needed:
> We try gicv3-its, gicv3, gicv2m and gicv2, in that order. That first one
> succeeding wins.
> For GICv2 machines the first two will always fail. For GICv2-backwards
> compatible GICv3 machines GICv3 is probably the better choice these days.
> 
> This algorithm is in effect is there is no explicit --irqchip parameter
> on the command line. We still allow the GIC type to be set explicitly.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arm/gic.c                    | 25 +++++++++++++++++++++++++
>  arm/include/arm-common/gic.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index abcbcc09..a86da20e 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -182,6 +182,8 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
>  		gic_device.type = KVM_DEV_TYPE_ARM_VGIC_V3;
>  		dist_attr.attr  = KVM_VGIC_V3_ADDR_TYPE_DIST;
>  		break;
> +	case IRQCHIP_AUTO:
> +		return -ENODEV;
>  	}
>  
>  	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
> @@ -199,6 +201,8 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
>  	case IRQCHIP_GICV3:
>  		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &redist_attr);
>  		break;
> +	case IRQCHIP_AUTO:
> +		return -ENODEV;
>  	}
>  	if (err)
>  		goto out_err;
> @@ -249,9 +253,30 @@ static int gic__create_irqchip(struct kvm *kvm)
>  
>  int gic__create(struct kvm *kvm, enum irqchip_type type)
>  {
> +	enum irqchip_type try;
>  	int err;
>  
>  	switch (type) {
> +	case IRQCHIP_AUTO:
> +		try = IRQCHIP_GICV3_ITS;
> +		err = gic__create(kvm, try);
> +		if (err) {
> +			try = IRQCHIP_GICV3;
> +			err = gic__create(kvm, try);
> +		}
> +		if (err) {
> +			try = IRQCHIP_GICV2M;
> +			err = gic__create(kvm, try);
> +		}
> +		if (err) {
> +			try = IRQCHIP_GICV2;
> +			err = gic__create(kvm, try);
> +		}
> +		if (err)
> +			return err;

Easier to write this as a descending loop over the irqchip_type enum?

Will

Re: [PATCH v2 0/4] Various convenience fixes

[Will Deacon]

On Fri, Feb 01, 2019 at 12:37:12PM +0000, Andre Przywara wrote:
> As I found myself using kvmtool more often in the last weeks, I got more
> and more annoyed by some smaller "ticks" that kvmtool shows.
> 
> So this is an attempt to post various smaller fixes I gathered up over
> the years, but never found worth enough to send out.
> Version 2 drops the two patches that have been merged already, and
> amends the remaining ones.
> 
> The patches address:
> - add /chosen/stdout-path node in .dtb
> - honour make -s switch
> - remove pointless kvmtool version number
> - introduce autodetection of supported GIC type
> 
> Changelog v1..v2:
> - stdout-path: Let hw/serial.c set the node path and use an alias
> - make -s: adapt to newest version from the Linux kernel
> - kvmtool "version" number: unchanged, but verified to be compatible
>   with the Debian package
> - GIC autodetection: ignore --force-pci, always try to have an MSI
>   controller
> 
> Please have a look!
> 
> Cheers,
> Andre.
> 
> Andre Przywara (4):
>   arm: fdt: add stdout-path to /chosen node
>   Makefile: support -s switch
>   Makefile: Remove echoing of kvmtools version file

I've applied these three, and left a comment on the other one.

Will