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;
IronPort-SDR: 3uHv/mNBS2PuxZxmApi10iGniE4BDbo7a1FVY6YBZxfcuULMNbvrESa4GCrBdBmTmCrSKD8lnI
 LSbwpX6W44QQ==
IronPort-SDR: 49PSRAP6HZ2kkUOmviJXbjwvhozsbej9LWy1KbXoOSWlD9Itl37uJJ9lcmtRvzpTqNciu0IzD/
 O+hF9jv9BXmA==
Subject: Re: [PATCH RFC 00/15] Add VFIO mediated device support and IMS
 support for the idxd driver.
To:     Jason Gunthorpe <jgg@mellanox.com>,
        Dave Jiang <dave.jiang@intel.com>
Cc:     vkoul@kernel.org, maz@kernel.org, bhelgaas@google.com,
        rafael@kernel.org, gregkh@linuxfoundation.org, tglx@linutronix.de,
        hpa@zytor.com, alex.williamson@redhat.com, jacob.jun.pan@intel.com,
        ashok.raj@intel.com, yi.l.liu@intel.com, baolu.lu@intel.com,
        kevin.tian@intel.com, sanjay.k.kumar@intel.com,
        tony.luck@intel.com, jing.lin@intel.com, dan.j.williams@intel.com,
        kwankhede@nvidia.com, eric.auger@redhat.com, parav@mellanox.com,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        x86@kernel.org, linux-pci@vger.kernel.org, kvm@vger.kernel.org
References: <158751095889.36773.6009825070990637468.stgit@djiang5-desk3.ch.intel.com>
 <20200421235442.GO11945@mellanox.com>
From:   "Dey, Megha" <megha.dey@linux.intel.com>
Message-ID: <d6b3c133-ac19-21af-b7a7-b9e7166b8166@linux.intel.com>
Date:   Wed, 22 Apr 2020 16:04:20 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20200421235442.GO11945@mellanox.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 4/21/2020 4:54 PM, Jason Gunthorpe wrote:
> On Tue, Apr 21, 2020 at 04:33:46PM -0700, Dave Jiang wrote:
>> The actual code is independent of the stage 2 driver code submission that adds
>> support for SVM, ENQCMD(S), PASID, and shared workqueues. This code series will
>> support dedicated workqueue on a guest with no vIOMMU.
>>    
>> A new device type "mdev" is introduced for the idxd driver. This allows the wq
>> to be dedicated to the usage of a VFIO mediated device (mdev). Once the work
>> queue (wq) is enabled, an uuid generated by the user can be added to the wq
>> through the uuid sysfs attribute for the wq.  After the association, a mdev can
>> be created using this UUID. The mdev driver code will associate the uuid and
>> setup the mdev on the driver side. When the create operation is successful, the
>> uuid can be passed to qemu. When the guest boots up, it should discover a DSA
>> device when doing PCI discovery.
> 
> I'm feeling really skeptical that adding all this PCI config space and
> MMIO BAR emulation to the kernel just to cram this into a VFIO
> interface is a good idea, that kind of stuff is much safer in
> userspace.
> 
> Particularly since vfio is not really needed once a driver is using
> the PASID stuff. We already have general code for drivers to use to
> attach a PASID to a mm_struct - and using vfio while disabling all the
> DMA/iommu config really seems like an abuse.
> 
> A /dev/idxd char dev that mmaps a bar page and links it to a PASID
> seems a lot simpler and saner kernel wise.
> 
>> The mdev utilizes Interrupt Message Store or IMS[3] instead of MSIX for
>> interrupts for the guest. This preserves MSIX for host usages and also allows a
>> significantly larger number of interrupt vectors for guest usage.
> 
> I never did get a reply to my earlier remarks on the IMS patches.
> 
> The concept of a device specific addr/data table format for MSI is not
> Intel specific. This should be general code. We have a device that can
> use this kind of kernel capability today.
> 

<resending to the mailing list, I had incorrect email options set>

Hi Jason,

I am sorry if I did not address your comments earlier.

The present IMS code is quite generic, most of the code is in the 
drivers/ folder. We basically introduce 2 APIS: allocate and free IMS 
interrupts and a IMS IRQ domain to allocate these interrupts from. These 
APIs are architecture agnostic.

We also introduce a new IMS IRQ domain which is architecture specific. 
This is because IMS generates interrupts only in the remappable format, 
hence interrupt remapping should be enabled for IMS. Currently, the 
interrupt remapping code is only available for Intel and AMD and I don’t 
see anything for ARM.

If a new architecture would want to use IMS, they must simply introduce 
a new IMS IRQ domain. I am not sure if there is any other way around 
this. If you have any ideas, please let me know.

Also, could you give more details on the device that could use IMS? Do 
you have some driver code already? We could then see if and how the 
current IMS code could be made more generic.

> Jason
>