Linux CXL
 help / color / mirror / Atom feed
From: Terry Bowman <Terry.Bowman@amd.com>
To: Dan Williams <dan.j.williams@intel.com>,
	alison.schofield@intel.com, vishal.l.verma@intel.com,
	ira.weiny@intel.com, bwidawsk@kernel.org, dave.jiang@intel.com,
	Jonathan.Cameron@huawei.com, linux-cxl@vger.kernel.org
Cc: rrichter@amd.com, linux-kernel@vger.kernel.org, bhelgaas@google.com
Subject: Re: [PATCH v10 12/15] cxl/pci: Disable root port interrupts in RCH mode
Date: Tue, 19 Sep 2023 10:08:10 -0500	[thread overview]
Message-ID: <f27f685c-6b53-8955-735f-e9c0c04354a5@amd.com> (raw)
In-Reply-To: <6504a5e23cee9_d7cc8294b7@dwillia2-xfh.jf.intel.com.notmuch>

Hi Dan,

I added comments below.

On 9/15/23 13:43, Dan Williams wrote:
> Terry Bowman wrote:
>> The RCH root port contains root command AER registers that should not be
>> enabled.[1] Disable these to prevent root port interrupts.
>>
>> [1] CXL 3.0 - 12.2.1.1 RCH Downstream Port-detected Errors
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Signed-off-by: Robert Richter <rrichter@amd.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> [..]
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 2a22a7ed4704..d195af72ed65 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1042,6 +1042,9 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>>  
>>  	cxl_dport_map_regs(dport);
>>  
>> +	if (dport->rch)
>> +		cxl_disable_rch_root_ints(dport);
>> +
> 
> Similar to the comment about cxl_dport_map_regs() not being appropriate
> in an enumeration routine, this also needs to move out of _add_dport. It
> occurs to me that it should also be undone on driver detach just like
> other device "enables".

Ok. I will move out of enumeration. 

Per the 'undo' request: This is a RCH downstream port (dport) with PCIe root port 
capability. PCI spec states root port error reporting is disabled by default at 
powerup. And SW does *not* enable the root port errors because the RCH dport is *not* 
bound to a root port driver (missing BDF, etc). This mask is added to follow the 
CXL spec precisely and if the rest of the system behaves as expected should not 
be necessary. 

I don't believe masking should be 'undone' in driver detach or elsewhere. Adding 
the 'undo' masking would potentially introduce RCH dport root port interrupt 
reporting which is incorrect for the RCH/RCD mode. Only CXL components (device, 
uport, switch) may reside under the RCH dport and never want RCH dport reporting 
root port errors. RCEC reports the root complex errors in RCH/RCD mode.

Regards,
Terry


  reply	other threads:[~2023-09-19 15:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31 17:02 [PATCH v10 08/15] PCI/AER: Refactor cper_print_aer() for use by CXL driver module Terry Bowman
2023-08-31 17:02 ` [PATCH v10 09/15] cxl/pci: Update CXL error logging to use RAS register address Terry Bowman
2023-08-31 17:02 ` [PATCH v10 10/15] cxl/pci: Map RCH downstream AER registers for logging protocol errors Terry Bowman
2023-09-15  0:27   ` Dan Williams
2023-08-31 17:02 ` [PATCH v10 11/15] cxl/pci: Add RCH downstream port error logging Terry Bowman
2023-08-31 17:02 ` [PATCH v10 12/15] cxl/pci: Disable root port interrupts in RCH mode Terry Bowman
2023-09-15 18:43   ` Dan Williams
2023-09-19 15:08     ` Terry Bowman [this message]
2023-09-26 21:47       ` Dan Williams
2023-08-31 17:02 ` [PATCH v10 13/15] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler Terry Bowman
2023-08-31 20:35   ` Dan Williams
2023-09-19 20:58     ` Terry Bowman
2023-09-20 15:28       ` Terry Bowman
2023-09-26 23:58       ` Dan Williams
2023-08-31 17:02 ` [PATCH v10 14/15] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling Terry Bowman
2023-08-31 17:02 ` [PATCH v10 15/15] cxl/core/regs: Rename phys_addr in cxl_map_component_regs() Terry Bowman

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=f27f685c-6b53-8955-735f-e9c0c04354a5@amd.com \
    --to=terry.bowman@amd.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=bhelgaas@google.com \
    --cc=bwidawsk@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rrichter@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox