On Jan 4, 2022, at 5:14 PM, Greg Favor <gfavor@...> wrote:

On Tue, Jan 4, 2022 at 11:16 AM Mayuresh Chitale <mchitale@...> wrote:

On Thu, Dec 30, 2021 at 3:37 AM Michael Klinglesmith <michael.klinglesmith@...> wrote:

2) section 4.7.3.4: A discussion of No_snoop must also include Relaxed Order (RO).  When a root port forwards a mix of traffic with NS=0, RO=0 and NS=1 , RO=0.  The requirement to enforce ordering tends to lead to ignoring the NS hint and snooping the trarffic anyway.

Proposed text for 4.7.3.4:

A system is required to provide hardware managed coherence for PCIe traffic. A system may ignore the No_snoop hint bit and treat all PCIe traffic as HW coherent.  A system may choose to honor the No_snoop (NS) hint bit on incoming PCIe transactions.  In this case software must manage coherence for the memory used by the device issuing the transactions.  Such systems must also honor the relaxed order (RO) hint bit.  To take full advantage of SW coherence the device should set both NS and RO to 1.  Otherwise, when RO=0 the snooped and non-snooped traffic arriving through a host bridge must be kept ordered, eliminating the benefit of routing some traffic around the cache hierarchy to memory.

 

AFAIK, these are two separate attributes and a requester could set either or both of them.  I am not sure if we can link the two together. Please correct me if I am wrong but does it mean that ,for e.g, all the mem write requests which may have the NS attribute set must also have the RO attribute set?

Fwiw, through seven generations of ARM SBSA and then the BSA, they talk about No_snoop in a similar way to what we do, but make no corresponding mentions about Relaxed Order (other than mentions of Relaxed Order and No Snoop enable/disable control bits in Device Control Registers).  In essence they don't mandate or even suggest any particular connections between the two features.

So it wouldn't be unreasonable for us to not say much and essentially leave complete flexibility to the system designer.  Or we could include some non-normative commentary or suggestions.

Greg

On Tue, Jan 4, 2022 at 11:16 AM Mayuresh Chitale <mchitale@...> wrote:

On Thu, Dec 30, 2021 at 3:37 AM Michael Klinglesmith <michael.klinglesmith@...> wrote:

Hello,

I’m new to participating in the platform WG.  I’m working at SiFive now.  I spent the last 20 years doing PCIe compliant IO fabrics for Intel chipsets.

I have a few comments / questions about the PCIe:

1) section 4.7.3.3: Why are we requiring INTx?  We should allow a modern system to be built without the legacy INTx requirements.

The intention behind requiring intx was to ensure better compatibility. There are few cases where we fallback to INTx if MSI is not available. For e.g:

https://elixir.bootlin.com/linux/v5.15.12/source/drivers/pci/pcie/portdrv_core.c#L165. 

But if such cases are considered to be rare and not required to be supported then this requirement can be removed.

2) section 4.7.3.4: A discussion of No_snoop must also include Relaxed Order (RO).  When a root port forwards a mix of traffic with NS=0, RO=0 and NS=1 , RO=0.  The requirement to enforce ordering tends to lead to ignoring the NS hint and snooping the trarffic anyway.

Proposed text for 4.7.3.4:

A system is required to provide hardware managed coherence for PCIe traffic. A system may ignore the No_snoop hint bit and treat all PCIe traffic as HW coherent.  A system may choose to honor the No_snoop (NS) hint bit on incoming PCIe transactions.  In this case software must manage coherence for the memory used by the device issuing the transactions.  Such systems must also honor the relaxed order (RO) hint bit.  To take full advantage of SW coherence the device should set both NS and RO to 1.  Otherwise, when RO=0 the snooped and non-snooped traffic arriving through a host bridge must be kept ordered, eliminating the benefit of routing some traffic around the cache hierarchy to memory.

 

AFAIK, these are two separate attributes and a requester could set either or both of them.  I am not sure if we can link the two together. Please correct me if I am wrong but does it mean that ,for e.g, all the mem write requests which may have the NS attribute set must also have the RO attribute set?

 

2) section 4.7.3.5: Why do we exclude the possibility of a Host bridge that supports both RCiEP and Root Ports?

Actually the requirement is to support atleast one of them ie RCiEP (+RCEC) or Root port but an RC can certainly  support both.

Cheers,

Michael.