From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A40FD21AEB0B9 for ; Wed, 26 Jul 2017 08:28:38 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F290DFFC9C; Wed, 26 Jul 2017 15:30:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F290DFFC9C Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4A25E69294; Wed, 26 Jul 2017 15:30:33 +0000 (UTC) To: Jason Wang , Brijesh Singh , edk2-devel@lists.01.org Cc: Tom Lendacky , Jordan Justen , "Michael S . Tsirkin" , Gerd Hoffmann , Ard Biesheuvel References: <1500502151-13508-1-git-send-email-brijesh.singh@amd.com> <841bec5f-6f6e-8b1f-25ba-0fd37a915b72@redhat.com> <904dae9f-e515-01ba-e16f-6561616c78af@redhat.com> From: Laszlo Ersek Message-ID: <97cb29d6-e7c9-2633-df9f-962fc39b12bc@redhat.com> Date: Wed, 26 Jul 2017 17:30:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <904dae9f-e515-01ba-e16f-6561616c78af@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 26 Jul 2017 15:30:41 +0000 (UTC) Subject: Re: [RFC v1 0/3] Add VIRTIO_F_IOMMU_PLATFORM support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 15:28:38 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 07/26/17 05:32, Jason Wang wrote: > > > On 2017年07月26日 02:17, Laszlo Ersek wrote: >> Adding Ard >> >> On 07/20/17 00:09, Brijesh Singh wrote: >>> I have found that OVMF fails to detect the disk when iommu_platform >>> is set from >>> qemu cli. The failure occurs during the feature bit negotiation. >>> >>> Recently, EDKII introduced IOMMU protocol d1fddc4533bf. SEV patch >>> series introduced >>> a IoMmu protocol driver f9d129e68a45 to set a DMA access attribute >>> and methods to >>> allocate, free, map and unmap the DMA memory for the master bus devices >>> >>> In this patch series, I have tried to enable the IOMMU_PLATFORM >>> feature for >>> VirtioBlkDevice. I am sending this as RFC to seek feedback before I >>> extend the support >>> for other Virtio devices. The patch has been tested in SEV guest - >>> mainly because >>> IoMmuDxe driver installs the IOMMU protocol for SEV guest only. If >>> needed, I can >>> extend the IoMmuDxe driver to install IOMMU protocol for non SEV guests. >>> >>> qemu cli used for testing: >>> >>> # $QEMU \ >>> ... >>> -drive file=${HDA_FILE},if=none,id=disk0,format=qcow2 \ >>> -device >>> virtio-blk-pci,drive=disk0,disable-legacy=on,iommu_platform=true,disable-modern=off,scsi=off >>> >>> ... >>> >>> Cc: Jordan Justen >>> Cc: Laszlo Ersek >>> Cc: Jason Wang >>> Cc: Michael S. Tsirkin >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Brijesh Singh >>> >>> Brijesh Singh (3): >>> OvmfPkg/Include/Virtio10: Define VIRTIO_F_IOMMU_PLATFORM feature bit >>> OvmfPkg/VirtioLib: Add IOMMU_PLATFORM support >>> OvmfPkg/VirtioBlkDxe: Add VIRITO_F_IOMMU_PLATFORM support >> In the course of processing my post-PTO email backlog, yesterday I >> skimmed this topic very quickly, and thought about it for an hour or so >> (while away for the computer). Here's my take on it. >> >> >> (1) The closest to any formal language that I found for >> VIRTIO_F_IOMMU_PLATFORM are: >> >> https://issues.oasis-open.org/browse/VIRTIO-154 >> https://lists.oasis-open.org/archives/virtio-dev/201610/msg00121.html >> >> Are these references up-to-date? The ticket also references SVN r587 as >> the resolution, but that link is dead. >> >> >> (2) A few weeks back I saw Jason's SeaBIOS patch (also pointed out to me >> more recently by Brijesh, in private): >> >> [SeaBIOS] [PATCH] virtio: IOMMU support >> >> I don't understand this patch. (I also don't understand the >> "iommu_platform" device property on the QEMU command line.) According to >> the spec language quoted from the mailing list above, we have four cases: >> >> (2.1) device does not offer VIRITO_F_IOMMU_PLATFORM --> everything works >> like before >> >> (2.2) device offers VIRITO_F_IOMMU_PLATFORM, but the driver does not >> negotiate it --> device is allowed to reject functioning >> >> * device offers VIRITO_F_IOMMU_PLATFORM and the driver negotiates it: >> there are two possibilities: >> (2.3) the driver*disables* the IOMMU, and works like before >> (2.4) the driver*configures* the IOMMU and works accordingly >> >> The SeaBIOS patch negotiates VIRITO_F_IOMMU_PLATFORM, but it*neither* >> disables*nor* configures the IOMMU. It simply*ignores* the IOMMU. I >> don't see how that is any different*in effect* from simply not >> negotiating VIRITO_F_IOMMU_PLATFORM -- case (2.2) --, and I don't >> understand why QEMU tolerates "ignoring the IOMMU" differently from "not >> negotiating the flag". > > I think the difference is we don't want give the ability of bypassing > IOMMU at driver level. That's why we forbid it in the case of 2.2. > > For 2.3 IOMMU was disabled by e.g guest os not driver. Ah! That makes a lot of sense. I guess this is again motivated by the DPDK use case -- the guest kernel would decide about IOMMU setup, but the virtio driver (which is in guest userspace) is required to comply with the IOMMU requirement, even if the guest kernel does not actually program the IOMMU. I hope my above interpretation is correct, because the recommendations in my other (long) email match it 100%. Namely, in UEFI, IOMMU setup (or non-setup) is separated to various platform drivers, and the virtio device drivers should be abstracted from the actual IOMMU presence (this is the gist of my long email). Hence simply confirming VIRITO_F_IOMMU_PLATFORM is right for the device drivers. Thanks Laszlo