Re: [PATCH] perf: RISC-V: fix IRQ detection on T-Head C908

From: Atish Patra
Date: Tue Mar 19 2024 - 16:08:19 EST

On 3/19/24 02:06, Conor Dooley wrote:
On Mon, Mar 18, 2024 at 05:48:13PM -0700, Atish Patra wrote:
On 3/18/24 16:48, Conor Dooley wrote:
On Mon, Mar 18, 2024 at 03:46:54PM -0700, Atish Patra wrote:

For 2.b, either we can start defining pseudo extensions or adding
vendor/arch/impid checks.

@Conor: You seems to prefer the earlier approach instead of adding the
checks. Care to elaborate why do you think that's a better method compared
to a simple check ?

Because I don't think that describing these as "errata" in the first
place is even accurate. This is not a case of a vendor claiming they
have Sscofpmf support but the implementation is flawed. As far as I
understand, this is a vendor creating a useful feature prior to the
creation of a standard extension.
A bit of a test for this could be "If the standard extension never
existed, would this be considered a new feature or an implementation
issue". I think this is pretty clearly in the former camp.


So we have 3 cases.

1. Pseudo extension: An vendor extension designed and/or implemented before
the standard RVI extension was ratified but do not violate any standard
encoding space.

2. Erratas: An genuine bug/design issue in the expected behavior from a
standard RVI extension (including violating standard encoding space)

3. Vendor extension: A new or a variant of standard RVI extension which is
different enough from standard extension.

IMO, the line between #2 and #1 may get blurry as we going forward because
of the sheer number of small extensions RVI is comping up with (which is a
problem as well).

Aye, I think some of that is verging on ridiculous.

Just to clarify: I am not too worried about this particular case as we know
that T-head's implementation predates the Sscofpmf extension.
But once we define a standard mechanism for this kind of situation, vendor
may start to abuse it.

How do you envisage it being abused by a vendor?
Pre-dating the standard extension does make this one fairly clear-cut,
but are you worried about people coming along and claiming to implement
XConorSscofpmf instead of Sscofpmf rather than suffer the "shame" of a
broken implementation?

Yes or just use this excuse continue their old implementation in a newer chip which was designed after the standard extension is ratified.


All this stuff is going to be pretty case-by-case (to begin with at
least) so I'm not too worried about that sort of abuse.

I do not think we should be using m*id detection implementations of a
feature prior to creation of a standard extension for the same purpose.
To me the main difference between a case like this and VentanaCondOps/Zicond
is that we are the ones calling this an extension (hence my use of pseudo)
and not the vendor of the IP. If T-Head were to publish a document tomorrow
on the T-Head github repo for official vendor extensions, that difference
would not even exist any longer.


Exactly! If vendor publishes these as an extension or an errata, that's a
binding agreement to call it in a specific way.

I don't agree that we are bound to call it the way that the vendor does.
We should just review these sorts of things on a case-by-case basis,
committing to doing what the vendor says is abusable.

I also do not believe that it is a "simple" check. The number of
implementations that could end up using this PMU could just balloon
if T-Head has no intention of switching to Sscofpmf. If they don't
balloon in this case, there's nothing stopping them ballooning in a

Ideally, they shouldn't as it a simple case of CSR number & IRQ number.
If they care to implement AIA, then they must change it to standard sscofpmf
as the current IRQ violates the AIA spec. But who knows if they care to
implement AIA or not.

What kinda "worried" me here is that the c908 implements /both/ Zicbom
and the T-Head CMO instructions and /both/ Svpbmt and their original
misuse of the reserved bits but they do not support Sscofpmf. Maybe it
just was not feasible to migrate entirely (but they did for vector) or
to support both interrupt numbers and to alias the CSR, but it seemed
like the opportunity to standardise a bunch of other stuff was taken,
but this particular extension was not. That's why I was worried that
we'd see some ballooning in these specific checks.



This is the exact abuse I was talking about. A vendor can keep churning new chips with incompatible extensions because Linux just allowed them get away with it by calling it a Vendor Extension.

+Guo: In case he has any insight behind this decision from Thead.

similar case in the future. We should let the platform firmware tell
explicitly, be that via DT or ACPI, what features are supported rather
than try to reverse engineer it ourselves via m*id.

Fair enough.


That leads into another general issue I have with using m*id detection,
which I think I have mentioned several times on the list - it prevents the
platform (hypervisor, emulator or firmware) from disabling that feature.


If that is the only concern, platform can just disable the actual
extension(i.e. sscofpmf in this case) to disable that feature for that
particular vendor.

Right. Maybe I wasn't clear that this is a problem with using m*id for
/detection/ of extensions and not with using m*id to work around
implementation issues with the extension. In the latter case, you're
applying a fixup only when the actual extension is communicated to be
present, which leaves that control in the hands of the platform.


Hmm. I guess there is no way predict how vendors will include standard extensions. Let's give them a benefit of doubt hoping they realize abusing the system won't help anybody. They will try to adopt the standard extensions if it is possible and creates erratas for genuine problems.

If I had a time machine back to when the T-Head perf or cmo stuff was
submitted, I was try to avoid any of it being merged with the m*id
detection method.

I agree that don't have the crystal ball and may be proven wrong in the
future (I will be definitely happy about that!). But given the diversity of
RISC-V ecosystem, I feel that may be our sad reality.

I don't understand what this comment is referring to, it lacks context
as to what the sad reality actually is.

I hope that all made sense and explained why I am against this method
for detecting what I believe to be features rather than errata,
Conor.


Yes.Thanks again for the clarification. Again, I am not opposed to the idea.
I just wanted to understand if this is the best option we have right now.

_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv