| Message ID | 20190815183803.13346-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc: Fix die-id validation and compatibility with libvirt | expand |
Reviewed-by: Vanderson M. do Rosario <vandersonmr2@gmail.com> Vanderson M. Rosario On Thu, Aug 15, 2019 at 3:48 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > The error message for die-id range validation is incorrect. Example: > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 > qemu-system-x86_64: -device > qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ > Invalid CPU die-id: 1 must be in range 0:5 > > The actual range for die-id in this example is 0:0. > > Fix the error message to use smp_dies and print the correct range. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/i386/pc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 549c437050..24b03bb49c 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler > *hotplug_dev, > return; > } else if (cpu->die_id > pcms->smp_dies - 1) { > error_setg(errp, "Invalid CPU die-id: %u must be in range > 0:%u", > - cpu->die_id, max_socket); > + cpu->die_id, pcms->smp_dies - 1); > return; > } > if (cpu->core_id < 0) { > -- > 2.21.0 > > >
Hi, On 2019/8/16 2:38, Eduardo Habkost wrote: > The error message for die-id range validation is incorrect. Example: > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 > qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ > Invalid CPU die-id: 1 must be in range 0:5 > > The actual range for die-id in this example is 0:0. There is one die per socket by default. The case sockets=6 means there are 6 dies by default and the range for die-id is 0:5. > > Fix the error message to use smp_dies and print the correct range. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/i386/pc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 549c437050..24b03bb49c 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, > return; > } else if (cpu->die_id > pcms->smp_dies - 1) { > error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", > - cpu->die_id, max_socket); > + cpu->die_id, pcms->smp_dies - 1); > return; > } > if (cpu->core_id < 0) { >
Eduardo Habkost <ehabkost@redhat.com> writes: > The error message for die-id range validation is incorrect. Example: > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 > qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ > Invalid CPU die-id: 1 must be in range 0:5 The error message makes me wonder whether the upper bound is inclusive. > The actual range for die-id in this example is 0:0. Aha. I guess it is. Hmm... $ git-grep 'must be between' \*.c block/qcow.c: error_setg(errp, "Cluster size must be between 512 and 64k"); block/qcow.c: error_setg(errp, "L2 table size must be between 512 and 64k"); hw/arm/armsse.c: error_setg(errp, "SRAM_ADDR_WIDTH must be between 1 and %d", hw/block/block.c: error_setg(errp, "cyls must be between 1 and %u", cyls_max); hw/block/block.c: error_setg(errp, "heads must be between 1 and %u", heads_max); hw/block/block.c: error_setg(errp, "secs must be between 1 and %u", secs_max); hw/block/virtio-blk.c: ", must be between 1 and %d", hw/block/virtio-blk.c: "), must be between 1 and %d", hw/net/rocker/rocker_of_dpa.c: DPRINTF("New vlan_id (%d) must be between 1 and 4095\n", hw/net/virtio-net.c: error_setg(errp, "'speed' must be between 0 and INT_MAX"); hw/nvram/spapr_nvram.c: "spapr-nvram must be between %" PRId64 hw/timer/a9gtimer.c: error_setg(errp, "%s: num-cpu must be between 1 and %d", hw/timer/arm_mptimer.c: error_setg(errp, "num-cpu must be between 1 and %d", hw/usb/hcd-ehci.c: error_setg(errp, "firstport must be between 0 and %u", net/slirp.c: "(IPv6 prefix length must be between 0 and 126)"); tests/test-throttle.c: /* burst_length must be between 1 and THROTTLE_VALUE_MAX */ util/keyval.c: * The length of any key-fragment must be between 1 and 127. > Fix the error message to use smp_dies and print the correct range. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/i386/pc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 549c437050..24b03bb49c 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, > return; > } else if (cpu->die_id > pcms->smp_dies - 1) { > error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", > - cpu->die_id, max_socket); > + cpu->die_id, pcms->smp_dies - 1); > return; > } > if (cpu->core_id < 0) {
On Fri, Aug 16, 2019 at 09:04:16AM +0800, Like Xu wrote: > Hi, > > On 2019/8/16 2:38, Eduardo Habkost wrote: > > The error message for die-id range validation is incorrect. Example: > > > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > > -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 > > qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ > > Invalid CPU die-id: 1 must be in range 0:5 > > > > The actual range for die-id in this example is 0:0. > > There is one die per socket by default. > > The case sockets=6 means there are 6 dies by default > and the range for die-id is 0:5. > I don't understand why you say that. die-id supposed to identify a die inside a socket. The code below is already checking for (cpu->die_id > pcms->smp_dies - 1) because of that. > > > > Fix the error message to use smp_dies and print the correct range. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > hw/i386/pc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 549c437050..24b03bb49c 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, > > return; > > } else if (cpu->die_id > pcms->smp_dies - 1) { > > error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", > > - cpu->die_id, max_socket); > > + cpu->die_id, pcms->smp_dies - 1); > > return; > > } > > if (cpu->core_id < 0) { > > >
On Thu, 15 Aug 2019 15:38:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > The error message for die-id range validation is incorrect. Example: > > $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ > -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 > qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ > Invalid CPU die-id: 1 must be in range 0:5 > > The actual range for die-id in this example is 0:0. > > Fix the error message to use smp_dies and print the correct range. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> PS: there is another unrelated bug, QEMU crashes if run like this: qemu-system-x86_64 -smp 1,sockets=6,dies=0,maxcpus=6 > --- > hw/i386/pc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 549c437050..24b03bb49c 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, > return; > } else if (cpu->die_id > pcms->smp_dies - 1) { > error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", > - cpu->die_id, max_socket); > + cpu->die_id, pcms->smp_dies - 1); > return; > } > if (cpu->core_id < 0) {
On 2019/8/16 21:49, Eduardo Habkost wrote: > On Fri, Aug 16, 2019 at 09:04:16AM +0800, Like Xu wrote: >> Hi, >> >> On 2019/8/16 2:38, Eduardo Habkost wrote: >>> The error message for die-id range validation is incorrect. Example: >>> >>> $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ >>> -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 >>> qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ >>> Invalid CPU die-id: 1 must be in range 0:5 >>> >>> The actual range for die-id in this example is 0:0. >> >> There is one die per socket by default. >> >> The case sockets=6 means there are 6 dies by default >> and the range for die-id is 0:5. >> > > I don't understand why you say that. die-id supposed to identify > a die inside a socket. The code below is already checking for > (cpu->die_id > pcms->smp_dies - 1) because of that. > You're right about this. Sorry to make a mess to support die topology. > >>> >>> Fix the error message to use smp_dies and print the correct range. >>> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >>> --- >>> hw/i386/pc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >>> index 549c437050..24b03bb49c 100644 >>> --- a/hw/i386/pc.c >>> +++ b/hw/i386/pc.c >>> @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, >>> return; >>> } else if (cpu->die_id > pcms->smp_dies - 1) { >>> error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", >>> - cpu->die_id, max_socket); >>> + cpu->die_id, pcms->smp_dies - 1); >>> return; >>> } >>> if (cpu->core_id < 0) { >>> >> >
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 549c437050..24b03bb49c 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2412,7 +2412,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, return; } else if (cpu->die_id > pcms->smp_dies - 1) { error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u", - cpu->die_id, max_socket); + cpu->die_id, pcms->smp_dies - 1); return; } if (cpu->core_id < 0) {
The error message for die-id range validation is incorrect. Example: $ qemu-system-x86_64 -smp 1,sockets=6,maxcpus=6 \ -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0 qemu-system-x86_64: -device qemu64-x86_64-cpu,socket-id=1,die-id=1,core-id=0,thread-id=0: \ Invalid CPU die-id: 1 must be in range 0:5 The actual range for die-id in this example is 0:0. Fix the error message to use smp_dies and print the correct range. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/i386/pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)