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

Greg Favor

On Sat, Jul 3, 2021 at 8:03 AM Atish Patra <atish.patra@...> wrote:
> +* Platform software shall configure ECAM I/O regions such that the
> effective
> +memory type (PMA + PBMT) is UC.

UC is not present in abbreviation section. Moreover, PMA + PBMT seems
bit ambiguous to me. It wouldn't hurt to be more verbose here.

The proposed RISC-V name for this memory type is "IO", but it is up in the air for the moment as to whether the the memory types supported by Svpbmt will have acronym names (i.e. IO and NC), or just use thier longer descriptive names, e.g. Non-cacheable, non-idempotent, strongly-ordered I/O memory for "IO".

Overall it is probably better to make the above statement in a somewhat more generic manner terminology-wise.  For example:

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).

This implicitly allows for systems that do and don't support Svpbmt.

> +Platform software would likely configure these per root port regions
> such that
> +their effective memory type (PMA + PBMT) is UC. 

same comment as above.

Same suggestion as above.
> +====== PCIe cache coherency
> +PCIe transactions that are not marked as No_snoop and access memory
> that is

/s/No_snoop/No Snoop ?

ARM uses "No_snoop", but others will use No-snoop and No Snoop.  Avoiding the last form avoids any ambiguity when not using it followed by "bit", i.e. "No Snoop bit" is pretty clear, whereas referring to just "No Snoop" as a transaction attribute to some might be ambiguous as to whether "Snoop" or "No Snoop" is the attribute.

Myself, I don't have a significant leaning, but one alternative to the above sentence could be:

PCIe transactions that are marked with the No Snoop bit as zero and access memory ...
or ...
PCIe transactions that are marked with a No Snoop bit of zero and access memory ...

I maybe lean a little bit towards that last option.  That also avoids use of the linguistic double negative.


Join { to automatically receive all group messages.