Re: [PATCH v2 2/5] hw/i386/apic: Defer error check from apic_get_class to kvm_apic_realize

[Philippe Mathieu-Daudé]

Hi Bernhard,

On 4/10/23 01:21, Bernhard Beschow wrote:
> Am 3. Oktober 2023 08:27:25 UTC schrieb "Philippe Mathieu-Daudé" 
> <philmd@linaro.org>:
> > apic_get_class() isn't supposed to fail. kvm_apic_realize() is
> > DeviceRealize() handler, which can fail. Defer the error check
> > to the latter.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > hw/i386/kvm/apic.c       | 5 +++++
> > target/i386/cpu-sysemu.c | 8 --------
> > 2 files changed, 5 insertions(+), 8 deletions(-)

                        "kvm-apic-msi", APIC_SPACE_SIZE);
> > diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
> > index 2375e48178..6a228c9178 100644
> > --- a/target/i386/cpu-sysemu.c
> > +++ b/target/i386/cpu-sysemu.c
> > @@ -253,10 +253,6 @@ APICCommonClass *apic_get_class(Error **errp)
> >
> >      /* TODO: in-kernel irqchip for hvf */
> >      if (kvm_enabled()) {
> > -        if (!kvm_irqchip_in_kernel()) {
> > -            error_setg(errp, "KVM does not support userspace APIC");
> > -            return NULL;
> > -        }
> >          apic_type = "kvm-apic";
> >      } else if (xen_enabled()) {
> >          apic_type = "xen-apic";
> > @@ -272,10 +268,6 @@ void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> >      APICCommonState *apic;
> >      APICCommonClass *apic_class = apic_get_class(errp);
> >
> > -    if (!apic_class) {
> > -        return;
> > -    }
> > -
>
> Did you intend to remove these lines in the next commit? There you're writing 
> to simplify x86_cpu_apic_create() which you're doing here already.

No: apic_get_class() doesn't return NULL anymore, so this is dead code.

> Best regards,
> Bernhard