Review comments/questions on Smqosid v0.1

[SiFiveHolland]

• When SSM==1 (i.e. exclusion of the top 2^SRL or 2^SML possible RCID or MCID values), how is supervisor software intended to discover the range of excluded values? Per the pseudocode, a write of an excluded RCID/MCID will be ignored entirely, so writing all 1's will not read back the largest allowed value. Should the behavior be changed to clamp the value instead, to allow this method of discovery?

[rsahita]

So a supervisor domain is expected to be unaware of other supervisor domains - so a supervisor domain is not intended to discover the range of excluded values. so for a supervisor domain where the RDSM is excluding some values of RCID or MCID values, it is expected that only the values the supervisor domain is allowed to write are discoverable.

[rsahita]

@SiFiveHolland does this address your question?

[SiFiveHolland]

No. Maybe my wording was confusing. To me "discover the range of excluded values" is the same as "the values the supervisor domain is allowed to write are discoverable" because those two non-overlapping sets of values are exhaustive. So I'll restate my question: how is supervisor software intended to discover the range of allowed RCID/MCID values when SSM == 1?

My concern is that the algorithm given in the spec will break existing supervisor software. Specifically, an OS today is going to write all 1s to srmcfg.RCID and read back the value to see how many bits are implemented. This works if Smqosid is not implemented, and it works if SSM == 0, but it doesn't work if SSM == 1, because the write will be ignored completely. To avoid breaking existing supervisor software, the hardware needs to instead transform the written excluded value to a valid value, specifically the maximum allowed value. (This also relies on supervisor software treating the read-back value as purely an integer, and not a bit mask, but that's required for SSM == 1 to work regardlesss.)

[ved-rivos]

The RDSM configures SRL to 3 and SSM to 1 for a SD and programs the srmcfg with a value of 24. This implies that the SD is allowed to program the low order 3 bits of srmcfg and thus the effective srmcfg.RCID values for that SD can be selected between 24 and 31. Now consider that the SD does a all 1's write to srmcfg. The processing is as follows for a RCID-value of 0xFFF (all 1's):

    SRL_MASK = (1 << SRL) - 1 => SRL_MASK = (1 << 3) - 1 = 0x7
    srmcfg.RCID = (srmcfg.RCID & ~0x7) | (RCID-value & 0x7) => 24 & ~0x7 | 0xFFF & 0x7 = 0x18 | 0x7 = 0x1F 

So at the end of this operation, srmcfg.RCID holds a value of 0x1F. When the SD reads it back, the operations are as follows:

    SRL_MASK = (1 << SRL) - 1 => 0x7
    RCID-value = srmcfg.RCID & SRL_MASK => 0x1F & 0x7 => 0x7

The read of the srmcfg CSR returns a value of 0x7 to the SD indicating that 3 lower bits of srmcfg are writable. The normal WARL discovery method would work. Please let me know if there are parts of the specification that would imply that the write will be ignored completely

[SiFiveHolland]

Please remember that the pseudocode had the SSM condition backward, so SSM==1 means this pseudocode for the write operation:

    if ((RCID-value & ~SRL_MASK) | SRL_MASK) != ((1 << RCIDLEN) - 1)
        srmcfg.RCID = RCID-value
    endif

Note there is no "else" case here, so if an all-1s value is written, the write will have no effect.

[ved-rivos]

Sorry, I see you asked about SSM=1. I see your point. We could define a legalized value for that case as RCID-value & (2^RCIDLEN - 2^SRL - 1).

[SiFiveHolland]

Yes, that sounds like the right thing to do.