Re: [PATCH v6 06/11] tools/libacpi: Use LUT of APIC IDs rather than function pointer

["Alejandro Vallejo" <alejandro.vallejo@cloud.com>]

On Wed Oct 9, 2024 at 3:25 PM BST, Jan Beulich wrote:
> On 01.10.2024 14:38, Alejandro Vallejo wrote:
> > @@ -148,7 +148,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt,
> >          lapic->length  = sizeof(*lapic);
> >          /* Processor ID must match processor-object IDs in the DSDT. */
> >          lapic->acpi_processor_id = i;
> > -        lapic->apic_id = config->lapic_id(i);
> > +        lapic->apic_id = config->cpu_to_apicid[i];
>
> Perhaps assert (like you do in an earlier patch) that the ID is small
> enough?

Actually, I just remembered why I didn't. libacpi is pulled into libxl, which
is integrated into libvirt. A failed assert here would kill the application,
which is not very nice.

HVM is already protected by the mp tables assert, so I'm not terribly worried
about it and, while PVH is not, it would crash pretty quickly due to the
corruption.

I'd rather have the domain crashing rather than virt-manager.

>
> > --- a/tools/libacpi/libacpi.h
> > +++ b/tools/libacpi/libacpi.h
> > @@ -84,7 +84,7 @@ struct acpi_config {
> >      unsigned long rsdp;
> >  
> >      /* x86-specific parameters */
> > -    uint32_t (*lapic_id)(unsigned cpu);
> > +    uint32_t *cpu_to_apicid; /* LUT mapping cpu id to (x2)APIC ID */
>
> const uint32_t *?
>
> > --- a/tools/libs/light/libxl_dom.c
> > +++ b/tools/libs/light/libxl_dom.c
> > @@ -1082,6 +1082,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
> >  
> >      dom->container_type = XC_DOM_HVM_CONTAINER;
> >  
> > +#if defined(__i386__) || defined(__x86_64__)
> > +    for ( uint32_t i = 0; i < info->max_vcpus; i++ )
>
> Plain unsigned int?
>
> Jan

Sigh... and this didn't have the libxl style either.

I really hate this style mix we have :/

Cheers,
Alejandro