2015-04-23 12:34-0600, James Sullivan:
> On 04/23/2015 07:49 AM, Radim Krčmář wrote:
>> 2015-04-06 17:45-0600, James Sullivan:
>>> Currently, apic_get_arb_pri() is unimplemented and returns 0.
>>>
>>> Implemented apic_get_arb_pri() and added two helper functions
>>> apic_compare_prio() and apic_lowest_prio() to be used for LAPIC
>>> arbitration.
>>>
>>> Signed-off-by: James Sullivan <sullivan.jame...@gmail.com>
>>> ---
>>> + for (i = 0; i < MAX_APIC_WORDS; i++) {
>>> + if (deliver_bitmask[i]) {
>>> + d = i * 32 + apic_ffs_bit(deliver_bitmask[i]);
>>> + if (!lowest || apic_compare_prio(local_apics[d], lowest) < 0) {
>>> + lowest = local_apics[d];
>>
>> deliver_bitmask is 8 times u32 to express all 255 possible APICs.
>> apic_ffs_bit() takes the last set bit, so this loop incorrectly
>> considers only up to 8 candidates for lowest priority delivery.
>> foreach_apic() could be used when fixing it, which would also avoid a
>> 'local_apic[d] == NULL' crash.
>>
>
> Dumb mistake, I'll fix the former point. I shied away from
> foreach_apic() because I think it's a bit messy to embed a block or an
> `if` statement into the macro like so:
>
> foreach_apic(apic,bmask,
> if (cond)
> foo();
> );
>
> But if people are okay with that sort of thing we can switch to it.
Yeah, I wouldn't use it either :)
It could use the canonical form (and optimizations for sparse bitmasks)
foreach(...)
code;
Moving this logic to the loop in [4/4] would be an elegant solution.
> > (For consideration:
> > APM 2-16.2.2 Lowest Priority Messages and Arbitration
> > If there is a tie for lowest priority, the local APIC with the highest
> > APIC ID is selected.
> >
> > Intel is undefined in spec and picks the lowest APIC ID in practice.)
> >
>
> Pretty quick change there, set lowest = local_apics[d] when
> apic_compare_prio(local_apics[d], lowest) <= 0 rather than < 0
local_apics[] aren't indexed by APIC ID, which brings two complications
- QEMU allows writing to APIC ID.
Luckily, the spec allows us to make it read only, but even then
- local_apics[] might not be ordered like their APIC IDs; (I don't know)
and future development can change that, so we should also encode the
expecation somewhere. (Comments don't work very well.)
Taking a look at the real APIC ID would be safest.
(Silently ignoring it is also a viable option.)
