From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq Date: Wed, 18 Mar 2015 22:09:32 -0300 Message-ID: <20150319010932.GA18338@amt.cnet> References: <5502FEDB.3030606@gmail.com> <20150318225225.GA8702@amt.cnet> <550A1F6A.6030602@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, gleb@kernel.org, pbonzini@redhat.com, Radim =?utf-8?B?S3LEjW3DocWZ?= To: James Sullivan Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43734 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbbCSBJu (ORCPT ); Wed, 18 Mar 2015 21:09:50 -0400 Content-Disposition: inline In-Reply-To: <550A1F6A.6030602@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Mar 18, 2015 at 06:59:22PM -0600, James Sullivan wrote: > > > > The documentation states the following: > > > > * When RH is 0, the interrupt is directed to the processor listed in the > > Destination ID field. > > > > * If RH is 0, then the DM bit is ignored and the message is sent ahead > > independent of whether the physical or logical destination mode is used. > > > > However, from the POV of a device writing to memory to generate an MSI > > interrupt, there is no (or i can't see any) other information that > > can be used to infer logical or physical mode for the interrupt message. > > > > Before your patch: > > > > (dm, rh) = (0, 0) => irq->dest_mode = 0 > > (dm, rh) = (0, 1) => irq->dest_mode = 0 > > (dm, rh) = (1, 0) => irq->dest_mode = 1 > > (dm, rh) = (1, 1) => irq->dest_mode = 1 > > > > After your patch: > > > > (dm, rh) = (0, 0) => irq->dest_mode = 0 > > (dm, rh) = (0, 1) => irq->dest_mode = 0 > > (dm, rh) = (1, 0) => irq->dest_mode = 0 > > (dm, rh) = (1, 1) => irq->dest_mode = 1 > > > > > > Am i missing some explicit documentation that refers > > to (dm, rh) = (1, 0) => irq->dest_mode = 0 ? > > >From the IA32 manual (Vol. 3, 10.11.2): > > * When RH is 0, the interrupt is directed to the processor listed > in the Destination ID field. > * When RH is 1 and the physical destination mode is used, the Destination > ID field must not be set to FFH; it must point to a processor that is > present and enabled to receive the interrupt. > * When RH is 1 and the logical destination mode is active in a system using > a flat addressing model, the Destination ID field must be set so that bits > set to 1 identify processors that are present and enabled to receive the > interrupt. > * If RH is set to 1 and the logical destination mode is active in a system > using cluster addressing model, then Destination ID field must not be > set to FFH; the processors identified with this field must be present > and enabled to receive the interrupt. > > My interpretation of this is that RH=0 indicates that the Dest. ID field > contains an APIC ID, and as such destination mode is physical. When RH=1, > depending on the value of DM, we either use physical or logical dest mode. > The result of this is that logical dest mode is set just when RH=1/DM=1, > as far as I understand. > > > > > See native_compose_msi_msg: > > > > msg->address_lo = > > MSI_ADDR_BASE_LO | > > ((apic->irq_dest_mode == 0) ? > > MSI_ADDR_DEST_MODE_PHYSICAL : > > MSI_ADDR_DEST_MODE_LOGICAL) | > > ((apic->irq_delivery_mode != dest_LowestPrio) ? > > MSI_ADDR_REDIRECTION_CPU : > > MSI_ADDR_REDIRECTION_LOWPRI) | > > MSI_ADDR_DEST_ID(dest); > > > > > > So it does configure DM = MSI_ADDR_DEST_MODE_LOGICAL > > and RH = MSI_ADDR_REDIRECTION_LOWPRI. > > > > ...and yet this is a good counterexample against my argument :) > > What I think I'll do is revert this particular change so that dest_mode is > set independently of RH. While I'm not entirely convinced that this is the > intended interpretation, I do think that consistency with the existing logic > is probably desirable for the time being. If I can get closure on the matter > I'll re-submit that change, but for the time being I will undo it. > > -James Just write MSI-X table entries on real hardware (say: modify native_compose_msi_msg or MSI-X equivalent), with all RH/DM combinations, and see what behaviour is comes up?