Re: [PATCH v4 1/1] server extension: PCIe requirements


Mayuresh Chitale
 




On Thu, Jul 15, 2021 at 11:55 AM Atish Patra <Atish.Patra@...> wrote:
On Mon, 2021-07-12 at 23:19 +0530, Mayuresh Chitale wrote:
> This patch adds requirements for PCIe support for the server extension
>
> Signed-off-by: Mayuresh Chitale <mchitale@...>
> ---
>  Makefile                 |  18 +++--
>  pcie-topology.ditaa      |  28 ++++++++
>  riscv-platform-spec.adoc | 147 ++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 185 insertions(+), 8 deletions(-)
>  create mode 100644 pcie-topology.ditaa
>
> diff --git a/Makefile b/Makefile
> index de5e0b0..06796f3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3,13 +3,19 @@
>  #
>  
>  ASCIIDOCTOR = asciidoctor
> +DITAA = ditaa
> +IMAGES = pcie-topology.png
>  PLATFORM_SPEC = riscv-platform-spec
>  PANDOC = pandoc
>  PARTS = changelog.adoc contributors.adoc introduction.adoc
> licensing.adoc \
>         profiles.adoc supervisor-level.adoc user-level.adoc
>  
>  # Build the platform spec in several formats
> -all: $(PLATFORM_SPEC).md $(PLATFORM_SPEC).pdf $(PLATFORM_SPEC).html
> +all: $(IMAGES) $(PLATFORM_SPEC).md $(PLATFORM_SPEC).pdf
> $(PLATFORM_SPEC).html
> +
> +%.png: %.ditaa
> +       rm -f $@
> +       $(DITAA) $<
>  
>  $(PLATFORM_SPEC).md: $(PLATFORM_SPEC).xml
>         $(PANDOC) -f docbook -t markdown_strict $< -o $@
> @@ -17,10 +23,10 @@ $(PLATFORM_SPEC).md: $(PLATFORM_SPEC).xml
>  $(PLATFORM_SPEC).xml: $(PLATFORM_SPEC).adoc
>         $(ASCIIDOCTOR) -d book -b docbook $<
>  
> -$(PLATFORM_SPEC).pdf: $(PLATFORM_SPEC).adoc
> +$(PLATFORM_SPEC).pdf: $(PLATFORM_SPEC).adoc $(IMAGES)
>         $(ASCIIDOCTOR) -d book -r asciidoctor-pdf -b pdf $<
>  
> -$(PLATFORM_SPEC).html: $(PLATFORM_SPEC).adoc
> +$(PLATFORM_SPEC).html: $(PLATFORM_SPEC).adoc $(IMAGES)
>         $(ASCIIDOCTOR) -d book -b html $<
>  
>  $(PLATFORM_SPEC).adoc: $(PARTS)
> @@ -31,11 +37,11 @@ clean:
>         rm -f $(PLATFORM_SPEC).md
>         rm -f $(PLATFORM_SPEC).pdf
>         rm -f $(PLATFORM_SPEC).html
> +       rm -f $(IMAGES)
>  
>  # handy shortcuts for installing necessary packages: YMMV
>  install-debs:
> -       sudo apt-get install pandoc asciidoctor ruby-asciidoctor-pdf
> +       sudo apt-get install pandoc asciidoctor ditaa ruby-asciidoctor-
> pdf
>  
>  install-rpms:
> -       sudo dnf install pandoc rubygem-asciidoctor rubygem-
> asciidoctor-pdf
> -
> +       sudo dnf install ditaa pandoc rubygem-asciidoctor rubygem-
> asciidoctor-pdf
> diff --git a/pcie-topology.ditaa b/pcie-topology.ditaa
> new file mode 100644
> index 0000000..7180035
> --- /dev/null
> +++ b/pcie-topology.ditaa
> @@ -0,0 +1,28 @@
> +            +----------+                             +----------+
> +            |   CPU    |                             |   CPU    |
> +            +-----+----+                             +-----+----+
> +                  |                                        |
> +                  |                                        |
> +                  |                                        |
> +    +-------------+------------+             +-------------+----------
> --+
> +    |                          |            
> |                          |
> +    |        Root Complex      |             |        Root
> Complex      |
> +    |                          |            
> |                          |
> +    |      +--------------+    |             |      +--------------
> +    |
> +    |      |  Host Bridge |    |             |      |  Host Bridge
> |    |
> +    |      +------+-------+    |             |      +------+-------
> +    |
> +    |             |            |             |            
> |            |
> +    |             |            |             |             | Bus
> 0      |
> +    |     +-------+------+     |             |       +-----+-------
> +    |
> +    |     |     Bus 0    |     |             |       |  Root Port 
> |    |
> +    |     |              |     |             |       +-----+-------
> +    |
> +    | +---+---+      +---+---+ |             |            
> |            |
> +    | | RCiEP |      | RCEC  | |             |             | PCIe
> Link  |
> +    | +-------+      +-------+ |             |            
> |            |
> +    |                          |             |       Bus 1
> |            |
> +    |                          |             |            
> |            |
> +    +--------------------------+             +------------------------
> --+
> +
> +
> +    RCiEP : Root complex integrated endpoint
> +    RCEC  : Root complex event collector
> diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
> index 4418788..b2949dc 100644
> --- a/riscv-platform-spec.adoc
> +++ b/riscv-platform-spec.adoc
> @@ -47,7 +47,20 @@ include::profiles.adoc[]
>  |RVA22     | RISC-V Application 2022
>  |EE        | Execution Environment
>  |RV32GC    | RISC-V 32-bit general purpose ISA described as
> RV32IMAFDC.
> -|RV64GC    | RISC-V 64-bit general purpose ISA described as
> RV64IMAFDC.     
> +|RV64GC    | RISC-V 64-bit general purpose ISA described as
> RV64IMAFDC.
> +|PCIe      | PCI Express
> +|ECAM      | Enhanced Configuration Access Mechanism
> +|BAR       | Base Address Register
> +|AER       | Advanced Error Reporting
> +|CRS       | Configuration Request Retry Status
> +|TLP       | Transaction Layer Packet
> +|RCiEP     | Root Complex Integrated Endpoint
> +|RCEC      | Root Complex Event Collector
> +|PME       | Power Management Event
> +|MSI       | Message Signaled Interrupts
> +|MSI-X     | Enhanced Message Signaled Interrupts
> +|INTx      | PCIe Legacy Interrupts
> +|PMA       | Physical Memory Attributes
>  |===
>  
>  === Specifications
> @@ -363,7 +376,137 @@
> https://lists.riscv.org/g/tech-privileged/message/404[Sstc] extension.
>  ** Platforms are required to delegate the supervisor timer interrupt
> to 'S'
>  mode. If the 'H' extension is implemented then the platforms are
> required to
>  delegate the virtual supervisor timer interrupt to 'VS' mode.
> -* PCI-E
> +
> +===== PCIe
> +Platforms are required to support at least PCIe Base Specification
> Revision 1.1
> +footnote:[https://pcisig.com/specifications].
> +
> +====== PCIe Config Space
> +* Platforms shall support access to the PCIe config space via ECAM as
> described
> +in the PCIe Base specification.
> +* The entire config space for a single PCIe domain should be
> accessible via a
> +single ECAM I/O region.
> +* Platform firmware should implement the MCFG table as listed in the
> ACPI System
> +Description Tables above to allow the operating systems to discover
> the supported
> +PCIe domains and map the ECAM I/O region for each domain.
> +* Platform software shall configure ECAM I/O regions such that the
> effective
> +memory attributes are that of a PMA I/O region (i.e. strongly-ordered,
> +non-cacheable, non-idempotent).
> +
> +====== PCIe Memory Space
> +Platforms are required to map PCIe address space directly in the
> system address
> +space and not have any address translation for outbound accesses from
> harts or
> +for inbound accesses to any component in the system address space
> +
> +* PCIe Outbound Memory +
> +PCIe devices and bridges/switches frequently implement BARs which only
> support
> +32-bit addressing or support 64 bit addressing but do not support
> prefetchable
> +memory. To support mapping of such BARs, platforms are required to
> reserve
> +some space below 4G for each root port present in the system.
> +
> +[sidebar]
> +--
> +[underline]*_Implementation Note_* +
> +Platform software would likely configure these per root port regions
> such that
> +their effective memory attributes are that of a PMA I/O region (i.e.
> +strongly-ordered, non-cacheable, non-idempotent). Platforms would
> likely also
> +reserve some space above 4G to map BARs that support 64 bit addressing
> and
> +prefetchable memory which could be configured by the platform software
> as either
> +I/O or memory.
> +--
> +
> +* PCIe Inbound Memory +
> +For security reasons, platforms must provide a mechanism controlled by
> M-mode
> +software to restrict inbound PCIe accesses from accessing regions of
> address
> +space intended to be accessible only to M-mode software.
> +
> +[sidebar]
> +--
> +[underline]*_Implementation Note_* +
> +Such an access control mechanism could be analogous to the per-hart
> PMP
> +as described in the RISC-V Privileged Architectures specification.
> +--
> +
> +====== PCIe Interrupts
> +* Platforms shall support both INTx and MSI/MSI-x interrupts.
> +* Integration with AIA +
> +TBD
> +
> +====== PCIe cache coherency
> +PCIe transactions that are marked with a No_Snoop bit of zero and
> access memory
> +that is cacheable by harts, as well as accesses to memory that is
> noncacheable
> +by harts, are I/O Coherent and no software coherency management is
> needed. In
> +contrast, PCIe transactions that are marked with a No_Snoop bit of one
> and
> +access memory that is cacheable by harts, must have coherency managed
> by
> +software.
> +
> +====== PCIe Topology
> +Platforms are required to implement at least one of the following
> topologies
> +and the components required in that topology.
> +
> +[#fig_intro1]
> +.PCIe Topologies
> +image::pcie-topology.png[width=524,height=218]
> +
> +* Host Bridge +
> +Following are the requirements for host bridges:
> +
> +** Any read or write access by a hart to an ECAM I/O region shall be
> converted
> +by the host bridge into the corresponding PCIe config read or config
> write
> +request.
> +** Any read or write access by a hart to a PCIe outbound region shall
> be
> +forwarded by the host bridge to a BAR or prefetch/non-prefetch memory
> window,
> +if the address falls within the region claimed by the BAR or prefetch/
> +non-prefetch memory window. Otherwise the host bridge shall return an
> error.
> +
> +** Host bridge shall return all 1s in the following cases:
> +*** Config read to non existent functions and devices on root bus.
> +*** Config reads that receive Unsupported Request response from
> functions and
> +devices on the root bus.
> +* Root ports +
> +Following are the requirements for root ports.
> +** Root ports shall appear as PCI-PCI bridge to software.
> +** Root ports shall implement all registers of Type 1 header.
> +** Root ports shall implement all capabilities specified in the PCIe
> Base
> +specification for a root port.
> +** Root ports shall forward type 1 configuration access when the bus
> number in
> +the TLP is greater than the root port's secondary bus number and less
> than or
> +equal to the root port's subordinate bus number.
> +** Root ports shall convert type 1 configuration access to a type 0
> +configuration access when bus number in the TLP is equal to the root
> port's
> +secondary bus number.
> +** Root ports shall respond to any type 0 configuration accesses it
> receives.
> +** Root ports shall forward memory accesses targeting its
> prefetch/non-prefetch
> +memory windows to downstream components. If address of the transaction
> does not
> +fall within the regions claimed by prefetch/non-prefetch memory
> windows then
> +the root port shall generate a Unsupported Request.
> +** Root port requester id or completer id shall be formed using the
> bdf of the
> +root port.
> +** The root ports shall support the CRS software visibility.
> +** The root port shall implement the AER capability.
> +** Root ports shall return all 1s in the following cases:
> +*** Config read to non existent functions and devices on secondary
> bus.
> +*** Config reads that receive Unsupported Request from downstream
> components.
> +*** Config read when root port's link is down.
> +
> +* RCiEP +
> +All the requirements for RCiEP in the PCIe Base specification shall be
> +implemented.
> +In addition the following requirements shall be met:
> +** If RCiEP is implemented then RCEC shall be implemented as well. All
> +requirements for RCEC specified in the PCIe Base specification shall
> be
> +implemented. RCEC is required to terminate the AER and PME messages
> from RCiEP.
> +** If both the topologies mentioned above are supported then RCiEP and
> RCEC
> +shall be implemented in a separate PCIe domain and shall be
> addressable via a
> +separate ECAM I/O region.
> +
> +===== PCIe Device Firmware Requirement
> +PCI expansion ROM code type 3 (UEFI) image must be provided by PCIe
> device for
> +OS/A server extension platform according to
> +https://pcisig.com/specifications/conventional/pci_firmware[PCI Firmwa
> re Specification Revision 3.3]
> +if that PCIe device is utilized during UEFI firmware boot process. The
> image
> +stored in PCI expansion ROM is an UEFI driver that must be compliant
> with
> +https://uefi.org/specifications[UEFI specification 2.9] 14.4.2 PCI
> Option ROMs.
>  
>  ==== Secure Boot
>  * TEE

Reviewed-by: Atish Patra <atish.patra@...>

Thanks Atish. Actually one more revision is required for this patch. 
During an internal review, a modification was suggested to me for the PCIe cache coherency requirement text. 
I will do that and send a v5 shortly. Please check that as well.
 
--
Regards,
Atish

Join {tech-unixplatformspec@lists.riscv.org to automatically receive all group messages.