| Message ID | 20240422091350.4701-2-yaoxt.fnst@fujitsu.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl/region: add interleave capability check | expand |
On Mon, 22 Apr 2024 05:13:49 -0400 Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote: > Per CXL specification (8.2.4.20 CXL HDM Decoder Capability Structure in > r3.1), the host-bridges with single port and switches with single dport are > not affiliated with any instance of the HDM capability structure. To avoid any confusion in future, this is an option, but even with single downstream ports they may support HDM decoders. > > Driver will add a passthrough decoder for each of them during init, so the > constraints imposed by the HDM capability structure do not apply to the > passthrough decoders. > > By utilizing this flag, we can swiftly ascertain whether a switch decoder > qualifies as a passthrough decoder, thereby avoiding the need to conduct a > string of capability constraint checks. > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> With above description amended to reflect that it's talking about an implementation option. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/core/hdm.c | 1 + > drivers/cxl/cxl.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > index 7d97790b893d..27fb4f9d489e 100644 > --- a/drivers/cxl/core/hdm.c > +++ b/drivers/cxl/core/hdm.c > @@ -57,6 +57,7 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port *port) > if (IS_ERR(cxlsd)) > return PTR_ERR(cxlsd); > > + cxlsd->passthrough = true; > device_lock_assert(&port->dev); > > xa_for_each(&port->dports, index, dport) > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 036d17db68e0..6f562baa164f 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -415,6 +415,7 @@ struct cxl_endpoint_decoder { > * struct cxl_switch_decoder - Switch specific CXL HDM Decoder > * @cxld: base cxl_decoder object > * @nr_targets: number of elements in @target > + * @passthrough: indicate whether the decoder is passthrough > * @target: active ordered target list in current decoder configuration > * > * The 'switch' decoder type represents the decoder instances of cxl_port's that > @@ -426,6 +427,7 @@ struct cxl_endpoint_decoder { > struct cxl_switch_decoder { > struct cxl_decoder cxld; > int nr_targets; > + bool passthrough; > struct cxl_dport *target[]; > }; >
> -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Monday, April 22, 2024 7:12 PM > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Cc: dave@stgolabs.net; dave.jiang@intel.com; alison.schofield@intel.com; > vishal.l.verma@intel.com; ira.weiny@intel.com; dan.j.williams@intel.com; > jim.harris@samsung.com; linux-cxl@vger.kernel.org > Subject: Re: [PATCH v4 1/2] cxl/core: add passthrough flag to struct > cxl_switch_decoder > > On Mon, 22 Apr 2024 05:13:49 -0400 > Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote: > > > Per CXL specification (8.2.4.20 CXL HDM Decoder Capability Structure in > > r3.1), the host-bridges with single port and switches with single dport are > > not affiliated with any instance of the HDM capability structure. > > To avoid any confusion in future, this is an option, but even with single > downstream ports they may support HDM decoders. > > > > > Driver will add a passthrough decoder for each of them during init, so the > > constraints imposed by the HDM capability structure do not apply to the > > passthrough decoders. > > > > By utilizing this flag, we can swiftly ascertain whether a switch decoder > > qualifies as a passthrough decoder, thereby avoiding the need to conduct a > > string of capability constraint checks. > > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > With above description amended to reflect that it's talking about an > implementation option. OK, got it. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > --- > > drivers/cxl/core/hdm.c | 1 + > > drivers/cxl/cxl.h | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > > index 7d97790b893d..27fb4f9d489e 100644 > > --- a/drivers/cxl/core/hdm.c > > +++ b/drivers/cxl/core/hdm.c > > @@ -57,6 +57,7 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port > *port) > > if (IS_ERR(cxlsd)) > > return PTR_ERR(cxlsd); > > > > + cxlsd->passthrough = true; > > device_lock_assert(&port->dev); > > > > xa_for_each(&port->dports, index, dport) > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > > index 036d17db68e0..6f562baa164f 100644 > > --- a/drivers/cxl/cxl.h > > +++ b/drivers/cxl/cxl.h > > @@ -415,6 +415,7 @@ struct cxl_endpoint_decoder { > > * struct cxl_switch_decoder - Switch specific CXL HDM Decoder > > * @cxld: base cxl_decoder object > > * @nr_targets: number of elements in @target > > + * @passthrough: indicate whether the decoder is passthrough > > * @target: active ordered target list in current decoder configuration > > * > > * The 'switch' decoder type represents the decoder instances of cxl_port's that > > @@ -426,6 +427,7 @@ struct cxl_endpoint_decoder { > > struct cxl_switch_decoder { > > struct cxl_decoder cxld; > > int nr_targets; > > + bool passthrough; > > struct cxl_dport *target[]; > > }; > >
Yao Xingtao wrote: > Per CXL specification (8.2.4.20 CXL HDM Decoder Capability Structure in > r3.1), the host-bridges with single port and switches with single dport are > not affiliated with any instance of the HDM capability structure. > > Driver will add a passthrough decoder for each of them during init, so the > constraints imposed by the HDM capability structure do not apply to the > passthrough decoders. > > By utilizing this flag, we can swiftly ascertain whether a switch decoder > qualifies as a passthrough decoder, thereby avoiding the need to conduct a > string of capability constraint checks. > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > --- > drivers/cxl/core/hdm.c | 1 + > drivers/cxl/cxl.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > index 7d97790b893d..27fb4f9d489e 100644 > --- a/drivers/cxl/core/hdm.c > +++ b/drivers/cxl/core/hdm.c > @@ -57,6 +57,7 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port *port) > if (IS_ERR(cxlsd)) > return PTR_ERR(cxlsd); > > + cxlsd->passthrough = true; > device_lock_assert(&port->dev); > > xa_for_each(&port->dports, index, dport) > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 036d17db68e0..6f562baa164f 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -415,6 +415,7 @@ struct cxl_endpoint_decoder { > * struct cxl_switch_decoder - Switch specific CXL HDM Decoder > * @cxld: base cxl_decoder object > * @nr_targets: number of elements in @target > + * @passthrough: indicate whether the decoder is passthrough > * @target: active ordered target list in current decoder configuration > * > * The 'switch' decoder type represents the decoder instances of cxl_port's that > @@ -426,6 +427,7 @@ struct cxl_endpoint_decoder { > struct cxl_switch_decoder { > struct cxl_decoder cxld; > int nr_targets; > + bool passthrough; This flag does not make sense to me. The definition of a passthrough decoder is one that can mirror the interleave settings of its upstream to its downstream, so just reflect that that in the interleave capabilities of the decoder and skip the special case. I.e. set interleave_mask and iw_cap_mask so that there is no limitation.
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c index 7d97790b893d..27fb4f9d489e 100644 --- a/drivers/cxl/core/hdm.c +++ b/drivers/cxl/core/hdm.c @@ -57,6 +57,7 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port *port) if (IS_ERR(cxlsd)) return PTR_ERR(cxlsd); + cxlsd->passthrough = true; device_lock_assert(&port->dev); xa_for_each(&port->dports, index, dport) diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 036d17db68e0..6f562baa164f 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -415,6 +415,7 @@ struct cxl_endpoint_decoder { * struct cxl_switch_decoder - Switch specific CXL HDM Decoder * @cxld: base cxl_decoder object * @nr_targets: number of elements in @target + * @passthrough: indicate whether the decoder is passthrough * @target: active ordered target list in current decoder configuration * * The 'switch' decoder type represents the decoder instances of cxl_port's that @@ -426,6 +427,7 @@ struct cxl_endpoint_decoder { struct cxl_switch_decoder { struct cxl_decoder cxld; int nr_targets; + bool passthrough; struct cxl_dport *target[]; };
Per CXL specification (8.2.4.20 CXL HDM Decoder Capability Structure in r3.1), the host-bridges with single port and switches with single dport are not affiliated with any instance of the HDM capability structure. Driver will add a passthrough decoder for each of them during init, so the constraints imposed by the HDM capability structure do not apply to the passthrough decoders. By utilizing this flag, we can swiftly ascertain whether a switch decoder qualifies as a passthrough decoder, thereby avoiding the need to conduct a string of capability constraint checks. Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> --- drivers/cxl/core/hdm.c | 1 + drivers/cxl/cxl.h | 2 ++ 2 files changed, 3 insertions(+)