Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp3988729pxj; Tue, 11 May 2021 17:04:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxE0OySvGtVdES/xYb6/XYHW0pW9BziuTKVNed37+PV+1zC4FZOsJKQS3vZfV1KLkh1dwKt X-Received: by 2002:a17:906:ca4c:: with SMTP id jx12mr28718946ejb.93.1620777883553; Tue, 11 May 2021 17:04:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1620777883; cv=none; d=google.com; s=arc-20160816; b=tUCV0iF5jNbbEnTkN27Ln8A5KjAFStHMM5auF48KjWKaVB+mXGuTEMOtcVBvhjzrkW pLJGFYdr637T8rT+TLT/mUbxBilNnIKtXYIqHOXMDqp16aMjJg9TF+y2Bg7CUZdpZ6RM u70iYm/JUgkt22Q+yTh8DGuCsNauaw8AyN6miOT0/jxBC/BcfSIw07ck33PsNuEvEOAs wakSe7iDhR2hqxYVQqvRVspWSBZIl5HIp4rdFHCYseIiTKwcYjs7Q5zCx9iLqMAz3u1r nDe4HwdqRRd7hT1GqLBPd2t0fhC5r3FziAnhmXx1HIZDOGa++7Za2Huo6KDEFkcl+Z7R AVgw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=32gqo5zV+RGURV86mp7uR8bqTCYhZqWaxrh2ZcUwp+Q=; b=pIt60MUEiPWdO3hNOsaC2ugu+jPmJ0hGTbiiSnNxlFKyxnPhnI2tSjJxDE3bitLwAm kh4N8V3lV+ge2jju+BhbhR2qrGBSzAmSkLFiknarJ/a2A2jNlOHUZnSc6mA1Po1dFe7p jmdinRCbJToUIacprJsdJ1/sJWa1HCQvb8daen2qRuWB9k7zGmbR7jZ1c0owcu/kX7l6 CJS8uIWLKHH0ouiraJA8oYVhZdIgZ8FkN6a4ZsK0gwx9N4DaaXXBUVGPWB4POtQ6IjND QO1nyC3jkK6jAbxglhB8ys16bxXyXCeHeldKIOiSSD4w0DxMAIIyy1PcAGjOWhn+CzMU SGFg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=CCsH611G; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id u12si10550784ejt.21.2021.05.11.17.04.17; Tue, 11 May 2021 17:04:43 -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=@google.com header.s=20161025 header.b=CCsH611G; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230118AbhELADs (ORCPT + 99 others); Tue, 11 May 2021 20:03:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37996 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229934AbhELADr (ORCPT ); Tue, 11 May 2021 20:03:47 -0400 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AB640C06175F for ; Tue, 11 May 2021 17:02:40 -0700 (PDT) Received: by mail-lf1-x12c.google.com with SMTP id t11so31080794lfl.11 for ; Tue, 11 May 2021 17:02:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=32gqo5zV+RGURV86mp7uR8bqTCYhZqWaxrh2ZcUwp+Q=; b=CCsH611G4x21fZfVYf0v8Uqz26dKRwG2KiRnW8DZ9z/vsVBoHqWJw6zP1N41FBHhYN zuDuzFqwXcYoNkcjP815gHThFTedy/Ch4f3inR1yWh9tefwvO5/Eic6epkuRMmqHwjgg zDAs5fkh6LnrUa4Q3fIy3ETxyxF8nfeW2RSbXJMVP79bgmfePYKIvYl4m9TdwO3FoXoj shtmxO29me0yaSlh5vAUHMAHp2ZKEAp5eX5gl79Jh+ZmG4hfo4xC8bWYk4LRhpBhlVrt kBKnwO4Nr8wl6pLw/vYtbm8C3yFsVH66K5up5BUm8+b52GDJK3jHhMQ4wVMG8KWFRsR9 2J+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=32gqo5zV+RGURV86mp7uR8bqTCYhZqWaxrh2ZcUwp+Q=; b=T72KI4nH7ggbqDcD/XjyIx/AOpkDVWEBIun5ox+a/IsWJLlAp9m8hCjqKMeBiaMo2Y 30hoWtSq4xZPcTTNQTOVnXp4LTiuVYNl2dkyvrm027pWVEkqhl2O/4qLyB41Jxq2H5Pc QS4+JgVz/bqN2KRIgDFNKWHoDrTevFBskqsBSHZ7wWSrxT+JR4tM6knwrHXP2gxoaaYY CBf/f25n370Fzz9CDaOXZJfFOSfRH8a/HM0EcpjLFKKxY4ynYvTRXXUBs1p+4d2w8SG8 jr7hsEUjDNkISr0mwP3R/seYoqWEXaIEzIMvTj3FrpBt1yaGC+aFEixkpdq8p7n4Wwkt MV3g== X-Gm-Message-State: AOAM530HELW30UVWptRdyfusl1yghNCw23M9XUOIS6OEQDJNPgoYDaX7 2c0JvJs93w8H/ujBSiQvi3pAjwylxc02PxSGPQ4cxA== X-Received: by 2002:a05:6512:2190:: with SMTP id b16mr22322694lft.122.1620777758826; Tue, 11 May 2021 17:02:38 -0700 (PDT) MIME-Version: 1.0 References: <20210511230228.GA2429744@bjorn-Precision-5520> In-Reply-To: <20210511230228.GA2429744@bjorn-Precision-5520> From: Rajat Jain Date: Tue, 11 May 2021 17:02:02 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] pci: Support "removable" attribute for PCI devices To: Bjorn Helgaas Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Bjorn Helgaas , Alan Stern , Linux Kernel Mailing List , linux-pci , "open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" , Rajat Jain , Jesse Barnes , Dmitry Torokhov , Oliver Neukum , David Laight Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 11, 2021 at 4:02 PM Bjorn Helgaas wrote: > > On Tue, May 11, 2021 at 03:15:11PM -0700, Rajat Jain wrote: > > On Tue, May 11, 2021 at 2:30 PM Bjorn Helgaas wrote: > > > On Fri, Apr 23, 2021 at 07:16:31PM -0700, Rajat Jain wrote: > > > ... > > > This looks like a good start. I think it would be useful to have a > > > more concrete example of how this information will be used. I know > > > that use would be in userspace, so an example probably would not be a > > > kernel patch. If you have user code published anywhere, that would > > > help. Or even a patch to an existing daemon. Or pointers to how > > > "removable" is used for USB devices. > > > > Sure, I'll point to some existing user space code (which will be using > > a similar attribute we are carrying internally). > > Great, thanks! > > > > > + set_pci_dev_removable(dev); > > > > > > So this *only* sets the "removable" attribute based on the > > > ExternalFacingPort or external-facing properties. I think Oliver and > > > David were hinting that maybe we should also set it for devices in > > > hotpluggable slots. What do you think? > > > > I did think about it. So I have a mixed feeling about this. Primarily > > because I have seen the use of hotpluggable slots in situations where > > we wouldn't want to classify the device as removable: > > > > - Using link-state based hotplug as a way to work around unstable PCIe > > links. I have seen PCIe devices marked as hot-pluggable only to ensure > > that if the PCIe device falls off PCI bus due to some reason (e.g. due > > to SI issues or device firmware bugs), the kernel should be able to > > detect it if it does come back up (remember quick "Link-Down" / > > "Link-Up" events in succession?). > > > > - Internal hot-pluggable PCI devices. In my past life, I was working > > on a large system that would have hot-pluggable daughter cards, but > > those wouldn't be user removable. Also, it is conceivable to have > > hot-pluggable M.2 slots for PCIe devices such as NVMEs etc, but they > > may still not be removable by user. I don't think these should be > > treated as "removable". I was also looking at USB as an example where > > this originally came from, USB does ensure that only devices that are > > "user visible" devices are marked as "removable": > > > > 54d3f8c63d69 ("usb: Set device removable state based on ACPI USB data") > > d35e70d50a06 ("usb: Use hub port data to determine whether a port is removable") > > IIUC your main concern is consumer platforms where PCI devices would > be hotplugged via a Thunderbolt or similar cable, and that port > would be marked as an "ExternalFacingPort" so we'd mark them as > "removable". Yes. > > A device in a server hotplug slot would probably *not* be marked as > "removable". The same device in an external chassis connected via an > iPass or similar cable *might* be "removable" depending on whether the > firmware calls the iPass port an "ExternalFacingPort". Yes. > > Does the following capture some of what you're thinking? Maybe some > wordsmithed version of it would be useful in a comment and/or commit > log? Yes, you captured my thoughts perfectly. I shall update the commit log and / or provide comments to reflect this. > > We're mainly concerned with consumer platforms with accessible > Thunderbolt ports that are vulnerable to DMA attacks, and we expect > those ports to be identified as "ExternalFacingPort". > > Devices in traditional hotplug slots are also "removable," but not > as vulnerable because these slots are less accessible to users. > > > > I wonder if this (and similar hooks like set_pcie_port_type(), > > > set_pcie_untrusted(), set_pcie_thunderbolt(), etc) should go *after* > > > the early fixups so we could use fixups to work around issues? > > > > I agree. We can do that if none of the early fixups actually use the > > fields set by these functions. I think it should be ok to move > > set_pcie_untrusted(), set_pcie_thunderbolt(), but I wonder if any > > early fixups already use the pcie_cap or any other fields set by > > set_pcie_port_type(). > > I think you should move the one you're adding > (set_pci_dev_removable()) and leave the others where they are for now. Ack, will do. Thanks, Rajat > > No need to expand the scope of your patch; I was just thinking they're > all basically similar and should ideally be done at similar times. > > > > > /* Early fixups, before probing the BARs */ > > > > pci_fixup_device(pci_fixup_early, dev); > > > > > > > > -- > > > > 2.31.1.498.g6c1eba8ee3d-goog > > > >