linux-kernel - Re: [PATCH v10 12/15] cxl/pci: Disable root port interrupts in RCH mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f27f685c-6b53-8955-735f-e9c0c04354a5@amd.com>
Date:   Tue, 19 Sep 2023 10:08:10 -0500
From:   Terry Bowman <Terry.Bowman@....com>
To:     Dan Williams <dan.j.williams@...el.com>,
        alison.schofield@...el.com, vishal.l.verma@...el.com,
        ira.weiny@...el.com, bwidawsk@...nel.org, dave.jiang@...el.com,
        Jonathan.Cameron@...wei.com, linux-cxl@...r.kernel.org
Cc:     rrichter@....com, linux-kernel@...r.kernel.org, bhelgaas@...gle.com
Subject: Re: [PATCH v10 12/15] cxl/pci: Disable root port interrupts in RCH
 mode

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@....com>
>> Signed-off-by: Robert Richter <rrichter@....com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
>> Reviewed-by: Dave Jiang <dave.jiang@...el.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