2015-03-10 16:39-0600, James Sullivan:
> On 03/10/2015 08:47 AM, Radim Krčmář wrote:
> >> + irq->dest_mode = phys ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL);
> >
> > (Should be APIC_DEST_LOGICAL. All works because it is a boolean and we
> > only checked for APIC_DEST_PHYSICAL, which is 0.)
> >
>
> Thank you, that should be as follows:
> irq->dest_mode = phys ? (APIC_DEST_PHYSICAL) : (APIC_DEST_LOGICAL);
> ?
Yes, thanks. (No need for parentheses around macros, we "agreed" to
cover that in definition and they don't make this more readable.)
> >> - /* TODO Deal with RH bit of MSI message address */
> >
> > RH bit still isn't deal with -- we do not use lowest-priority-like
> > delivery in logical destination mode ...
>
> Just want to make sure I understand this comment-
> Isn't low-pri delivery mode used in kvm_irq_delivery_to_apic_fast
> when irq->dest_mode > APIC_DEST_PHYSICAL (ie, logical)? (See below.)
Not necessarily -- we have 'dest_mode' and 'delivery_mode';
- dest_mode is either logical or physical
- delivery_mode is fixed/lowest-priority/NMI/...
Logical destination can be used with multiple delivery modes and
lowest-priority is just one of them:
> bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
| [...]
> if (irq->dest_mode == APIC_DEST_PHYSICAL) {
| [...]
> } else {
| [...]
> if (irq->delivery_mode == APIC_DM_LOWEST) {
Without lowest priority, it delivers to all matching APICs.
> Do you mean that this patch will interfere with this? As long as
> irq->dest_mode is still APIC_DEST_LOGICAL this shouldn't change.
Your patch improves the situation, but removing the TODO is not
warranted -- RH still doesn't do what it should:
> > How does DM=1/RH=1 work on real hardware?
> > (There seem to be interesting corner cases with irq->delivery_mode like
> > APIC_DM_NMI.)
>
> The IA32 manual says that if DM=1/RH=1, we operate in logical destination mode
> similarly to other IPIs. I don't believe this patch introduces any invalid
> settings listed in section 10-21, Vol. 3, so this shouldn't create any weirdness.
Quoting SDM Jan 2015, 10.11.1 Message Address Register Format:
This bit [RH] indicates whether the message should be directed to the
processor with the lowest interrupt priority among processors that can
receive the interrupt.
=> it should behave like lowest priority delivery.
Deliver to just one APIC -- we don't do that.
KVM interrupt delivery functions can only specify lowest priority though
delivery_mode and we would have to rework KVM if MSI can set something
else in the delivery_mode (like NMI).
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
