All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	"Natu, Mahesh" <mahesh.natu@intel.com>,
	Chet R Douglas <chet.r.douglas@intel.com>,
	Ben Widawsky <ben.widawsky@intel.com>,
	"Vishal L Verma" <vishal.l.verma@intel.com>
Subject: Re: [RFC] ACPI Code First ECR: Generic Target
Date: Wed, 10 Feb 2021 11:23:30 +0000	[thread overview]
Message-ID: <20210210112330.00003e74@Huawei.com> (raw)
In-Reply-To: <CAPcyv4gmd_cygXK0PpGkXmJLC3_ctEpRvpi5P-QcuXusFX5oNQ@mail.gmail.com>

On Tue, 9 Feb 2021 19:55:05 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> While the platform BIOS is able to describe the performance
> characteristics of CXL memory that is present at boot, it is unable to
> statically enumerate the performance of CXL memory hot inserted
> post-boot. The OS can enumerate most of the characteristics from link
> registers and CDAT, but the performance from the CPU to the host
> bridge, for example, is not enumerated by PCIE or CXL. Introduce an
> ACPI mechanism for this purpose. Critically this is achieved with a
> small tweak to how the existing Generic Initiator proximity domain is
> utilized in the HMAT.

Hi Dan,

Agree there is a hole here, but I think the proposed solution has some
issues for backwards compatibility.

Just to clarify, I believe CDAT from root ports is sufficient for the
other direction (GI on CXL, memory in host).  I wondered initially if
this was a two way issue, but after a reread, I think that is fine
with the root port providing CDAT or potentially treating the root
port as a GI (though that runs into the same naming / representation issue
as below and I think would need some clarifying text in UEFI GI description)

http://uefi.org/sites/default/files/resources/Coherent%20Device%20Attribute%20Table_1.01.pdf

For the case you are dealing with here potentially we 'could' add something
to CDAT as alternative to changing SRAT, but it would be more complex
so your approach here makes more sense to me.

> 
> ---
> 
> # Title: Introduce a Generic Target for CXL
> 
> # Status: Draft
> 
> # Document: ACPI Specification 6.4
> 
> # License
> SPDX-License Identifier: CC-BY-4.0
> 
> # Submitter:
> * Sponsor: Dan Williams, Intel
> * Creators/Contributors:
>     * Mahesh Natu, Intel
>     * Chet Douglas, Intel
>     * Deepak Shivakumar, Intel
> 
> # Summary of the Change
> Introduce a "Generic Target" concept to the SRAT to describe the root
> performance parameters in the path to dynamically discovered (outside of
> ACPI enumeration) CXL memory target endpoints.
> 
> # Benefits of the Change
> Consider the case of a system with a set of CXL host bridges (ACPI0016),

Superficially feels like this new SRAT entry might reference the CXL 2.0 Root
ports or the host bridge.

> and no devices attached at initial system power-on. In this scenario
> platform firmware is unable to perform the end-to-end enumeration
> necessary to populate SRAT and HMAT for the endpoints that may be
> hot-inserted behind those bridges post power-on. The address-range is
> unknown so SRAT can not be pre-populated, the performance is unknown (no
> CDAT nor interleave configuration) so HMAT can not be pre-populated.
> 
> However, what is known to platform firmware that generates the SRAT and
> HMAT is the performance characteristics of the path between CPU and
> Generic Initiators to the CXL host bridge target. With either
> CPU-to-Generic-Target, or Generic-Initiator-to-Generic-Target entries in
> the HMAT the OS CXL subsystem can enumerate the remaining details (PCIE
> link status, device CDAT, interleave configuration) to calculate the
> bandwidth and latency of a dynamically discovered CXL memory target.

I'm wondering if the term "generic target" is a good name.
Would something like "generic target bridge" be clearer?
The point being this isn't an actual target but a point along the way.
Mind you this is close to bike shedding.

As mentioned above, maybe "generic bridge" that can give us a node to hang
data off for both, a) GI on CXL to host memory, and b) Initiator in host to CXL memory
and hence give cleaner representation.

> 
> # Impact of the Change
> The existing Generic Initiator Affinity Structure (ACPI 6.4 Section
> 5.2.16.6) already contains all the fields necessary to enumerate a
> generic target proximity domain. All that is missing is the
> interpretation of that proximity domain optionally as a target
> identifier in the HMAT.
> 
> Given that the OS still needs to dynamically enumerate and instantiate
> the memory ranges behind the host bridge. The assumption is that
> operating systems that do not support native CXL enumeration will ignore
> this data in the HMAT, while CXL native enumeration aware environments
> will use this fragment of the performance path to calculate the
> performance characteristics.

I don't think it is true that OS not supporting native CXL will ignore the
data.

Linux will create a small amount of infrastructure to reflect them (more or
less the same as a memoryless node) and also they will appear in places
like access0 as a possible initiator of transactions.  It's small stuff,
but I'd rather the impact on legacy was zero.

So my gut feeling here is we shouldn't reuse the generic initiator, but
should invent something new.  Would look similar to GI, but with a different
ID - to ensure legacy OS ignores it.

Unfortunately we can't just add a flag because backwards compatibility
with old OS would mean it was ignored.  Hence I think this needs to be
a new type.

If we define a new node type rather than extend GI, we need to be careful
around the same issue with _PXM that we had when introducing Generic
Initiators (not sure the protections on that made it back to stable)
so might need to modify DSDT _PXM responses based on appropriate _OSC.
May well be fine but I'm not convinced yet.  Perhaps we need to say
that using _PXM to place anything in a node defined only via this new means
is not valid.

Jonathan

> 
> # References
> * Compute Express Link Specification v2.0,
> <https://www.computeexpresslink.org/>
> 
> # Detailed Description of the Change
> 
> * Replace "Generic Initiator" with "Generic Initiator / Target" in all
> locations except where an "initiator" or "target" is implied.
> Specifically 5.2.27.3 "Memory Proximity Domain Attributes Structure"
> need not replace occurrences of "generic initiator" in field: "Proximity
> Domain for Attached Initiator". Additionally field: "Proximity Domain
> for the Memory" must be renamed to "Proximity Domain for the Memory /
> Generic Target" with a new description "Integer that represents the
> memory / generic target proximity domain to which this memory belongs."
> 
> * Revise "5.2.16.6 Generic Initiator Affinity Structure" to make it
>   consistent with being referenced as either a target or initiator.
> 
>         * Description: (replace all text)
> 
>         > The Generic Initiator / Target Affinity Structure provides the
>         > association between a Generic Initiator and a Memory Proximity
>         > Domain, or another Generic Target Proximity Domain. The
>         > distinction as to whether this structure represents an
>         > Initiator, a Target, or both depends on how it is referenced
>         > in the HMAT. See Section 5.2.27.3 for details.  
> 
>         > Support of Generic Initiator / Target Affinity Structures by
>         > OSPM is optional, and the platform may query whether the OS
>         > supports it via the _OSC method. See Section 6.2.11.2.  
> 
>         * Architectural transactions: (append after current text)
> 
>         > If this proximity domain is referenced as a target then it
>         > supports all the transaction types inferred above.  
> 
>         * Other updates are simple Initiator => Initiator / Target
>           replacements.



  reply	other threads:[~2021-02-10 11:27 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10  3:55 [RFC] ACPI Code First ECR: Generic Target Dan Williams
2021-02-10 11:23 ` Jonathan Cameron [this message]
2021-02-10 15:18   ` Natu, Mahesh
2021-02-10 16:02     ` Jonathan Cameron
2021-02-10 16:24   ` Dan Williams
2021-02-11  9:42     ` Jonathan Cameron
2021-02-11 17:06       ` Dan Williams
2021-02-12 12:24         ` Jonathan Cameron
2021-02-12 23:51           ` Dan Williams
2021-02-16 11:06             ` Jonathan Cameron
2021-02-16 16:29               ` Dan Williams
2021-02-16 18:06                 ` Jonathan Cameron
2021-02-16 18:22                   ` Dan Williams
2021-02-16 18:58                     ` Jonathan Cameron
2021-02-16 19:41                       ` Dan Williams
2021-02-17  9:53                         ` Jonathan Cameron
2021-02-10 17:02 ` Vikram Sethi
2021-02-12  0:13   ` Natu, Mahesh

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=20210210112330.00003e74@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=ben.widawsky@intel.com \
    --cc=chet.r.douglas@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=mahesh.natu@intel.com \
    --cc=vishal.l.verma@intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.