Re: [PATCH] i386/apic: Add hint on boot failure because of disabling x2APIC

[Zhao Liu <zhao1.liu@intel.com>]

Hi Philippe,

On Fri, Jun 07, 2024 at 08:17:36AM +0200, Philippe Mathieu-Daudé wrote:
> Date: Fri, 7 Jun 2024 08:17:36 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: Re: [PATCH] i386/apic: Add hint on boot failure because of
>  disabling x2APIC
> 
> On 6/6/24 16:08, Zhao Liu wrote:
> > Currently, the Q35 supports up to 4096 vCPUs (since v9.0), but for TCG
> > cases, if x2APIC is not actively enabled to boot more than 255 vCPUs (
> > e.g., qemu-system-i386 -M pc-q35-9.0 -smp 666), the following error is
> > reported:
> > 
> > Unexpected error in apic_common_set_id() at ../hw/intc/apic_common.c:449:
> > qemu-system-i386: APIC ID 255 requires x2APIC feature in CPU
> > Aborted (core dumped)
> > 
> > This error can be resolved by setting x2apic=on in -cpu. In order to
> > better help users deal with this scenario, add the error hint to
> > instruct users on how to enable the x2apic feature.
> 
> Why not automatically set x2apic=on in this case instead?

The default CPU model is qemu64 without x2APIC. I think it might be
necessary to update the version to add x2APIC in qemu64, and I'd like to
look into it again for any other potential issues.

In addition to adding x2APIC directly to the qemu64, this hint can also
help some users who want a large number of vCPUs based on other older
CPU models. Though, it's not very common but this hint would be helpful.

> > Then, the error
> > report becomes the following:
> > 
> > Unexpected error in apic_common_set_id() at ../hw/intc/apic_common.c:448:
> > qemu-system-i386: APIC ID 255 requires x2APIC feature in CPU
> > Try x2apic=on in -cpu.
> > Aborted (core dumped)
> > 
> > Note since @errp is &error_abort, error_append_hint() can't be applied
> > on @errp. And in order to separate the exact error message from the
> > (perhaps effectively) hint, adding a hint via error_append_hint() is
> > also necessary. Therefore, introduce @local_error in
> > apic_common_set_id() to handle both the error message and the error
> > hint.
> > 
> > Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   hw/intc/apic_common.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!