From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 62C07C77B7D for ; Tue, 18 Apr 2023 16:44:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B51D610E296; Tue, 18 Apr 2023 16:44:33 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by gabe.freedesktop.org (Postfix) with ESMTPS id 97C5C10E296 for ; Tue, 18 Apr 2023 16:44:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1681836269; 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=nHWVPIc4x5U/mlAf83FqLeQfAoumlxuLoM0oc93GYKc=; b=cPGZN6dMgQU4gX4a7sfGzWiK3jBcl2CzRNVw04IUGksyKDwjl+tfbocpdIHG5811C3pwn7 Etqrwhxo9LU7exETdVkgStOPej53e0ZFvfwxXtuowm9w6A0ELMtvOwAXBzd6iLrWIW8TET cNszF1pFOGOyhOvTk144mZgiH5SCeRw= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-34-O5t0_WHBP3qn0_MIxKMNMA-1; Tue, 18 Apr 2023 12:44:18 -0400 X-MC-Unique: O5t0_WHBP3qn0_MIxKMNMA-1 Received: by mail-io1-f72.google.com with SMTP id ca18e2360f4ac-760da06b86aso164430039f.0 for ; Tue, 18 Apr 2023 09:44:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681836257; x=1684428257; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nHWVPIc4x5U/mlAf83FqLeQfAoumlxuLoM0oc93GYKc=; b=K38er0zxZgGwSajzi/a2MwXjMCNo2ZtiTM5v4DKcbUvEkjtsa6/1xqv7SLCMsHkkM6 EpipKpzAA0I6Udz5/1nPtM9e7CH5r2FtmuTG+zkaRbVaQxzwK1EQbtoIM0G9CQv645X7 3MehkSebEpXn1XpkTkmk6ql4Kv6lyDkb8ipveRXLzqe9a7eU5AqX7m4YR3/dzIwDQ5/N DZ+31YlGOmw4SVc56yPMZy6aRQZ6tSjsH+UE3E9dvdupeuf0PpxFXXsdWHAbNkEtR4ne wAjV1Al76s2tgPmnaq4k8MXjXX3dH/pTN2ln249i8FJwSAqQkHE107N86L8xu1+RVtIq eB9g== X-Gm-Message-State: AAQBX9eeFwUIii2aO0ZQCiWi/QjYla02X8Tm+4JgqK0lVyZqGo5r+Smv WyXSzwlhigUA3afkhdppa67FfkGWKMNz7EmN3WIvxjo2YVpT4PbhD6ZZyu6Fk0L/szXT88xdWYX ldHsr2jpdSR8b9P8HLMslgnfqnK3x X-Received: by 2002:a05:6602:228e:b0:763:5ab7:a8ec with SMTP id d14-20020a056602228e00b007635ab7a8ecmr2330258iod.12.1681836257615; Tue, 18 Apr 2023 09:44:17 -0700 (PDT) X-Google-Smtp-Source: AKy350YitzyKqrsLa63dC12LmXhePfvCEuOzHSgyVKtNmIoubcbobEdIKF/6U2yIwGxHSAEsU59enQ== X-Received: by 2002:a05:6602:228e:b0:763:5ab7:a8ec with SMTP id d14-20020a056602228e00b007635ab7a8ecmr2330244iod.12.1681836257256; Tue, 18 Apr 2023 09:44:17 -0700 (PDT) Received: from redhat.com ([38.15.36.239]) by smtp.gmail.com with ESMTPSA id ch5-20020a0566383e8500b0040bd1d947bfsm4241002jab.158.2023.04.18.09.44.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Apr 2023 09:44:16 -0700 (PDT) Date: Tue, 18 Apr 2023 10:44:15 -0600 From: Alex Williamson To: "Tian, Kevin" Message-ID: <20230418104415.5cdecb5e.alex.williamson@redhat.com> In-Reply-To: References: <20230412105045.79adc83d.alex.williamson@redhat.com> <20230413120712.3b9bf42d.alex.williamson@redhat.com> <20230414111043.40c15dde.alex.williamson@redhat.com> <20230417130140.1b68082e.alex.williamson@redhat.com> <20230417140642.650fc165.alex.williamson@redhat.com> <20230417221033.778c00c9.alex.williamson@redhat.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.35; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Intel-gfx] [PATCH v3 12/12] vfio/pci: Report dev_id in VFIO_DEVICE_GET_PCI_HOT_RESET_INFO X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "mjrosato@linux.ibm.com" , "jasowang@redhat.com" , "Hao, Xudong" , "Duan, Zhenzhong" , "peterx@redhat.com" , "Xu, Terrence" , "chao.p.peng@linux.intel.com" , "linux-s390@vger.kernel.org" , "Liu, Yi L" , "kvm@vger.kernel.org" , "lulu@redhat.com" , "Jiang, Yanting" , "joro@8bytes.org" , "nicolinc@nvidia.com" , Jason Gunthorpe , "Zhao, Yan Y" , "intel-gfx@lists.freedesktop.org" , "eric.auger@redhat.com" , "intel-gvt-dev@lists.freedesktop.org" , "yi.y.sun@linux.intel.com" , "cohuck@redhat.com" , "shameerali.kolothum.thodi@huawei.com" , "suravee.suthikulpanit@amd.com" , "robin.murphy@arm.com" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 18 Apr 2023 05:02:44 +0000 "Tian, Kevin" wrote: > > From: Alex Williamson > > Sent: Tuesday, April 18, 2023 12:11 PM > > > > On Tue, 18 Apr 2023 03:24:46 +0000 > > "Tian, Kevin" wrote: > > > > > > From: Alex Williamson > > > > Sent: Tuesday, April 18, 2023 4:07 AM > > > > > > > > On Mon, 17 Apr 2023 16:31:56 -0300 > > > > Jason Gunthorpe wrote: > > > > > > > > > On Mon, Apr 17, 2023 at 01:01:40PM -0600, Alex Williamson wrote: > > > > > > Yes, it's not trivial, but Jason is now proposing that we consider > > > > > > mixing groups, cdevs, and multiple iommufd_ctxs as invalid. I think > > > > > > this means that regardless of which device calls INFO, there's only one > > > > > > answer (assuming same set of devices opened, all cdev, all within > > same > > > > > > iommufd_ctx). Based on what I explained about my understanding of > > > > INFO2 > > > > > > and Jason agreed to, I think the output would be: > > > > > > > > > > > > flags: NOT_RESETABLE | DEV_ID > > > > > > { > > > > > > { valid devA-id, devA-BDF }, > > > > > > { valid devC-id, devC-BDF }, > > > > > > { valid devD-id, devD-BDF }, > > > > > > { invalid dev-id, devE-BDF }, > > > > > > } > > > > > > > > > > > > Here devB gets dropped because the kernel understands that devB is > > > > > > unopened, affected, and owned. It's therefore not a blocker for > > > > > > hot-reset. > > > > > > > > > > I don't think we want to drop anything because it makes the API > > > > > ill suited for the debugging purpose. > > > > > > > > > > devb should be returned with an invalid dev_id if I understand your > > > > > example. Maybe it should return with -1 as the dev_id instead of 0, to > > > > > make the debugging a bit better. > > > > > > > > > > Userspace should look at only NOT_RESETTABLE to determine if it > > > > > proceeds or not, and it should use the valid dev_id list to iterate > > > > > over the devices it has open to do the config stuff. > > > > > > > > If an affected device is owned, not opened, and not interfering with > > > > the reset, what is it adding to the API to report it for debugging > > > > purposes? I'm afraid this leads into expanding "invalid dev-id" into an > > > > > > consistent output before and after devB is opened. > > > > In the case where devB is not opened including it only provides > > useless information. In the case where devB is opened it's necessary > > to be reported as an opened, affected device. > > > > > > errno or bitmap of error conditions that the user needs to parse. > > > > > > > > > > Not exactly. > > > > > > If RESETABLE invalid dev_id doesn't matter. The user only use the > > > valid dev_id list to iterate as Jason pointed out. > > > > Yes, but... > > > > > If NOT_RESETTABLE due to devE not assigned to the VM one can > > > easily figure out the fact by simply looking at the list of affected BDFs > > > and the configuration of assigned devices of the VM. Then invalid > > > dev_id also doesn't matter. > > > > Huh? > > > > Given: > > > > flags: NOT_RESETABLE | DEV_ID > > { > > { valid devA-id, devA-BDF }, > > { invalid dev-id, devB-BDF }, > > { valid devC-id, devC-BDF }, > > { valid devD-id, devD-BDF }, > > { invalid dev-id, devE-BDF }, > > } > > > > How does the user determine that devE is to blame and not devB based on > > BDF? The user cannot rely on sysfs for help, they don't know the IOMMU > > grouping, nor do they know the BDF except as inferred by matching valid > > dev-ids in the above output. > > emmm aren't we talking about the 'person' who does diagnostic? This guy > will look at the VM configuration file to know that devA/B/C/D have been > assigned to the VM but not devE. Actually the scenario is that devA/C/D are assigned, devB is implicitly owned, and it's devE that blocks the reset. If you've followed any of the community forums for vfio over the years, it should be readily apparent that placing the burden solely on the end user to perform such a diagnosis is an unreasonable expectation. > > > If NOT_RESETTABLE while devE is already assigned to the VM then it's > > > indication of mixing groups, cdevs or multiple iommufd_ctxs. Then > > > people should debug with other means/hints to dig out the exact > > > culprit. > > > > I don't know what situation you're trying to explain here. If devE > > were opened within the same iommufd_ctx, this becomes: > > It's about a scenario where the mgmt.. stack has assigned all affected > devices to Qemu but Qemu itself messed it up with mixed group/cdev > or multiple iommufd_ctx so hitting the NON_RESETTABLE situation. Is this a reasonable scenario? I expect the QEMU support to favor cdev access where available and fd passing methods will only use cdev, so QEMU should never mess up to create such an environment. There should never be a case where a device is exclusively available via group rather than cdev. > > flags: RESETABLE | DEV_ID > > { > > { valid devA-id, devA-BDF }, > > { invalid dev-id, devB-BDF }, > > { valid devC-id, devC-BDF }, > > { valid devD-id, devD-BDF }, > > { valid devE-id, devE-BDF }, > > } > > > > Yes, the user should only be looking at the flag to determine the > > availability of hot-reset, (here's the but) but how is it consistent to > > indicate both that hot-reset is available and include an invalid > > dev-id? The consistency as I propose is that an invalid dev-id is only > > presented with NOT_RESETTABLE for the device blocking hot-reset. In > > the previous case, devB is not blocking reset and reporting an invalid > > dev-id only serves to obfuscate determining the blocking device. > > > > For the cases of affected group-opened devices or separate > > iommufd_ctxs, the user gets invalid dev-ids for anything outside of > > the calling device's iommufd_ctx. > > > > We haven't discussed how it fails when called on a group-opened device > > in a mixed environment. I'd propose that the INFO ioctl behaves > > exactly as it does today, reporting group-id and BDF for each affected > > device. However, the hot-reset ioctl itself is not extended to accept > > devicefd because there is no proof-of-ownership model for cdevs. > > Therefore even if the user could map group-id to devicefd, they get > > -EINVAL calling HOT_RESET with a devicefd when the ioctl is called from > > a group-opened device. Thanks, > > > > Yes I chatted with Yi about it. > > If the calling device of the INFO ioctl is opened by group then behave > as it does today. > > If the calling device is opened via cdev then use dev_id scheme as > discussed above. > > in hot_reset ioctl the fd array only accepts group fd's. > > cdev can be reset only via null fd array. > > It remains a small open that null fd array could potentially work for > group-opened device too if vfio-compat is used. In that case devices > are in same iommufd ctx with valid dev_id even though they are opened > via group. But probably it's not worthy blocking it? Yes, let's not create new models for the compatibility interface, stick with group-opened = group-id = proof-of-ownership. Thanks, Alex