Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp896246pxj; Fri, 11 Jun 2021 14:40:50 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyJVNsHEF/3Ez32ggTcPSxHoXJR6S7I1q2yVmo0a34wIvQVjKbrJIwdhdySHSVzDyRxobhq X-Received: by 2002:a05:6402:2707:: with SMTP id y7mr5700567edd.0.1623447649819; Fri, 11 Jun 2021 14:40:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623447649; cv=none; d=google.com; s=arc-20160816; b=ODREzT1ns/x1r0ej+1l15I5H0gWV4kABo+Kgee46AsYh6ztSD6sMyt9ow3dSIxfT+H +W4Ukz5RTEHYdTFJWoKQ0Vq3jbVZcLv3Plh4kRirnjwoCBnYTPn+HeW5LB2QsyuolSOM XqEsK7iBc850DX1AXvOPrRfNUWDn0ABG6FlHbKFC1PqRHAPFZecNkexI+07q7Sl0TwQM Qp8D5Z2uV9zxD1uhTXD+pMuuJ0neIh8/gsFFPGtM12Qt1VshqiDEJcnza/9xV2bZjaOy cY3hY5aSyeiSHsd0Ud2uRklJa+ppaYfXFyocaqlqMzONQIgsi0t3gt3PYc2y1k5qbBNA 7RCg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :date:dkim-signature; bh=C6IliJr15GgETFgAUbqX8vnW2P0SKmHz1sKoUHwwIt0=; b=P8gavTsZLBEKgCxN7fD4FVwbao8vqBG/DcW+2ArYGZM8WGuM9Hp6/7/uhNHxSeO1w4 aJ6Vw/8+sR7iFMq+soO6yC079aHtQBLV9urdPNYyPsT2gHhBc4lzyTTE4goWjQ5a4dI7 mNwTtcRh5tH1juBPzHjW80cMQ138F02nr/xEVL9z1bqyItPgu+Gjdyu/9cG7XsJPCYO6 j9yvCiSHPoLCBqvAsNkn9CmUZeSM5gfqe594h+Zs60aRzwcNh2alvZ3ROvKUrKZg9wrv 41EUhJCO7M4OfqpO16eep2Sqbw137RYtwApraNbcNRg0HXQP97PfwnjjLqBr3DiOsY/9 KdxQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HiQfqxwr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id e14si6255020edm.431.2021.06.11.14.40.25; Fri, 11 Jun 2021 14:40:49 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HiQfqxwr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230028AbhFKVky (ORCPT + 99 others); Fri, 11 Jun 2021 17:40:54 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:27913 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229874AbhFKVkx (ORCPT ); Fri, 11 Jun 2021 17:40:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623447534; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C6IliJr15GgETFgAUbqX8vnW2P0SKmHz1sKoUHwwIt0=; b=HiQfqxwrRGWYHfW9TwblmbKPwM/aNCEsKpomsKuF7zaAes10uDSgn6PClvufIR0zjzTvia IZzbNMbcB31yyaZN3En2M71FPk1/f8KRl4kDREfpCZDIdxyL7YWmj+3Fzc8XL3g507/Zrs HzB9xedscD3gkB24i5ceZNVVv0CxUa0= Received: from mail-ot1-f70.google.com (mail-ot1-f70.google.com [209.85.210.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-501-BzDzm3_lOBG9-NcmGhrirQ-1; Fri, 11 Jun 2021 17:38:53 -0400 X-MC-Unique: BzDzm3_lOBG9-NcmGhrirQ-1 Received: by mail-ot1-f70.google.com with SMTP id b3-20020a0568303103b02903ed1990d4c1so2693137ots.16 for ; Fri, 11 Jun 2021 14:38:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:organization:mime-version:content-transfer-encoding; bh=C6IliJr15GgETFgAUbqX8vnW2P0SKmHz1sKoUHwwIt0=; b=Uagl0ezDzhfr89Z9PIxNdmJxiS3HdylaVR5MMOx5crU7XAIpOZXSZSuhtnuTX/ML4+ JpOYarWCNVQs7bP4lWn+W82W0dcGKLN1xQo+xyNKvkCdiGFQJB1ijuQrO5iVNfHYqy0S vOEhJPpCBTOTVKZyAToo7hNkrLspv8qLfVTLDXgvnHMvPmqHo5BY2ZeUDjWC9H9qvv7D 1YU3YAInUDK8JwLwj9oL3oSUcJ9W4NBcdaePxR5bBxpsYfgjgUDGWU/qv7FdnZkSAnt+ 6Mpm5FdEL52MHLgjw8oeXZ5Qq/bMRvXpwI2D0EzdhmoQMUgPACSgf+bXt06pIjVTmv8/ f5QA== X-Gm-Message-State: AOAM530KDhWPyEcllJ6rZ/sB/XNWkRxYLsF0Zwsnlx0WpEW1XVkaet0Z NWWFmv/S++NfgqSSLaRl5HZFEgLYKec26oa0tfENrNY8ybjpWL7nTyNP8Cf9InBIrJyR4DOo6Js FYkpR7py/qB77mHMjZCF/KcyZ X-Received: by 2002:a9d:6a09:: with SMTP id g9mr4632036otn.222.1623447532712; Fri, 11 Jun 2021 14:38:52 -0700 (PDT) X-Received: by 2002:a9d:6a09:: with SMTP id g9mr4632009otn.222.1623447532427; Fri, 11 Jun 2021 14:38:52 -0700 (PDT) Received: from redhat.com ([198.99.80.109]) by smtp.gmail.com with ESMTPSA id q22sm1467015otl.11.2021.06.11.14.38.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Jun 2021 14:38:52 -0700 (PDT) Date: Fri, 11 Jun 2021 15:38:50 -0600 From: Alex Williamson To: "Tian, Kevin" Cc: Jason Gunthorpe , Joerg Roedel , Jean-Philippe Brucker , David Gibson , "Jason Wang" , "parav@mellanox.com" , "Enrico Weigelt, metux IT consult" , Paolo Bonzini , Shenming Lu , Eric Auger , Jonathan Corbet , "Raj, Ashok" , "Liu, Yi L" , "Wu, Hao" , "Jiang, Dave" , Jacob Pan , Kirti Wankhede , "Robin Murphy" , "kvm@vger.kernel.org" , "iommu@lists.linux-foundation.org" , "David Woodhouse" , LKML , "Lu Baolu" Subject: Re: Plan for /dev/ioasid RFC v2 Message-ID: <20210611153850.7c402f0b.alex.williamson@redhat.com> In-Reply-To: References: <20210609123919.GA1002214@nvidia.com> <20210609150009.GE1002214@nvidia.com> <20210609101532.452851eb.alex.williamson@redhat.com> <20210609102722.5abf62e1.alex.williamson@redhat.com> <20210609184940.GH1002214@nvidia.com> <20210610093842.6b9a4e5b.alex.williamson@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 11 Jun 2021 00:58:35 +0000 "Tian, Kevin" wrote: > Hi, Alex, > > > From: Alex Williamson > > Sent: Thursday, June 10, 2021 11:39 PM > > > > On Wed, 9 Jun 2021 15:49:40 -0300 > > Jason Gunthorpe wrote: > > > > > On Wed, Jun 09, 2021 at 10:27:22AM -0600, Alex Williamson wrote: > > > > > > > > > It is a kernel decision, because a fundamental task of the kernel is to > > > > > > ensure isolation between user-space tasks as good as it can. And if a > > > > > > device assigned to one task can interfer with a device of another task > > > > > > (e.g. by sending P2P messages), then the promise of isolation is > > broken. > > > > > > > > > > AIUI, the IOASID model will still enforce IOMMU groups, but it's not an > > > > > explicit part of the interface like it is for vfio. For example the > > > > > IOASID model allows attaching individual devices such that we have > > > > > granularity to create per device IOASIDs, but all devices within an > > > > > IOMMU group are required to be attached to an IOASID before they can > > be > > > > > used. > > > > > > Yes, thanks Alex > > > > > > > > It's not entirely clear to me yet how that last bit gets > > > > > implemented though, ie. what barrier is in place to prevent device > > > > > usage prior to reaching this viable state. > > > > > > The major security checkpoint for the group is on the VFIO side. We > > > must require the group before userspace can be allowed access to any > > > device registers. Obtaining the device_fd from the group_fd does this > > > today as the group_fd is the security proof. > > > > > > Actually, thinking about this some more.. If the only way to get a > > > working device_fd in the first place is to get it from the group_fd > > > and thus pass a group-based security check, why do we need to do > > > anything at the ioasid level? > > > > > > The security concept of isolation was satisfied as soon as userspace > > > opened the group_fd. What do more checks in the kernel accomplish? > > > > Opening the group is not the extent of the security check currently > > required, the group must be added to a container and an IOMMU model > > configured for the container *before* the user can get a devicefd. > > Each devicefd creates a reference to this security context, therefore > > access to a device does not exist without such a context. > > IIUC each device has a default domain when it's probed by iommu driver > at boot time. This domain includes an empty page table, implying that > device is already in a security context before it's probed by device driver. The default domain could be passthrough though, right? > Now when this device is added to vfio, vfio creates another security > context through above sequence. This sequence requires the device to > switch from default security context to this new one, before it can be > accessed by user. This is true currently, we use group semantics with the type1 IOMMU backend to attach all devices in the group to a secure context, regardless of the default domain. > Then I wonder whether it's really necessary. As long as a device is in > a security context at any time, access to a device can be allowed. The > user itself should ensure that the access happens only after the device > creates a reference to the new security context that is desired by this > user. > > Then what does group really bring to us? By definition an IOMMU group is the smallest set of devices that we can consider isolated from all other devices. Therefore devices in a group are not necessarily isolated from each other. Therefore if any device within a group is not isolated, the group is not isolated. VFIO needs to know when it's safe to provide userspace access to the device, but the device isolation is dependent on the group isolation. The group is therefore part of this picture whether implicit or explicit. > With this new proposal we just need to make sure that a device cannot > be attached to any IOASID before all devices in its group are bound to > the IOASIDfd. If we want to start with a vfio-like policy, then all devices > in the group must be attached to the same IOASID. Or as Jason suggests, > they can attach to different IOASIDs (if in the group due to !ACS) if the > user wants, or have some devices attached while others detached since > both are in a security context anyway. But if it's the device attachment to the IOASID that provides the isolation and the user might attach a device to multiple IOASIDs within the same IOASIDfd, and presumably make changes to the mapping of device to IOASID dynamically, are we interrupting user access around each of those changes? How would vfio be able to track this, and not only track it per device, but for all devices in the group. Suggesting a user needs to explicitly attach every device in the group is also a semantic change versus existing vfio, where other devices in the group must only be considered to be in a safe state for the group to be usable. The default domain may indeed be a solution to the problem, but we need to enforce a secure default domain for all devices in the group. To me that suggests that binding the *group* to an IOASIDfd is the point at which device access becomes secure. VFIO should be able to consider that the IOASIDfd binding has taken over ownership of the DMA context for the device and it will always be either an empty, isolated, default domain or a user defined IOASID. Maybe the model relative to vfio is something like: 1. bind a group to an IOASIDfd VFIO_GROUP_BIND_IOASID_FD(groupfd, ioasidfd) 2. create an IOASID label for each device VFIO_DEVICE_SET_IOASID_LABEL(devicefd, device_ioasid_label) VFIO can open access to the device after step 1, the IOASIDfd takes responsibility for the device IOMMU context. After step 2, shouldn't the user switch to the IOASID uAPI? I don't see why vfio would be involved in attaching devices to specific IOASID contexts within the IOASIDfd at that point, we might need internal compatibility interfaces, but a native IOASID user should have all they need to attach device labels to IOASIDs using the IOASIDfd at this point. We'll need to figure out what the release model looks like too. A group should hold a reference on the IOASIDfd and each open device should hold a reference on the group so that the isolation of the group cannot be broken while any device is open. > > This proposal has of course put the device before the group, which then > > makes it more difficult for vfio to retroactively enforce security. > > > > > Yes, we have the issue where some groups require all devices to use > > > the same IOASID, but once someone has the group_fd that is no longer a > > > security issue. We can fail VFIO_DEVICE_ATTACH_IOASID callss that > > > don't make sense. > > > > The groupfd only proves the user has an ownership claim to the devices, > > it does not itself prove that the devices are in an isolated context. > > Device access is not granted until that isolated context is configured. > > > > vfio owns the device, so it would make sense for vfio to enforce the > > security of device access only in a secure context, but how do we know > > a device is in a secure context? > > > > Is it sufficient to track the vfio device ioctls for attach/detach for > > an IOASID or will the user be able to manipulate IOASID configuration > > for a device directly via the IOASIDfd? > > > > What happens on detach? As we've discussed elsewhere in this thread, > > revoking access is more difficult than holding a reference to the > > secure context, but I'm under the impression that moving a device > > between IOASIDs could be standard practice in this new model. A device > > that's detached from a secure context, even temporarily, is a problem. > > Access to other devices in the same group as a device detached from a > > secure context is a problem. > > as long as the device is switched back to the default security context > after detach then it should be fine. So long as the default context is secure, and ideally if IOMMU context switches are atomic. > > > > > > > Groups should be primarily about isolation security, not about > > IOASID > > > > > > > matching. > > > > > > > > > > > > That doesn't make any sense, what do you mean by 'IOASID matching'? > > > > > > > > > > One of the problems with the vfio interface use of groups is that we > > > > > conflate the IOMMU group for both isolation and granularity. I think > > > > > what Jason is referring to here is that we still want groups to be the > > > > > basis of isolation, but we don't want a uAPI that presumes all devices > > > > > within the group must use the same IOASID. > > > > > > Yes, thanks again Alex > > > > > > > > For example, if a user owns an IOMMU group consisting of > > > > > non-isolated functions of a multi-function device, they should be > > > > > able to create a vIOMMU VM where each of those functions has its > > > > > own address space. That can't be done today, the entire group > > > > > would need to be attached to the VM under a PCIe-to-PCI bridge to > > > > > reflect the address space limitation imposed by the vfio group > > > > > uAPI model. Thanks, > > > > > > > > Hmm, likely discussed previously in these threads, but I can't come up > > > > with the argument that prevents us from making the BIND interface > > > > at the group level but the ATTACH interface at the device level? For > > > > example: > > > > > > > > - VFIO_GROUP_BIND_IOASID_FD > > > > - VFIO_DEVICE_ATTACH_IOASID > > > > > > > > AFAICT that makes the group ownership more explicit but still allows > > > > the device level IOASID granularity. Logically this is just an > > > > internal iommu_group_for_each_dev() in the BIND ioctl. Thanks, > > > > > > At a high level it sounds OK. > > > > > > However I think your above question needs to be answered - what do we > > > want to enforce on the iommu_fd and why? > > > > > > Also, this creates a problem with the device label idea, we still > > > need to associate each device_fd with a label, so your above sequence > > > is probably: > > > > > > VFIO_GROUP_BIND_IOASID_FD(group fd) > > > VFIO_BIND_IOASID_FD(device fd 1, device_label) > > > VFIO_BIND_IOASID_FD(device fd 2, device_label) > > > VFIO_DEVICE_ATTACH_IOASID(..) > > > > > > And then I think we are back to where I had started, we can trigger > > > whatever VFIO_GROUP_BIND_IOASID_FD does automatically as soon as all > > > of the devices in the group have been bound. > > > > How to label a device seems like a relatively mundane issue relative to > > ownership and isolated contexts of groups and devices. The label is > > essentially just creating an identifier to device mapping, where the > > identifier (label) will be used in the IOASID interface, right? As I > > Three usages in v2: > > 1) when reporting per-device capability/format info to user; > 2) when handling device-wide iotlb invalidation from user; > 3) when reporting device-specific fault data to user; As above, it seems more complete to me to move attach/detach of devices to IOASIDs using the labels as well. Thanks, Alex