From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex Williamson <alex.williamson@redhat.com>
Subject: Re: [PATCH 2/2] Device assignment: Fix MSI IRQ affinity setting
Date: Thu, 24 May 2012 16:11:46 -0600
Message-ID: <1337897506.4714.55.camel@ul30vt>
References: <1337878924-39069-1-git-send-email-richard@nod.at>
	  <1337878924-39069-2-git-send-email-richard@nod.at>
	 <1337883627.4714.32.camel@ul30vt>
	 <alpine.LFD.2.02.1205242323230.3231@ionos> <4FBEADCB.2010900@siemens.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Richard Weinberger <richard@nod.at>, kvm@vger.kernel.org,
	avi@redhat.com, Marcelo Tosatti <mtosatti@redhat.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:15726 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752476Ab2EXWMC (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 24 May 2012 18:12:02 -0400
In-Reply-To: <4FBEADCB.2010900@siemens.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, 2012-05-24 at 18:53 -0300, Jan Kiszka wrote:
> On 2012-05-24 18:39, Thomas Gleixner wrote:
> > On Thu, 24 May 2012, Alex Williamson wrote:
> >> On Thu, 2012-05-24 at 18:02 +0100, Richard Weinberger wrote:
> >>> +            if (address == msi_start + PCI_MSI_DATA_32)
> >>> +                handle_cfg_write_msi(pci_dev, assigned_dev);
> >>
> >> Why didn't we just use range_covers_byte(address, len, pci_dev->msi_cap
> >> + PCI_MSI_DATA_32) to start with?  But how does this handle the enable
> >> bit?
> > 
> > The problem with the current implementation is that it only changes
> > the routing if the msi entry goes from masked to unmasked state.
> > 
> > Linux does not mask the entries on affinity changes and never did,
> > neither for MSI nor for MSI-X.
> > 
> > I know it's probably not according to the spec, but we can't fix that
> > retroactively.
> 
> For MSI, this is allowed. For MSI-X, this would clearly be a Linux bug,
> waiting for hardware to dislike this spec violation.
> 
> However, if this is the current behavior of such a prominent guest, I
> guess we have to stop optimizing the QEMU MSI-X code that it only
> updates routings on mask changes. Possibly other OSes get this wrong too...
> 
> Thanks, for the clarification. Should go into the changelog.

Hmm, if Linux didn't mask MSIX before updating vectors it'd not only be
a spec violation, but my testing of the recent changes to fix MSIX
vector updates for exactly this would have failed...

        } else if (msix_masked(&orig) && !msix_masked(entry)) {
            ... update vector...

So I'm not entirely sure I believe that.  Thanks,

Alex