Re: [PATCH 4/5] qrap: Introduce machine-specific PCI address base

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4370 bytes --]

On Fri, Feb 24, 2023 at 07:49:48PM +0100, Andrea Bolognani wrote:
11;rgb:ffff/ffff/ffff> For pc machines, devices are placed directly on pci.0 with
> addresses like
> 
>   bus=pci.0,addr=0xa
> 
> and in this case the existing code works correctly.
> 
> For q35 machines, however, a separate PCI bus is created for
> each devices using a pcie-root-port, and the resulting
> addresses look like
> 
>   bus=pci.9,addr=0x0
> 
> In this case, we need to treat PCI addresses as decimal, not
> hexadecimal, both when parsing and generating them.
> 
> This issue has gone unnoticed for a long time because it only
> shows up when enough PCI devices are present: for small
> numbers, decimal and hexadecimal overlap, masking the issue.
> 
> Reported-by: Alona Paz <alkaplan@redhat.com>
> Fixes: 5307faa05997 ("qrap: Strip network devices from command line, set them up according to machine")
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Clunky, but, whatever, it's qrap.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  qrap.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/qrap.c b/qrap.c
> index 3077944..27c12ed 100644
> --- a/qrap.c
> +++ b/qrap.c
> @@ -77,6 +77,7 @@ static const struct drop_arg {
>   * @template_post:	Suffix for device specification (last part of address)
>   * @template_json:		Device prefix for when JSON is used
>   * @template_json_post:	Device suffix for when JSON is used
> + * @base:		Base used for PCI addresses
>   * @first:		First usable PCI address
>   * @last:		Last usable PCI address
>   */
> @@ -87,6 +88,7 @@ static const struct pci_dev {
>  	char *template_post;
>  	char *template_json;
>  	char *template_json_post;
> +	int base;
>  	int first;
>  	int last;
>  } pci_devs[] = {
> @@ -94,19 +96,19 @@ static const struct pci_dev {
>  		"pc-q35", "virtio-net-pci",
>  		"bus=pci.", ",addr=0x0",
>  		"\"bus\":\"pci.", ",\"addr\":\"0x0\"",
> -		3, /* 2: hotplug bus */ 31
> +		10, 3, /* 2: hotplug bus */ 31
>  	},
>  	{
>  		"pc-", "virtio-net-pci",
>  		"bus=pci.0,addr=0x", "",
>  		"\"bus\":\"pci.0\",\"addr\":\"0x", "",
> -		2, /* 1: ISA bridge */ 31
> +		16, 2, /* 1: ISA bridge */ 31
>  	},
>  	{
>  		"s390-ccw", "virtio-net-ccw",
>  		"devno=fe.0.", "",
>  		"\"devno\":\"fe.0.", "",
> -		1, 16
> +		16, 1, 16
>  	},
>  	{ 0 },
>  };
> @@ -264,7 +266,7 @@ int main(int argc, char **argv)
>  			if (template) {
>  				long n;
>  
> -				n = strtol(p + strlen(template), NULL, 16);
> +				n = strtol(p + strlen(template), NULL, dev->base);
>  				if (!errno)
>  					addr_map |= (1 << n);
>  			}
> @@ -285,13 +287,25 @@ int main(int argc, char **argv)
>  	if (has_dev) {
>  		qemu_argv[qemu_argc++] = "-device";
>  		if (!has_json) {
> -			snprintf(dev_str, ARG_MAX,
> -			         "%s,%s%x%s,netdev=hostnet0,x-txburst=4096",
> -			         dev->name, dev->template, i, dev->template_post);
> +			if (dev->base == 16) {
> +				snprintf(dev_str, ARG_MAX,
> +				         "%s,%s%x%s,netdev=hostnet0,x-txburst=4096",
> +				         dev->name, dev->template, i, dev->template_post);
> +			} else if (dev->base == 10) {
> +				snprintf(dev_str, ARG_MAX,
> +				         "%s,%s%d%s,netdev=hostnet0,x-txburst=4096",
> +				         dev->name, dev->template, i, dev->template_post);
> +			}
>  		} else {
> -			snprintf(dev_str, ARG_MAX,
> -			         "{\"driver\":\"%s\",%s%x\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}",
> -			         dev->name, dev->template_json, i, dev->template_json_post);
> +			if (dev->base == 16) {
> +				snprintf(dev_str, ARG_MAX,
> +				         "{\"driver\":\"%s\",%s%x\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}",
> +				         dev->name, dev->template_json, i, dev->template_json_post);
> +			} else if (dev->base == 10) {
> +				snprintf(dev_str, ARG_MAX,
> +				         "{\"driver\":\"%s\",%s%d\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}",
> +				         dev->name, dev->template_json, i, dev->template_json_post);
> +			}
>  		}
>  		qemu_argv[qemu_argc++] = dev_str;
>  	}

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]