From patchwork Fri Sep 2 12:11:23 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 1121752 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p82CBofe014683 for ; Fri, 2 Sep 2011 12:11:50 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751627Ab1IBMLc (ORCPT ); Fri, 2 Sep 2011 08:11:32 -0400 Received: from david.siemens.de ([192.35.17.14]:24671 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751564Ab1IBML2 (ORCPT ); Fri, 2 Sep 2011 08:11:28 -0400 Received: from mail1.siemens.de (localhost [127.0.0.1]) by david.siemens.de (8.13.6/8.13.6) with ESMTP id p82CBOZ6008972; Fri, 2 Sep 2011 14:11:24 +0200 Received: from mchn199C.mchp.siemens.de ([139.25.109.49]) by mail1.siemens.de (8.13.6/8.13.6) with ESMTP id p82CBNIo009474; Fri, 2 Sep 2011 14:11:24 +0200 Message-ID: <4E60C7EB.9060401@siemens.com> Date: Fri, 02 Sep 2011 14:11:23 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Sasha Levin CC: kvm@vger.kernel.org, Avi Kivity , Marcelo Tosatti , Gleb Natapov Subject: Re: [PATCH v2] KVM: Implement support for the RH bit References: <1314949721-32761-1-git-send-email-levinsasha928@gmail.com> <4E60BDBA.5060806@siemens.com> <4E60BFD9.6090507@siemens.com> In-Reply-To: <4E60BFD9.6090507@siemens.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Fri, 02 Sep 2011 12:11:50 +0000 (UTC) On 2011-09-02 13:36, 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. I tend to believe that this is what the spec tries to tell us: ie. the DM flag is only relevant if RH is set, and RH==0 is equivalent to RH==1 && DH==0. BTW, irq_comm.c is surely the wrong place for all this IA32-specific interpretation of MSI address and data. And we have yet another guest-triggerable printk in kvm_irq_delivery_to_apic (messages to physical ID 0xff). Jan diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c index 9f614b4..b72f77a 100644 --- a/virt/kvm/irq_comm.c +++ b/virt/kvm/irq_comm.c @@ -128,7 +128,8 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT; irq.vector = (e->msi.data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT; - irq.dest_mode = (1 << MSI_ADDR_DEST_MODE_SHIFT) & e->msi.address_lo; + irq.dest_mode = ((e->msi.address_lo & MSI_ADDR_DEST_MODE_LOGICAL) && + (e->msi.address_lo & MSI_ADDR_REDIRECTION_LOWPRI)); irq.trig_mode = (1 << MSI_DATA_TRIGGER_SHIFT) & e->msi.data; irq.delivery_mode = e->msi.data & 0x700; irq.level = 1;