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;
MIME-Version: 1.0
References: <20220120000409.2706549-1-rajatja@google.com> <20220121214117.GA1154852@bhelgaas>
 <Ye5GvQbFKo+CFtRb@lahna> <Ye/X7E2dKb+zem34@lahna> <Ye/btvA1rLB2rp02@kroah.com>
 <Ye/zTHR5aCG58z87@lahna> <CAJZ5v0gitdeEAxcgSoB1=VHA9FnRdCtmUqA_cN_f1a2yFRDghQ@mail.gmail.com>
In-Reply-To: <CAJZ5v0gitdeEAxcgSoB1=VHA9FnRdCtmUqA_cN_f1a2yFRDghQ@mail.gmail.com>
From:   Rajat Jain <rajatja@google.com>
Date:   Thu, 27 Jan 2022 14:26:07 -0800
Message-ID: <CACK8Z6H2DLTJgxgS3pcvfOh=5S8cxEMKvwEPfB9zoVf1g2H_UQ@mail.gmail.com>
Subject: Re: [PATCH] PCI: ACPI: Allow internal devices to be marked as untrusted
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     Mika Westerberg <mika.westerberg@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Bjorn Helgaas <helgaas@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
        Linux PCI <linux-pci@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Rajat Jain <rajatxjain@gmail.com>,
        Dmitry Torokhov <dtor@google.com>,
        Jesse Barnes <jsbarnes@google.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.org>,
        Pavel Machek <pavel@denx.de>,
        "Oliver O'Halloran" <oohall@gmail.com>,
        Joerg Roedel <joro@8bytes.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello Rafael, Bjorn, Mika, Dmitry, Greg,

Thanks a lot for your comments.

On Tue, Jan 25, 2022 at 6:45 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, Jan 25, 2022 at 1:55 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > On Tue, Jan 25, 2022 at 12:15:02PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 25, 2022 at 12:58:52PM +0200, Mika Westerberg wrote:
> > > > On Mon, Jan 24, 2022 at 08:27:17AM +0200, Mika Westerberg wrote:
> > > > > > > This patch introduces a new "UntrustedDevice" property that can be used
> > > > > > > by the firmware to mark any device as untrusted.
> > > > >
> > > > > I think this new property should be documented somewhere too (also
> > > > > explain when to use it instead of ExternalFacingPort). If not in the
> > > > > next ACPI spec or some supplemental doc then perhaps in the DT bindings
> > > > > under Documentation/devicetree/bindings.
> > > >
> > > > Actually Microsoft has similar already:
> > > >
> > > > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-internal-pcie-ports-accessible-to-users-and-requiring-dma-protection
> > > >
> > > > I think we should use that too here.

But because this property also applies to a root port (only), it only
helps if the device is downstream a PCIe root port. In our case, we
have an internal (wifi) device 00:14.3 (sits on the internal PCI bus
0), so cannot use this.

> > >
> > > But we do not have "dma protection" for Linux, so how will that value
> > > make sense?
> >
> > Yes I think we do - IOMMU. That's the same thing what we do now for
> > "External Facing Ports". This one just is for internal ones.
> >
> > > And shouldn't this be an ACPI standard?
> >
> > Probably should or some supplemental doc but not sure how easy these
> > "properties" can be added there to be honest.

AIUI, the principal comment I have received here is that this property
needs to be documented somewhere. I agree.

Rafael, do you know if this new property can be added to the ACPI
spec, and if so, how to do so? I'm happy to initiate a process if
someone can point me to, I just hope that publishing a new property to
the ACPI does not have to block this patch.

The other option I was thinking of was to use the same property name
(say "untrusted-device") for both ACPI and device tree platforms, and
document it in Documentation/devicetree/bindings/pci/pci.txt along
with others. Since there are other properties there that seem to be
used similarly (Mika highlighted some below), perhaps that is an
acceptable solution?

I had one last question on the property name itself. I was trying to
understand why a property might have 2 names i.e. "external-facing"
for DT and "ExternalFacingPort" in ACPI? Are there any naming
convention requirements that require ACPI and DT property names to be
different? Is "untrusted-device" an acceptable ACPI property name?

Thanks & Best Regards,

Rajat

> >
> > Some of these that we use in Linux too are from that same page:
> >
> > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports
> >
> > Namely these: HotPlugSupportInD3, ExternalFacingPort, usb4-host-interface,
> > usb4-port-number and StorageD3Enable.
>
> Right.
>
> We are kind of on the receiving end here, because at the time we learn
> about these things the decisions to use them have been made already.