Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 24 Jun 2021 15:44:34 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     "Tian\, Kevin" <kevin.tian@intel.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        "Dey\, Megha" <megha.dey@intel.com>,
        "Raj\, Ashok" <ashok.raj@intel.com>,
        "Pan\, Jacob jun" <jacob.jun.pan@intel.com>,
        "Jiang\, Dave" <dave.jiang@intel.com>,
        "Liu\, Yi L" <yi.l.liu@intel.com>,
        "Lu\, Baolu" <baolu.lu@intel.com>,
        "Williams\, Dan J" <dan.j.williams@intel.com>,
        "Luck\, Tony" <tony.luck@intel.com>,
        "Kumar\, Sanjay K" <sanjay.k.kumar@intel.com>,
        LKML <linux-kernel@vger.kernel.org>, KVM <kvm@vger.kernel.org>,
        Kirti Wankhede <kwankhede@nvidia.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Marc Zyngier <maz@kernel.org>,
        Bjorn Helgaas <helgaas@kernel.org>
Subject: Re: Virtualizing MSI-X on IMS via VFIO
Message-ID: <20210624154434.11809b8f.alex.williamson@redhat.com>
In-Reply-To: <8735t7wazk.ffs@nanos.tec.linutronix.de>
References: <MWHPR11MB1886E14C57689A253D9B40C08C079@MWHPR11MB1886.namprd11.prod.outlook.com>
        <8735t7wazk.ffs@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 24 Jun 2021 17:14:39 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> After studying the MSI-X specification again, I think there is another
> option to solve this for MSI-X, i.e. the dynamic sizing part:
> 
> MSI requires to disable MSI in order to update the number of enabled
> vectors in the control word.

Exactly what part of the spec requires this?  This is generally the
convention I expect too, and there are complications around contiguous
vectors and data field alignment, but I'm not actually able to find a
requirement in the spec that MSI Enable must be 0 when modifying other
writable fields or that writable fields are latched when MSI Enable is
set.

> MSI-X does not have that requirement as there is no 'number of used
> vectors' control field. MSI-X provides a fixed sized vector table and
> enabling MSI-X "activates" the full table.
> 
> System software has to set proper messages in the table and eventually
> associate the table entries to device (sub)functions if that's not
> hardwired in the device and controlled by queue enablement etc.
> 
> According to the specification there is no requirement for masked table
> entries to contain a valid message:
> 
>  "Mask Bit: ... When this bit is set, the function is prohibited from
>                 sending a message using this MSI-X Table entry."
> 
> which means that the function must reread the table entry when the mask
> bit in the vector control word is cleared.

What is a "valid" message as far as the device is concerned?  "Valid"
is meaningful to system software and hardware, the device doesn't care.

Like MSI above, I think the real question is when is the data latched
by the hardware.  For MSI-X this seems to be addressed in (PCIe 5.0
spec) 6.1.4.2 MSI-X Configuration:

  Software must not modify the Address, Data, or Steering Tag fields of
  an entry while it is unmasked.

Followed by 6.1.4.5 Per-vector Masking and Function Masking:

  For MSI-X, a Function is permitted to cache Address and Data values
  from unmasked MSI-X Table entries. However, anytime software unmasks
  a currently masked MSI-X Table entry either by Clearing its Mask bit
  or by Clearing the Function Mask bit, the Function must update any
  Address or Data values that it cached from that entry. If software
  changes the Address or Data value of an entry while the entry is
  unmasked, the result is undefined.

So caching/latching occurs on unmask for MSI-X, but I can't find
similar statements for MSI.  If you have, please note them.  It's
possible MSI is per interrupt.

Anyway, at least MSI-X if not also MSI could have a !NORESIZE
implementation, which is why this flag exists in vfio.  Thanks,

Alex