On 2011-09-02 14:25, Gleb Natapov wrote: > On Fri, Sep 02, 2011 at 01:36:57PM +0200, Jan Kiszka wrote: >> On 2011-09-02 13:27, Jan Kiszka wrote: >>> On 2011-09-02 09:48, Sasha Levin wrote: >>>> The RH bit exists in the message address register (lower 32 bits of >>>> the address). >>>> >>>> The bit indicates whether the message should go to the processor which was >>>> indicated in the destination ID bits, or whether it should go to the >>>> processor running at the lowest priority. >>>> >>>> Cc: Avi Kivity <[email protected]> >>>> Cc: Marcelo Tosatti <[email protected]> >>>> Signed-off-by: Sasha Levin <[email protected]> >>>> --- >>>> virt/kvm/irq_comm.c | 17 ++++++++++++++++- >>>> 1 files changed, 16 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c >>>> index 9f614b4..0ba3a3d 100644 >>>> --- a/virt/kvm/irq_comm.c >>>> +++ b/virt/kvm/irq_comm.c >>>> @@ -134,7 +134,22 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry >>>> *e, >>>> irq.level = 1; >>>> irq.shorthand = 0; >>>> >>>> - /* TODO Deal with RH bit of MSI message address */ >>>> + /* >>>> + * If the RH bit is set, we'll deliver to the processor running >>>> + * at the lowest priority. >>>> + */ >>>> + if (e->msi.address_lo & MSI_ADDR_REDIRECTION_LOWPRI) { >>>> + irq.delivery_mode = MSI_DATA_DELIVERY_LOWPRI; >>>> + } else { >>>> + /* >>>> + * If the RH bit is not set, we'll deliver to the specific >>>> + * processor mentioned in destination ID, and ignore the DM >>>> + * bit. >>>> + */ >>>> + irq.dest_mode = MSI_ADDR_DEST_MODE_PHYSICAL; >>>> + irq.delivery_mode = MSI_DATA_DELIVERY_FIXED; >>>> + } >>>> + >>>> return kvm_irq_delivery_to_apic(kvm, NULL, &irq); >>>> } >>>> >>> >>> Do you happen have a kvm unit test for this? Or how did you validate the >>> change? It doesn't look incorrect to me, I'd just like to check it QEMU >>> as well which apparently already has the logic above but also some >>> contradictory comment. >> >> Err, no, QEMU does not have this logic, it also ignores RH. >> >> But the above bits make "irq.delivery_mode = e->msi.data & 0x700" >> pointless. And that strongly suggests something is still wrong. >> > Yes, looks like delivery_mode assignment in the else clause is not > needed. This RH bit is strange. How is it different from setting > delivery mode to lowest priority in the data register? What practical > problem Sasha tries to fix here?
Logical addressing should not be available without RH==1. It was so far nevertheless because we evaluated DM even if RH was 0. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
