Replace uses of the LAPIC_ID() macro with accesses to the
cpu_to_apicid[] lookup table. This table contains the APIC IDs of each
vCPU as probed at runtime rather than assuming a predefined relation.

Moved smp_initialise() ahead of apic_setup() in order to initialise
cpu_to_apicid ASAP and avoid using it uninitialised. Note that bringing
up the APs doesn't need the APIC in hvmloader becasue it always runs
virtualized and uses the PV interface.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v1->v2:
  * No changes
Changes wrt original series
  * No changes (it was wrongly stated in v1 that something did. That was part
    of the following patch)

---
 tools/firmware/hvmloader/config.h    | 3 ++-
 tools/firmware/hvmloader/hvmloader.c | 6 +++---
 tools/firmware/hvmloader/mp_tables.c | 2 +-
 tools/firmware/hvmloader/util.c      | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index cd716bf39245..6e1da137d779 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
 
 #define IOAPIC_ID           0x01
 
+extern uint32_t *cpu_to_apicid;
+
 #define LAPIC_BASE_ADDRESS  0xfee00000
-#define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
 
 #define PCI_ISA_DEVFN       0x08    /* dev 1, fn 0 */
 #define PCI_ISA_IRQ_MASK    0x0c20U /* ISA IRQs 5,10,11 are PCI connected */
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index f8af88fabf24..4e330fc1e241 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -224,7 +224,7 @@ static void apic_setup(void)
 
     /* 8259A ExtInts are delivered through IOAPIC pin 0 (Virtual Wire Mode). */
     ioapic_write(0x10, APIC_DM_EXTINT);
-    ioapic_write(0x11, SET_APIC_ID(LAPIC_ID(0)));
+    ioapic_write(0x11, SET_APIC_ID(cpu_to_apicid[0]));
 }
 
 struct bios_info {
@@ -341,11 +341,11 @@ int main(void)
 
     printf("CPU speed is %u MHz\n", get_cpu_mhz());
 
+    smp_initialise();
+
     apic_setup();
     pci_setup();
 
-    smp_initialise();
-
     perform_tests();
 
     if ( bios->bios_info_setup )
diff --git a/tools/firmware/hvmloader/mp_tables.c b/tools/firmware/hvmloader/mp_tables.c
index 77d3010406d0..3c93a5c947d9 100644
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -199,7 +199,7 @@ static void fill_mp_config_table(struct mp_config_table *mpct, int length)
 static void fill_mp_proc_entry(struct mp_proc_entry *mppe, int vcpu_id)
 {
     mppe->type = ENTRY_TYPE_PROCESSOR;
-    mppe->lapic_id = LAPIC_ID(vcpu_id);
+    mppe->lapic_id = cpu_to_apicid[vcpu_id];
     mppe->lapic_version = 0x11;
     mppe->cpu_flags = CPU_FLAG_ENABLED;
     if ( vcpu_id == 0 )
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index d3b3f9038e64..2d07ce129013 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -827,7 +827,7 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt,
 
 static uint32_t acpi_lapic_id(unsigned cpu)
 {
-    return LAPIC_ID(cpu);
+    return cpu_to_apicid[cpu];
 }
 
 void hvmloader_acpi_build_tables(struct acpi_config *config,
-- 
2.48.1

On 04.02.2025 15:45, Alejandro Vallejo wrote:
> --- a/tools/firmware/hvmloader/config.h
> +++ b/tools/firmware/hvmloader/config.h
> @@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
>  
>  #define IOAPIC_ID           0x01
>  
> +extern uint32_t *cpu_to_apicid;

Strictly speaking this ought to be part of the earlier patch. If hvmloader
was Misra-checked, this would be a (transient) violation.

config.h is also somewhat odd a place to put this declaration, yet then I
can't really suggest anything better.

Jan

On Tue Feb 4, 2025 at 3:07 PM GMT, Jan Beulich wrote:
> On 04.02.2025 15:45, Alejandro Vallejo wrote:
> > --- a/tools/firmware/hvmloader/config.h
> > +++ b/tools/firmware/hvmloader/config.h
> > @@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
> >  
> >  #define IOAPIC_ID           0x01
> >  
> > +extern uint32_t *cpu_to_apicid;
>
> Strictly speaking this ought to be part of the earlier patch. If hvmloader
> was Misra-checked, this would be a (transient) violation.

Hmmm. I don't see it. The previous patch is fully contained in smp.c and this
extern isn't required until now. Does MISRA have mandates on non-static symbols
not present in headers?

The global could be static in patch1, but seems silly seeing how it'd be undone
here.

>
> config.h is also somewhat odd a place to put this declaration, yet then I
> can't really suggest anything better.
>
> Jan

Any header will do but there's no better one I could find, and I'd rather not
create a new one just for this.

Cheers,
Alejandro

On 04.02.2025 16:25, Alejandro Vallejo wrote:
> On Tue Feb 4, 2025 at 3:07 PM GMT, Jan Beulich wrote:
>> On 04.02.2025 15:45, Alejandro Vallejo wrote:
>>> --- a/tools/firmware/hvmloader/config.h
>>> +++ b/tools/firmware/hvmloader/config.h
>>> @@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
>>>  
>>>  #define IOAPIC_ID           0x01
>>>  
>>> +extern uint32_t *cpu_to_apicid;
>>
>> Strictly speaking this ought to be part of the earlier patch. If hvmloader
>> was Misra-checked, this would be a (transient) violation.
> 
> Hmmm. I don't see it. The previous patch is fully contained in smp.c and this
> extern isn't required until now. Does MISRA have mandates on non-static symbols
> not present in headers?

Every non-static definition is expected to have exactly one earlier
declaration.

Jan

On Tue Feb 4, 2025 at 3:46 PM GMT, Jan Beulich wrote:
> On 04.02.2025 16:25, Alejandro Vallejo wrote:
> > On Tue Feb 4, 2025 at 3:07 PM GMT, Jan Beulich wrote:
> >> On 04.02.2025 15:45, Alejandro Vallejo wrote:
> >>> --- a/tools/firmware/hvmloader/config.h
> >>> +++ b/tools/firmware/hvmloader/config.h
> >>> @@ -48,8 +48,9 @@ extern uint8_t ioapic_version;
> >>>  
> >>>  #define IOAPIC_ID           0x01
> >>>  
> >>> +extern uint32_t *cpu_to_apicid;
> >>
> >> Strictly speaking this ought to be part of the earlier patch. If hvmloader
> >> was Misra-checked, this would be a (transient) violation.
> > 
> > Hmmm. I don't see it. The previous patch is fully contained in smp.c and this
> > extern isn't required until now. Does MISRA have mandates on non-static symbols
> > not present in headers?
>
> Every non-static definition is expected to have exactly one earlier
> declaration.
>
> Jan

I had no idea. Fair enough then, I'll adjust...

Cheers,
Alejandro