As per the previous patch, the own APIC instance is already available in
apic_msr_{read,write}, and can be passed along. In apic_mem_{read,write}, the
own APIC instance is available as the opaque parameter, since it gets registered
when initializing the io_memory attribute. As a result, no
cpu_get_current_apic() is involved any longer.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/intc/apic.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index ba0eda3921..fee5201372 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -769,17 +769,11 @@ static void apic_timer(void *opaque)
     apic_timer_update(s, s->next_time);
 }
 
-static int apic_register_read(int index, uint64_t *value)
+static int apic_register_read(APICCommonState *s, int index, uint64_t *value)
 {
-    APICCommonState *s;
     uint32_t val;
     int ret = 0;
 
-    s = cpu_get_current_apic();
-    if (!s) {
-        return -1;
-    }
-
     switch(index) {
     case 0x02: /* id */
         if (is_x2apic_mode(s)) {
@@ -876,7 +870,7 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
     }
 
     index = (addr >> 4) & 0xff;
-    apic_register_read(index, &val);
+    apic_register_read(opaque, index, &val);
 
     return val;
 }
@@ -891,7 +885,7 @@ int apic_msr_read(APICCommonState *s, int index, uint64_t *val)
         return -1;
     }
 
-    return apic_register_read(index, val);
+    return apic_register_read(s, index, val);
 }
 
 static void apic_send_msi(MSIMessage *msi)
@@ -919,15 +913,8 @@ static void apic_send_msi(MSIMessage *msi)
     apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
 }
 
-static int apic_register_write(int index, uint64_t val)
+static int apic_register_write(APICCommonState *s, int index, uint64_t val)
 {
-    APICCommonState *s;
-
-    s = cpu_get_current_apic();
-    if (!s) {
-        return -1;
-    }
-
     trace_apic_register_write(index, val);
 
     switch(index) {
@@ -1073,7 +1060,7 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
         return;
     }
 
-    apic_register_write(index, val);
+    apic_register_write(opaque, index, val);
 }
 
 int apic_msr_write(APICCommonState *s, int index, uint64_t val)
@@ -1086,7 +1073,7 @@ int apic_msr_write(APICCommonState *s, int index, uint64_t val)
         return -1;
     }
 
-    return apic_register_write(index, val);
+    return apic_register_write(s, index, val);
 }
 
 static void apic_pre_save(APICCommonState *s)
-- 
2.51.1.dirty

17.10.2025 17:11, Bernhard Beschow wrote:
> ... In apic_mem_{read,write}, the
> own APIC instance is available as the opaque parameter

> diff --git a/hw/intc/apic.c b/hw/intc/apic.c

> @@ -876,7 +870,7 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
>       }
>   
>       index = (addr >> 4) & 0xff;
> -    apic_register_read(index, &val);
> +    apic_register_read(opaque, index, &val);

I think it would be better to use local variable here:

  APICCommonState *s = opaque;

and use it down the line.  Yes, there's just one usage, but it is
still clearer this way (in my opinion anyway).

Ditto in apic_mem_write.

But it's more a nitpick really.

Thanks,

/mjt

Am 17. Oktober 2025 14:58:50 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:
>17.10.2025 17:11, Bernhard Beschow wrote:
>> ... In apic_mem_{read,write}, the
>> own APIC instance is available as the opaque parameter
>
>> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>
>> @@ -876,7 +870,7 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
>>       }
>>         index = (addr >> 4) & 0xff;
>> -    apic_register_read(index, &val);
>> +    apic_register_read(opaque, index, &val);
>
>I think it would be better to use local variable here:
>
> APICCommonState *s = opaque;
>
>and use it down the line.  Yes, there's just one usage, but it is
>still clearer this way (in my opinion anyway).
>
>Ditto in apic_mem_write.

I agree. Will fix in the next iteration.

Best regards,
Bernhard

>
>But it's more a nitpick really.
>
>Thanks,
>
>/mjt

Am 17. Oktober 2025 19:34:36 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 17. Oktober 2025 14:58:50 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:
>>17.10.2025 17:11, Bernhard Beschow wrote:
>>> ... In apic_mem_{read,write}, the
>>> own APIC instance is available as the opaque parameter
>>
>>> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>>
>>> @@ -876,7 +870,7 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
>>>       }
>>>         index = (addr >> 4) & 0xff;
>>> -    apic_register_read(index, &val);
>>> +    apic_register_read(opaque, index, &val);
>>
>>I think it would be better to use local variable here:
>>
>> APICCommonState *s = opaque;
>>
>>and use it down the line.  Yes, there's just one usage, but it is
>>still clearer this way (in my opinion anyway).
>>
>>Ditto in apic_mem_write.
>
>I agree. Will fix in the next iteration.

I couldn't use the opaque parameter in v3 at all, so I needed an `APICCommonState *s` anyway.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>>
>>But it's more a nitpick really.
>>
>>Thanks,
>>
>>/mjt