diff mbox series

[11/15] cxl/acpi: Extract the host's component register base address from RCRB

Message ID 20220831081603.3415-12-rrichter@amd.com
State Superseded
Delegated to: Dan Williams
Headers show
Series cxl: Add support for Restricted CXL hosts (RCD mode) | expand

Commit Message

Robert Richter Aug. 31, 2022, 8:15 a.m. UTC
A downstream port must be connected to a component register block.
Determine its base address from the RCRB.

The implementation is analog to how cxl_setup_regs() is implemented
for CXL VH mode. A struct cxl_component_reg_map is filled in, mapped
and probed.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/acpi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Jonathan Cameron Aug. 31, 2022, 11:56 a.m. UTC | #1
On Wed, 31 Aug 2022 10:15:59 +0200
Robert Richter <rrichter@amd.com> wrote:

> A downstream port must be connected to a component register block.
> Determine its base address from the RCRB.
> 
> The implementation is analog to how cxl_setup_regs() is implemented
> for CXL VH mode. A struct cxl_component_reg_map is filled in, mapped
> and probed.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
A few comments inline.  Mostly requests for references for things
I couldn't find in the specs.

> ---
>  drivers/cxl/acpi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 439df9df2741..88bbd2bb61fc 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -401,12 +401,84 @@ static resource_size_t cxl_get_rcrb(u32 uid)
>  	return ctx.chbcr;
>  }
>  
> +static resource_size_t cxl_get_component_reg_phys(resource_size_t rcrb)
> +{
> +	resource_size_t component_reg_phys;
> +	u32 bar0, bar1;
> +	void *addr;
> +
> +	/*
> +	 * RCRB's BAR[0..1] point to component block containing CXL subsystem
> +	 * component registers.
> +	 * CXL 8.2.4 - Component Register Layout Definition.

For references include spec version.  Based on discussion other day,
preference is always latest version. So r3.0 8.2.3
is probably right. I think your references are CXL r2.0?


> +	 *
> +	 * Also, RCRB accesses must use MMIO readl()/readq() to guarantee
> +	 * 32/64-bit access.
> +	 * CXL 8.2.2 - CXL 1.1 Upstream and Downstream Port Subsystem Component
> +	 * Registers
> +	 */
> +	addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8);
> +	bar0 = readl(addr + PCI_BASE_ADDRESS_0);
> +	bar1 = readl(addr + PCI_BASE_ADDRESS_1);

The spec is a bit confusing on this, but I think you are reading into
MEMBAR0 of the RCRB, for which there isn't a lot of description other than
it being an address. It's referred to as a 64-bit BAR in places so you
might be right - or it might be intended to be a bare address..

We might want a clarification on this...

Also it's a 64 bit address so we need to read it in one go. However it's
referred to as a being a 64 bit address at 0x10 and 0x14 so who knows...


> +	iounmap(addr);
> +
> +	/* sanity check */
> +	if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO))
> +		return CXL_RESOURCE_NONE;
> +
> +	component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK;
> +	if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64)
> +		component_reg_phys |= ((u64)bar1) << 32;
> +
> +	if (!component_reg_phys)
> +		return CXL_RESOURCE_NONE;
> +
> +	/*
> +	 * Must be 8k aligned (size of combined CXL 1.1 Downstream and
> +	 * Upstream Port RCRBs).

The rcrb is 8k though I'm not immediately spotting an alignment requirement,
but I'm not sure the component regs have any restrictions do... Add a reference perhaps?
For non RCD devices there is a 64K alignment requirement, but I can't find
anything for RCDs (might just be missing it).

> +	 */
> +	if (component_reg_phys & (SZ_8K - 1))
> +		return CXL_RESOURCE_NONE;
> +
> +	return component_reg_phys;
> +}
> +
> +static int cxl_setup_component_reg(struct device *parent,
> +				   resource_size_t component_reg_phys)
> +{
> +	struct cxl_component_reg_map comp_map;
> +	void __iomem *base;
> +
> +	if (component_reg_phys == CXL_RESOURCE_NONE)
> +		return -EINVAL;
> +
> +	base = ioremap(component_reg_phys, SZ_64K);

Add a spec reference for the size. Table 8-21 perhaps?

> +	if (!base) {
> +		dev_err(parent, "failed to map registers\n");
> +		return -ENOMEM;
> +	}
> +
> +	cxl_probe_component_regs(parent, base, &comp_map);
> +	iounmap(base);
> +
> +	if (!comp_map.hdm_decoder.valid) {
> +		dev_err(parent, "HDM decoder registers not found\n");
> +		return -ENXIO;

Hmm. HDM decoder capability is optional for RCDs - might be using the
range registers.  Seems like we'd really want to handle that for
RCDs.  Future work I guess.

> +	}
> +
> +	dev_dbg(parent, "Set up component registers\n");
> +
> +	return 0;
> +}
> +
>  static int __init cxl_restricted_host_probe(struct platform_device *pdev)
>  {
>  	struct pci_host_bridge *host = NULL;
>  	struct acpi_device *adev;
>  	unsigned long long uid = ~0;
>  	resource_size_t rcrb;
> +	resource_size_t component_reg_phys;
Trivial: As before, if we can reduce scope to inside the while loop, slightly cleaner
and more local.
> +	int rc;
>  
>  	while ((host = cxl_find_next_rch(host)) != NULL) {
>  		adev = ACPI_COMPANION(&host->dev);
> @@ -425,10 +497,18 @@ static int __init cxl_restricted_host_probe(struct platform_device *pdev)
>  
>  		dev_dbg(&host->dev, "RCRB found: 0x%08llx\n", (u64)rcrb);
>  
> +		component_reg_phys = cxl_get_component_reg_phys(rcrb);
> +		rc = cxl_setup_component_reg(&host->dev, component_reg_phys);

Perhaps rename to make it clear this is getting the DSP component registers.

Future work would be to add support for the CXL 3.0 feature that lets even an
RCD just put some of these registers in a BAR as per CXL 2.0 devices.

> +		if (rc)
> +			goto fail;

> +
>  		dev_info(&host->dev, "host supports CXL\n");
>  	}
>  
>  	return 0;
> +fail:

Better to have a more specific error message and return directly above.
Note that so far vast majority of CXL error messages are dev_dbg,
so for consistency perhaps this should be as well.
(I prefer dev_err() but not my subsystem ;)

> +	dev_err(&host->dev, "failed to initialize CXL host: %d\n", rc);
dev_err_probe() is slightly nicer to use if things can only happen in
probe() paths.

> +	return rc;
>  }
>  
>  static struct lock_class_key cxl_root_key;
Robert Richter Sept. 1, 2022, 7:38 a.m. UTC | #2
On 31.08.22 12:56:56, Jonathan Cameron wrote:
> On Wed, 31 Aug 2022 10:15:59 +0200
> Robert Richter <rrichter@amd.com> wrote:

> A few comments inline.  Mostly requests for references for things
> I couldn't find in the specs.

Most of it comes from the pci base spec (5 or 6).

> 
> > ---
> >  drivers/cxl/acpi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> > 
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 439df9df2741..88bbd2bb61fc 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -401,12 +401,84 @@ static resource_size_t cxl_get_rcrb(u32 uid)
> >  	return ctx.chbcr;
> >  }
> >  
> > +static resource_size_t cxl_get_component_reg_phys(resource_size_t rcrb)
> > +{
> > +	resource_size_t component_reg_phys;
> > +	u32 bar0, bar1;
> > +	void *addr;
> > +
> > +	/*
> > +	 * RCRB's BAR[0..1] point to component block containing CXL subsystem
> > +	 * component registers.
> > +	 * CXL 8.2.4 - Component Register Layout Definition.
> 
> For references include spec version.  Based on discussion other day,
> preference is always latest version. So r3.0 8.2.3
> is probably right. I think your references are CXL r2.0?

Right will update the comment.

> 
> 
> > +	 *
> > +	 * Also, RCRB accesses must use MMIO readl()/readq() to guarantee
> > +	 * 32/64-bit access.
> > +	 * CXL 8.2.2 - CXL 1.1 Upstream and Downstream Port Subsystem Component
> > +	 * Registers
> > +	 */
> > +	addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8);
> > +	bar0 = readl(addr + PCI_BASE_ADDRESS_0);
> > +	bar1 = readl(addr + PCI_BASE_ADDRESS_1);
> 
> The spec is a bit confusing on this, but I think you are reading into
> MEMBAR0 of the RCRB, for which there isn't a lot of description other than
> it being an address. It's referred to as a 64-bit BAR in places so you
> might be right - or it might be intended to be a bare address..
> 
> We might want a clarification on this...
> 
> Also it's a 64 bit address so we need to read it in one go. However it's
> referred to as a being a 64 bit address at 0x10 and 0x14 so who knows...

This is part of the pci base spec and clearly defined there. There are
also some similar implementation in the kernel already.

> 
> 
> > +	iounmap(addr);
> > +
> > +	/* sanity check */
> > +	if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO))
> > +		return CXL_RESOURCE_NONE;
> > +
> > +	component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK;
> > +	if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64)
> > +		component_reg_phys |= ((u64)bar1) << 32;
> > +
> > +	if (!component_reg_phys)
> > +		return CXL_RESOURCE_NONE;
> > +
> > +	/*
> > +	 * Must be 8k aligned (size of combined CXL 1.1 Downstream and
> > +	 * Upstream Port RCRBs).
> 
> The rcrb is 8k though I'm not immediately spotting an alignment requirement,
> but I'm not sure the component regs have any restrictions do... Add a reference perhaps?
> For non RCD devices there is a 64K alignment requirement, but I can't find
> anything for RCDs (might just be missing it).

This are the requirements of the pci base spec to membar ranges. The
range size is power of 2 and base must be aligned to its size.

> 
> > +	 */
> > +	if (component_reg_phys & (SZ_8K - 1))
> > +		return CXL_RESOURCE_NONE;
> > +
> > +	return component_reg_phys;
> > +}
> > +
> > +static int cxl_setup_component_reg(struct device *parent,
> > +				   resource_size_t component_reg_phys)
> > +{
> > +	struct cxl_component_reg_map comp_map;
> > +	void __iomem *base;
> > +
> > +	if (component_reg_phys == CXL_RESOURCE_NONE)
> > +		return -EINVAL;
> > +
> > +	base = ioremap(component_reg_phys, SZ_64K);
> 
> Add a spec reference for the size. Table 8-21 perhaps?

Can add a comment here.

> 
> > +	if (!base) {
> > +		dev_err(parent, "failed to map registers\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	cxl_probe_component_regs(parent, base, &comp_map);
> > +	iounmap(base);
> > +
> > +	if (!comp_map.hdm_decoder.valid) {
> > +		dev_err(parent, "HDM decoder registers not found\n");
> > +		return -ENXIO;
> 
> Hmm. HDM decoder capability is optional for RCDs - might be using the
> range registers.  Seems like we'd really want to handle that for
> RCDs.  Future work I guess.

I used the same message as for the non-RCD code path. HDM decoding is
just a subset of features handled with component regs. We need to
generalize the code for this in the future.

> 
> > +	}
> > +
> > +	dev_dbg(parent, "Set up component registers\n");
> > +
> > +	return 0;
> > +}
> > +
> >  static int __init cxl_restricted_host_probe(struct platform_device *pdev)
> >  {
> >  	struct pci_host_bridge *host = NULL;
> >  	struct acpi_device *adev;
> >  	unsigned long long uid = ~0;
> >  	resource_size_t rcrb;
> > +	resource_size_t component_reg_phys;
> Trivial: As before, if we can reduce scope to inside the while loop, slightly cleaner
> and more local.
> > +	int rc;
> >  
> >  	while ((host = cxl_find_next_rch(host)) != NULL) {
> >  		adev = ACPI_COMPANION(&host->dev);
> > @@ -425,10 +497,18 @@ static int __init cxl_restricted_host_probe(struct platform_device *pdev)
> >  
> >  		dev_dbg(&host->dev, "RCRB found: 0x%08llx\n", (u64)rcrb);
> >  
> > +		component_reg_phys = cxl_get_component_reg_phys(rcrb);
> > +		rc = cxl_setup_component_reg(&host->dev, component_reg_phys);
> 
> Perhaps rename to make it clear this is getting the DSP component registers.
> 
> Future work would be to add support for the CXL 3.0 feature that lets even an
> RCD just put some of these registers in a BAR as per CXL 2.0 devices.

Yes, this is left out atm.

> 
> > +		if (rc)
> > +			goto fail;
> 
> > +
> >  		dev_info(&host->dev, "host supports CXL\n");
> >  	}
> >  
> >  	return 0;
> > +fail:
> 
> Better to have a more specific error message and return directly above.
> Note that so far vast majority of CXL error messages are dev_dbg,
> so for consistency perhaps this should be as well.
> (I prefer dev_err() but not my subsystem ;)

There is more verbosity on the errors with dbg enabled. Note there are
only a few dev_info/dev_err messages to not polute the logs. dev_err()
is only used if something unexpected happens (e.g. the device exists
but component regs are broken).

> 
> > +	dev_err(&host->dev, "failed to initialize CXL host: %d\n", rc);
> dev_err_probe() is slightly nicer to use if things can only happen in
> probe() paths.

Will consider that.

-Robert

> 
> > +	return rc;
> >  }
> >  
> >  static struct lock_class_key cxl_root_key;
>
Jonathan Cameron Sept. 1, 2022, 11 a.m. UTC | #3
On Thu, 1 Sep 2022 09:38:13 +0200
Robert Richter <rrichter@amd.com> wrote:

> On 31.08.22 12:56:56, Jonathan Cameron wrote:
> > On Wed, 31 Aug 2022 10:15:59 +0200
> > Robert Richter <rrichter@amd.com> wrote:  
> 
> > A few comments inline.  Mostly requests for references for things
> > I couldn't find in the specs.  
> 
> Most of it comes from the pci base spec (5 or 6).

Ok. Extra references appreciated - these specs are huge, so saving
searching time always good!

> 
> > 
> >   
> > > +	 *
> > > +	 * Also, RCRB accesses must use MMIO readl()/readq() to guarantee
> > > +	 * 32/64-bit access.
> > > +	 * CXL 8.2.2 - CXL 1.1 Upstream and Downstream Port Subsystem Component
> > > +	 * Registers
> > > +	 */
> > > +	addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8);
> > > +	bar0 = readl(addr + PCI_BASE_ADDRESS_0);
> > > +	bar1 = readl(addr + PCI_BASE_ADDRESS_1);  
> > 
> > The spec is a bit confusing on this, but I think you are reading into
> > MEMBAR0 of the RCRB, for which there isn't a lot of description other than
> > it being an address. It's referred to as a 64-bit BAR in places so you
> > might be right - or it might be intended to be a bare address..
> > 
> > We might want a clarification on this...
> > 
> > Also it's a 64 bit address so we need to read it in one go. However it's
> > referred to as a being a 64 bit address at 0x10 and 0x14 so who knows...  
> 
> This is part of the pci base spec and clearly defined there. There are
> also some similar implementation in the kernel already.

There isn't a cross reference from CXL spec and PCI doesn't use
the term membar.

I guess it is fairly obvious though that it's an abbreviation
of Base Address Register for Memory.  I might raise the wish to tidy that
up for a future spec revision.


> 
> > 
> >   
> > > +	iounmap(addr);
> > > +
> > > +	/* sanity check */
> > > +	if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO))
> > > +		return CXL_RESOURCE_NONE;
> > > +
> > > +	component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK;
> > > +	if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64)
> > > +		component_reg_phys |= ((u64)bar1) << 32;
> > > +
> > > +	if (!component_reg_phys)
> > > +		return CXL_RESOURCE_NONE;
> > > +
> > > +	/*
> > > +	 * Must be 8k aligned (size of combined CXL 1.1 Downstream and
> > > +	 * Upstream Port RCRBs).  
> > 
> > The rcrb is 8k though I'm not immediately spotting an alignment requirement,
> > but I'm not sure the component regs have any restrictions do... Add a reference perhaps?
> > For non RCD devices there is a 64K alignment requirement, but I can't find
> > anything for RCDs (might just be missing it).  
> 
> This are the requirements of the pci base spec to membar ranges. The
> range size is power of 2 and base must be aligned to its size.

Ok.  It feels convoluted to rely on the CXL glossary entry for BAR
to cover MEMBAR0 and hence inherit the restrictions of a PCIe bar.

Maybe just add a comment here so that anyone who hits this can understand
the source of the restriction seeing as it isn't in the CXL spec and this
isn't a PCI BAR.

> 
> >   
> > > +	 */
> > > +	if (component_reg_phys & (SZ_8K - 1))
> > > +		return CXL_RESOURCE_NONE;
> > > +
> > > +	return component_reg_phys;
> > > +}
> > > +



> >   
> > > +	if (!base) {
> > > +		dev_err(parent, "failed to map registers\n");
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > > +	cxl_probe_component_regs(parent, base, &comp_map);
> > > +	iounmap(base);
> > > +
> > > +	if (!comp_map.hdm_decoder.valid) {
> > > +		dev_err(parent, "HDM decoder registers not found\n");
> > > +		return -ENXIO;  
> > 
> > Hmm. HDM decoder capability is optional for RCDs - might be using the
> > range registers.  Seems like we'd really want to handle that for
> > RCDs.  Future work I guess.  
> 
> I used the same message as for the non-RCD code path. HDM decoding is
> just a subset of features handled with component regs. We need to
> generalize the code for this in the future.

Sure - much more likely to need that generalized code for an RCD.
IIRC a CXL 2.0 device must implement HDM decoders, even though the
other path can be used by software that doesn't understand CXL 2.0.
Our RCD might be because the device is CXL 1.1...



> 
> >   
> > > +		if (rc)
> > > +			goto fail;  
> >   
> > > +
> > >  		dev_info(&host->dev, "host supports CXL\n");
> > >  	}
> > >  
> > >  	return 0;
> > > +fail:  
> > 
> > Better to have a more specific error message and return directly above.
> > Note that so far vast majority of CXL error messages are dev_dbg,
> > so for consistency perhaps this should be as well.
> > (I prefer dev_err() but not my subsystem ;)  
> 
> There is more verbosity on the errors with dbg enabled. Note there are
> only a few dev_info/dev_err messages to not polute the logs. dev_err()
> is only used if something unexpected happens (e.g. the device exists
> but component regs are broken).
> 
Ok. I'll leave the question of balance between the two for CXL maintainers
to comment on if they wish.

Thanks,

Jonathan
Robert Richter Sept. 6, 2022, 11:32 a.m. UTC | #4
On 01.09.22 12:00:03, Jonathan Cameron wrote:
> On Thu, 1 Sep 2022 09:38:13 +0200
> Robert Richter <rrichter@amd.com> wrote:
> 
> > On 31.08.22 12:56:56, Jonathan Cameron wrote:
> > > On Wed, 31 Aug 2022 10:15:59 +0200
> > > Robert Richter <rrichter@amd.com> wrote:  
> > 
> > > A few comments inline.  Mostly requests for references for things
> > > I couldn't find in the specs.  
> > 
> > Most of it comes from the pci base spec (5 or 6).
> 
> Ok. Extra references appreciated - these specs are huge, so saving
> searching time always good!

I will add comments for all ther refs in this patch.

[...]

> > > > +	cxl_probe_component_regs(parent, base, &comp_map);
> > > > +	iounmap(base);
> > > > +
> > > > +	if (!comp_map.hdm_decoder.valid) {
> > > > +		dev_err(parent, "HDM decoder registers not found\n");
> > > > +		return -ENXIO;  
> > > 
> > > Hmm. HDM decoder capability is optional for RCDs - might be using the
> > > range registers.  Seems like we'd really want to handle that for
> > > RCDs.  Future work I guess.  
> > 
> > I used the same message as for the non-RCD code path. HDM decoding is
> > just a subset of features handled with component regs. We need to
> > generalize the code for this in the future.
> 
> Sure - much more likely to need that generalized code for an RCD.
> IIRC a CXL 2.0 device must implement HDM decoders, even though the
> other path can be used by software that doesn't understand CXL 2.0.
> Our RCD might be because the device is CXL 1.1...

Yes, this will be a follow on series.

Thanks,

-Robert
Jonathan Zhang Sept. 8, 2022, 8:59 p.m. UTC | #5
> On Aug 31, 2022, at 1:15 AM, Robert Richter <rrichter@amd.com> wrote:
> 
> A downstream port must be connected to a component register block.
> Determine its base address from the RCRB.
> 
> The implementation is analog to how cxl_setup_regs() is implemented
> for CXL VH mode. A struct cxl_component_reg_map is filled in, mapped
> and probed.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
> drivers/cxl/acpi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 439df9df2741..88bbd2bb61fc 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -401,12 +401,84 @@ static resource_size_t cxl_get_rcrb(u32 uid)
> 	return ctx.chbcr;
> }
> 
> +static resource_size_t cxl_get_component_reg_phys(resource_size_t rcrb)
> +{
> +	resource_size_t component_reg_phys;
> +	u32 bar0, bar1;
> +	void *addr;
> +
> +	/*
> +	 * RCRB's BAR[0..1] point to component block containing CXL subsystem
> +	 * component registers.
> +	 * CXL 8.2.4 - Component Register Layout Definition.
> +	 *
> +	 * Also, RCRB accesses must use MMIO readl()/readq() to guarantee
> +	 * 32/64-bit access.
> +	 * CXL 8.2.2 - CXL 1.1 Upstream and Downstream Port Subsystem Component
> +	 * Registers
> +	 */
> +	addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8);
> +	bar0 = readl(addr + PCI_BASE_ADDRESS_0);
> +	bar1 = readl(addr + PCI_BASE_ADDRESS_1);
> +	iounmap(addr);
> +
> +	/* sanity check */
> +	if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO))
> +		return CXL_RESOURCE_NONE;
> +
> +	component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK;
> +	if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64)
> +		component_reg_phys |= ((u64)bar1) << 32;
> +
> +	if (!component_reg_phys)
> +		return CXL_RESOURCE_NONE;
> +
> +	/*
> +	 * Must be 8k aligned (size of combined CXL 1.1 Downstream and
> +	 * Upstream Port RCRBs).
> +	 */
> +	if (component_reg_phys & (SZ_8K - 1))
> +		return CXL_RESOURCE_NONE;
> +
> +	return component_reg_phys;
> +}
> +
> +static int cxl_setup_component_reg(struct device *parent,
> +				   resource_size_t component_reg_phys)
> +{
> +	struct cxl_component_reg_map comp_map;
> +	void __iomem *base;
> +
> +	if (component_reg_phys == CXL_RESOURCE_NONE)
> +		return -EINVAL;
> +
> +	base = ioremap(component_reg_phys, SZ_64K);
> +	if (!base) {
> +		dev_err(parent, "failed to map registers\n");
> +		return -ENOMEM;
> +	}
> +
> +	cxl_probe_component_regs(parent, base, &comp_map);
> +	iounmap(base);
> +
> +	if (!comp_map.hdm_decoder.valid) {
> +		dev_err(parent, "HDM decoder registers not found\n");
> +		return -ENXIO;
> +	}
> +
> +	dev_dbg(parent, "Set up component registers\n");
> +
> +	return 0;
> +}
> +
> static int __init cxl_restricted_host_probe(struct platform_device *pdev)
> {
> 	struct pci_host_bridge *host = NULL;
> 	struct acpi_device *adev;
> 	unsigned long long uid = ~0;
> 	resource_size_t rcrb;
> +	resource_size_t component_reg_phys;
> +	int rc;
> 
> 	while ((host = cxl_find_next_rch(host)) != NULL) {
> 		adev = ACPI_COMPANION(&host->dev);
> @@ -425,10 +497,18 @@ static int __init cxl_restricted_host_probe(struct platform_device *pdev)
> 
> 		dev_dbg(&host->dev, "RCRB found: 0x%08llx\n", (u64)rcrb);
> 
> +		component_reg_phys = cxl_get_component_reg_phys(rcrb);
> +		rc = cxl_setup_component_reg(&host->dev, component_reg_phys);

cxl_setup_component_reg() calls cxl_probe_component_regs() which depends on the existence
of HDM decoder capability register. Such register does not exist when the device is in RCRB
mode, attached to a RCH.

> +		if (rc)
> +			goto fail;
> +
> 		dev_info(&host->dev, "host supports CXL\n");
> 	}
> 
> 	return 0;
> +fail:
> +	dev_err(&host->dev, "failed to initialize CXL host: %d\n", rc);
> +	return rc;
> }
> 
> static struct lock_class_key cxl_root_key;
> -- 
> 2.30.2
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 439df9df2741..88bbd2bb61fc 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -401,12 +401,84 @@  static resource_size_t cxl_get_rcrb(u32 uid)
 	return ctx.chbcr;
 }
 
+static resource_size_t cxl_get_component_reg_phys(resource_size_t rcrb)
+{
+	resource_size_t component_reg_phys;
+	u32 bar0, bar1;
+	void *addr;
+
+	/*
+	 * RCRB's BAR[0..1] point to component block containing CXL subsystem
+	 * component registers.
+	 * CXL 8.2.4 - Component Register Layout Definition.
+	 *
+	 * Also, RCRB accesses must use MMIO readl()/readq() to guarantee
+	 * 32/64-bit access.
+	 * CXL 8.2.2 - CXL 1.1 Upstream and Downstream Port Subsystem Component
+	 * Registers
+	 */
+	addr = ioremap(rcrb, PCI_BASE_ADDRESS_0 + SZ_8);
+	bar0 = readl(addr + PCI_BASE_ADDRESS_0);
+	bar1 = readl(addr + PCI_BASE_ADDRESS_1);
+	iounmap(addr);
+
+	/* sanity check */
+	if (bar0 & (PCI_BASE_ADDRESS_MEM_TYPE_1M | PCI_BASE_ADDRESS_SPACE_IO))
+		return CXL_RESOURCE_NONE;
+
+	component_reg_phys = bar0 & PCI_BASE_ADDRESS_MEM_MASK;
+	if (bar0 & PCI_BASE_ADDRESS_MEM_TYPE_64)
+		component_reg_phys |= ((u64)bar1) << 32;
+
+	if (!component_reg_phys)
+		return CXL_RESOURCE_NONE;
+
+	/*
+	 * Must be 8k aligned (size of combined CXL 1.1 Downstream and
+	 * Upstream Port RCRBs).
+	 */
+	if (component_reg_phys & (SZ_8K - 1))
+		return CXL_RESOURCE_NONE;
+
+	return component_reg_phys;
+}
+
+static int cxl_setup_component_reg(struct device *parent,
+				   resource_size_t component_reg_phys)
+{
+	struct cxl_component_reg_map comp_map;
+	void __iomem *base;
+
+	if (component_reg_phys == CXL_RESOURCE_NONE)
+		return -EINVAL;
+
+	base = ioremap(component_reg_phys, SZ_64K);
+	if (!base) {
+		dev_err(parent, "failed to map registers\n");
+		return -ENOMEM;
+	}
+
+	cxl_probe_component_regs(parent, base, &comp_map);
+	iounmap(base);
+
+	if (!comp_map.hdm_decoder.valid) {
+		dev_err(parent, "HDM decoder registers not found\n");
+		return -ENXIO;
+	}
+
+	dev_dbg(parent, "Set up component registers\n");
+
+	return 0;
+}
+
 static int __init cxl_restricted_host_probe(struct platform_device *pdev)
 {
 	struct pci_host_bridge *host = NULL;
 	struct acpi_device *adev;
 	unsigned long long uid = ~0;
 	resource_size_t rcrb;
+	resource_size_t component_reg_phys;
+	int rc;
 
 	while ((host = cxl_find_next_rch(host)) != NULL) {
 		adev = ACPI_COMPANION(&host->dev);
@@ -425,10 +497,18 @@  static int __init cxl_restricted_host_probe(struct platform_device *pdev)
 
 		dev_dbg(&host->dev, "RCRB found: 0x%08llx\n", (u64)rcrb);
 
+		component_reg_phys = cxl_get_component_reg_phys(rcrb);
+		rc = cxl_setup_component_reg(&host->dev, component_reg_phys);
+		if (rc)
+			goto fail;
+
 		dev_info(&host->dev, "host supports CXL\n");
 	}
 
 	return 0;
+fail:
+	dev_err(&host->dev, "failed to initialize CXL host: %d\n", rc);
+	return rc;
 }
 
 static struct lock_class_key cxl_root_key;