Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Hanjun Guo <guohanjun@huawei.com>
To: Robin Murphy <robin.murphy@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>,
	Will Deacon <will@kernel.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	<iommu@lists.linux-foundation.org>, <linux-acpi@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>, Joerg Roedel <joro@8bytes.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Marc Zyngier <maz@kernel.org>,
	Makarand Pawagi <makarand.pawagi@nxp.com>,
	Diana Craciun <diana.craciun@oss.nxp.com>,
	Laurentiu Tudor <laurentiu.tudor@nxp.com>
Subject: Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC
Date: Thu, 2 Jul 2020 16:22:00 +0800
Message-ID: <0cbd1da8-e283-7e13-d2b3-4d14775fd870@huawei.com> (raw)
In-Reply-To: <952a6720-f401-1441-5548-5b40cfc76d3a@arm.com>

Hi Robin,

On 2020/7/2 0:12, Robin Murphy wrote:
> On 2020-06-30 14:04, Hanjun Guo wrote:
>> On 2020/6/30 18:24, Lorenzo Pieralisi wrote:
>>> On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:
>>>
>>> [...]
>>>
>>>>> For devices that aren't described in the DSDT - IORT translations
>>>>> are determined by their ACPI parent device. Do you see/Have you
>>>>> found any issue with this approach ?
>>>>
>>>> The spec says "Describes the IO relationships between devices
>>>> represented in the ACPI namespace.", and in section 3.1.1.3 Named
>>>> component node, it says:
>>>
>>> PCI devices aren't necessarily described in the ACPI namespace and we
>>> still use IORT to describe them - through the RC node.
>>>
>>>> "Named component nodes are used to describe devices that are also
>>>> included in the Differentiated System Description Table (DSDT). See
>>>> [ACPI]."
>>>>
>>>> So from my understanding, the IORT spec for now, can only do ID
>>>> translations for devices in the DSDT.
>>>
>>> I think you can read this multiple ways but this patch does not
>>> change this concept. What changes, is applying parent's node IORT
>>> mapping to child nodes with no associated DSDT nodes, it is the
>>> same thing we do with PCI and the _DMA method - we could update
>>> the wording in the specs if that clarifies but I don't think this
>>> deliberately disregards the specifications.
>>
>> I agree, but it's better to update the wording of the spec.
>>
>>>
>>>>>> For a platform device, if I use its parent's full path name for
>>>>>> its named component entry, then it will match, but this will violate
>>>>>> the IORT spec.
>>>>>
>>>>> Can you elaborate on this please I don't get the point you
>>>>> are making.
>>>>
>>>> For example, device A is not described in DSDT so can't represent
>>>> as a NC node in IORT. Device B can be described in DSDT and it
>>>> is the parent of device A, so device B can be represented in IORT
>>>> with memory access properties and node flags with Substream width
>>>> and Stall supported info.
>>>>
>>>> When we trying to translate device A's ID, we reuse all the memory
>>>> access properties and node flags from its parent (device B), but
>>>> will it the same?
>>>
>>> I assume so why wouldn't it be ? Why would be describe them in
>>> a parent-child relationship if that's not how the system looks like
>>> in HW ?
>>
>> The point I'm making is that I'm not sure all the memory access and
>> stall properties are the same for the parent and the device itself.
> 
> Is that even a valid case though? The principal thing we want to 
> accommodate here is when device B *is* the one accessing memory, either 
> because it is a bridge with device A sat behind it, or because device A 
> is actually just some logical function or subset of physical device B.

Thanks for the clarify, for CCA attributes, child device should be the
same as its parent and that was written in the ACPI spec, so it's better
to make it clear for other properties in the spec as well.

> 
> If the topology is such that device A is a completely independent device 
> with its own path to memory such that it could have different 
> properties, I would expect that it *should* be described in DSDT, and I 
> can't easily think of a good reason why it wouldn't be. I'm also 
> struggling to imagine how it might even have an ID that had to be 
> interpreted in the context of device B if it wasn't one of the cases 
> above :/
> 
> I don't doubt that people could - or maybe even have - come up with crap 
> DSDT bindings that don't represent the hardware sufficiently accurately, 
> but I'm not sure that should be IORT's problem...

As I said in previous email, I'm not against this patch, and seems
have no regressions for platforms that using named component node
such as D05/D06 (I will test it shortly to make sure), but it's better
to update the wording of the spec (even after this patch set is merged).

Thanks
Hanjun


  reply index

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 12:59 [PATCH 00/12] ACPI/OF: Upgrade MSI/IOMMU ID mapping APIs Lorenzo Pieralisi
2020-05-21 12:59 ` [PATCH 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC Lorenzo Pieralisi
2020-05-21 12:59 ` [PATCH 02/12] ACPI/IORT: Make iort_get_device_domain IRQ domain agnostic Lorenzo Pieralisi
2020-05-21 19:56   ` Bjorn Helgaas
2020-05-21 12:59 ` [PATCH 03/12] ACPI/IORT: Make iort_msi_map_rid() PCI agnostic Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 04/12] ACPI/IORT: Remove useless PCI bus walk Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 05/12] ACPI/IORT: Add an input ID to acpi_dma_configure() Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic Lorenzo Pieralisi
2020-05-21 22:47   ` Rob Herring
2020-06-04 14:27     ` Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 07/12] of/device: Add input id to of_dma_configure() Lorenzo Pieralisi
2020-05-21 23:02   ` Rob Herring
2020-06-04 14:49     ` Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 08/12] of/irq: make of_msi_map_get_device_domain() bus agnostic Lorenzo Pieralisi
2020-05-21 19:57   ` Bjorn Helgaas
2020-05-21 13:00 ` [PATCH 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus Lorenzo Pieralisi
2020-05-21 23:10   ` Rob Herring
2020-05-22  9:42     ` Robin Murphy
2020-05-22  9:57       ` Diana Craciun OSS
2020-05-22 14:08         ` Rob Herring
2020-05-22 14:34           ` Robin Murphy
2020-05-22 14:02       ` Rob Herring
2020-05-22 15:38         ` Laurentiu Tudor
2020-05-21 13:00 ` [PATCH 10/12] of/irq: Make of_msi_map_rid() PCI bus agnostic Lorenzo Pieralisi
2020-05-21 23:17   ` Rob Herring
2020-06-04 15:08     ` Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 11/12] bus/fsl-mc: Refactor the MSI domain creation in the DPRC driver Lorenzo Pieralisi
2020-05-21 13:00 ` [PATCH 12/12] bus: fsl-mc: Add ACPI support for fsl-mc Lorenzo Pieralisi
2020-05-21 15:03   ` Laurentiu Tudor
2020-05-22  5:32     ` Makarand Pawagi
2020-05-22  9:53       ` Lorenzo Pieralisi
2020-06-19  8:20 ` [PATCH v2 00/12] ACPI/OF: Upgrade MSI/IOMMU ID mapping APIs Lorenzo Pieralisi
2020-06-19  8:20   ` [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC Lorenzo Pieralisi
2020-06-29  4:24     ` Hanjun Guo
2020-06-29  9:05       ` Lorenzo Pieralisi
2020-06-30  3:06         ` Hanjun Guo
2020-06-30 10:24           ` Lorenzo Pieralisi
2020-06-30 13:04             ` Hanjun Guo
2020-07-01 16:12               ` Robin Murphy
2020-07-02  8:22                 ` Hanjun Guo [this message]
2020-07-09  9:21                   ` Lorenzo Pieralisi
2020-07-09 12:48                     ` Hanjun Guo
2020-06-19  8:20   ` [PATCH v2 02/12] ACPI/IORT: Make iort_get_device_domain IRQ domain agnostic Lorenzo Pieralisi
2020-06-19  8:20   ` [PATCH v2 03/12] ACPI/IORT: Make iort_msi_map_rid() PCI agnostic Lorenzo Pieralisi
2020-07-15  9:15     ` Lorenzo Pieralisi
2020-07-21 14:59     ` Bjorn Helgaas
2020-07-27  6:06       ` [EXT] " Makarand Pawagi
2020-06-19  8:20   ` [PATCH v2 04/12] ACPI/IORT: Remove useless PCI bus walk Lorenzo Pieralisi
2020-06-19  8:20   ` [PATCH v2 05/12] ACPI/IORT: Add an input ID to acpi_dma_configure() Lorenzo Pieralisi
2020-07-09  9:35     ` Lorenzo Pieralisi
2020-07-15  9:13       ` Lorenzo Pieralisi
2020-07-28 12:48         ` Lorenzo Pieralisi
2020-07-28 13:00           ` Rafael J. Wysocki
2020-06-19  8:20   ` [PATCH v2 06/12] of/iommu: Make of_map_rid() PCI agnostic Lorenzo Pieralisi
2020-06-22 13:26     ` Joerg Roedel
2020-07-13 23:57     ` Rob Herring
2020-06-19  8:20   ` [PATCH v2 07/12] of/device: Add input id to of_dma_configure() Lorenzo Pieralisi
2020-06-30 21:50     ` Rob Herring
2020-06-19  8:20   ` [PATCH v2 08/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus Lorenzo Pieralisi
2020-06-30 21:55     ` Rob Herring
2020-06-19  8:20   ` [PATCH v2 09/12] of/irq: make of_msi_map_get_device_domain() bus agnostic Lorenzo Pieralisi
2020-06-30 21:50     ` Rob Herring
2020-06-19  8:20   ` [PATCH v2 10/12] of/irq: Make of_msi_map_rid() PCI " Lorenzo Pieralisi
2020-06-30 21:56     ` Rob Herring
2020-06-19  8:20   ` [PATCH v2 11/12] bus/fsl-mc: Refactor the MSI domain creation in the DPRC driver Lorenzo Pieralisi
2020-07-15 13:05     ` Marc Zyngier
2020-06-19  8:20   ` [PATCH v2 12/12] bus: fsl-mc: Add ACPI support for fsl-mc Lorenzo Pieralisi
2020-07-01 16:55     ` Laurentiu Tudor
2020-07-09  9:19       ` Lorenzo Pieralisi
2020-07-09  9:26         ` [EXT] " Makarand Pawagi
2020-07-09 10:14           ` Laurentiu Tudor
2020-07-09 10:37             ` Makarand Pawagi
2020-07-09 10:39               ` Laurentiu Tudor
2020-07-09 10:47               ` Diana Craciun OSS
2020-07-09 10:52                 ` Makarand Pawagi
2020-07-15 10:06                   ` Lorenzo Pieralisi
2020-07-16  3:23                     ` Makarand Pawagi
2020-07-16  7:57                       ` Marc Zyngier
2020-07-20 16:54   ` [PATCH v2 00/12] ACPI/OF: Upgrade MSI/IOMMU ID mapping APIs Lorenzo Pieralisi
2020-07-21  4:28     ` [EXT] " Makarand Pawagi
2020-07-28 17:01   ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0cbd1da8-e283-7e13-d2b3-4d14775fd870@huawei.com \
    --to=guohanjun@huawei.com \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=diana.craciun@oss.nxp.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=laurentiu.tudor@nxp.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=makarand.pawagi@nxp.com \
    --cc=maz@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=sudeep.holla@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git