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 <avi@xxxxxxxxxx>
>>>> Cc: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
>>>> Signed-off-by: Sasha Levin <levinsasha928@xxxxxxxxx>
>>>> ---
>>>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
