From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v2] KVM: Implement support for the RH bit Date: Fri, 02 Sep 2011 15:00:05 +0200 Message-ID: <4E60D355.50304@siemens.com> References: <1314949721-32761-1-git-send-email-levinsasha928@gmail.com> <4E60BDBA.5060806@siemens.com> <4E60BFD9.6090507@siemens.com> <20110902122541.GF26451@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Sasha Levin , "kvm@vger.kernel.org" , Avi Kivity , Marcelo Tosatti To: Gleb Natapov Return-path: Received: from thoth.sbs.de ([192.35.17.2]:24594 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751726Ab1IBNAS (ORCPT ); Fri, 2 Sep 2011 09:00:18 -0400 In-Reply-To: <20110902122541.GF26451@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 >>>> Cc: Marcelo Tosatti >>>> Signed-off-by: Sasha Levin >>>> --- >>>> 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