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 withaddresses 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(a)redhat.com> Fixes: 5307faa05997 ("qrap: Strip network devices from command line, set them up according to machine") Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>Clunky, but, whatever, it's qrap. Reviewed-by: David Gibson <david(a)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